[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190228220129.GA2383@darth.lan>
Date: Thu, 28 Feb 2019 23:01:29 +0100
From: Sebastian Kemper <sebastian_ml@....net>
To: musl@...ts.openwall.com
Cc: Szabolcs Nagy <nsz@...t70.net>
Subject: Re: Asterisk 16 segmentation fault
On Thu, Feb 28, 2019 at 10:19:02PM +0100, Szabolcs Nagy wrote:
> * Sebastian Kemper <sebastian_ml@....net> [2019-02-28 21:27:51 +0100]:
> > I'm not a programmer so I have problems making sense of this. So I'm
> > hoping that maybe one of you can shine a light.
>
> it means dlerror() returned 0 and ast_strdupa calls strlen on
> this return value which segfaults as expected.
>
> it is entirely valid for dlerror() to return 0 if there was no
> error. did the dlopen("res_pjproject.so", flags) call succeed?
> i.e. mod->lib != 0 ?.
Hello Szabolcs,
Thanks for your reply. And thanks to Rich & Jeffrey as well, of course.
Again I'm blown away by how fast one gets a reply on the musl list :-)
I stepped through it again and tried to get the value of mod->lib:
952 if (resource_being_loaded) {
(gdb) step
951 mod->lib = dlopen(filename, flags);
(gdb) step
952 if (resource_being_loaded) {
(gdb) p mod->lib
$10 = (void *) 0x5fc210
(gdb) p mod->lib
$11 = (void *) 0x5fc210
(gdb) p mod->lib
$12 = (void *) 0x5fc210
(gdb) p mod->lib
$13 = (void *) 0x5fc210
(gdb) p (*mod).lib
$14 = (void *) 0x5fc210
(gdb) p (*mod).li
There is no member named li.
(gdb) p (*mod).lib
$15 = (void *) 0x5fc210
(gdb) step
955 const char *dlerror_msg = ast_strdupa(dlerror());
(gdb) step
Thread 1 "asterisk" received signal SIGSEGV, Segmentation fault.
strlen (s=0x0, s@...ry=0x48d79d <load_dynamic_module+120> "\t\360\"\223\f\234\200\353\216#\005\032\240z\364e") at src/string/strlen.c:17
17 for (w = (const void *)s; !HASZERO(*w); w++);
(gdb)
So if I'm not mistaken mod->lib is indeed != 0.
> e.g. the segfault can be avoided by
>
> - const char *dlerror_msg = ast_strdupa(dlerror());
> + const char *dlerror_msg = dlerror(); dlerror_msg = ast_strdupa(dlerror_msg ? dlerror_msg : "");
>
> but we would need to know what this code is trying to do
> (and how it worked before) for a proper fix.
astmm.h defines ast_strdupa() like this:
#if !defined(ast_strdupa) && defined(__GNUC__)
/*!
* \brief duplicate a string in memory from the stack
* \param s The string to duplicate
*
* This macro will duplicate the given string. It returns a pointer to the stack
* allocatted memory for the new string.
*/
#define ast_strdupa(s) \
(__extension__ \
({ \
const char *__old = (s); \
size_t __len = strlen(__old) + 1; \
char *__new = __builtin_alloca(__len); \
memcpy (__new, __old, __len); \
__new; \
}))
#endif
This define is the same in both Asterisk 15 and 16 (the define only has
moved from include/asterisk/utils.h to include/asterisk/astmm.h).
The 'static struct ast_module *load_dlopen()' in which it's used has
changed, though, between Asterisk 15 and 16:
@@ -569,13 +950,60 @@
resource_being_loaded = mod;
mod->lib = dlopen(filename, flags);
if (resource_being_loaded) {
+ struct ast_str *list;
+ int c = 0;
+ const char *dlerror_msg = ast_strdupa(dlerror());
+
resource_being_loaded = NULL;
if (mod->lib) {
- ast_log(LOG_ERROR, "Module '%s' did not register itself during load\n", resource_in);
+ module_load_error("Module '%s' did not register itself during load\n", resource_in);
logged_dlclose(resource_in, mod->lib);
- } else if (!suppress_logging) {
- ast_log(LOG_WARNING, "Error loading module '%s': %s\n", resource_in, dlerror());
+
+ goto error_return;
+ }
+
+ if (suppress_logging) {
+ goto error_return;
}
+
+ resource_being_loaded = mod;
+ mod->lib = dlopen(filename, RTLD_LAZY | RTLD_LOCAL);
+ if (resource_being_loaded) {
+ resource_being_loaded = NULL;
+
+ module_load_error("Error loading module '%s': %s\n", resource_in, dlerror_msg);
+ logged_dlclose(resource_in, mod->lib);
+
+ goto error_return;
+ }
+
+ list = ast_str_create(64);
+ if (list) {
+ if (module_post_register(mod)) {
+ goto loaded_error;
+ }
+
+ c = load_dlopen_missing(&list, &mod->requires);
+ c += load_dlopen_missing(&list, &mod->enhances);
+#ifndef OPTIONAL_API
+ c += load_dlopen_missing(&list, &mod->optional_modules);
+#endif
+ }
+
+ if (list && ast_str_strlen(list)) {
+ module_load_error("Error loading module '%s', missing %s: %s\n",
+ resource_in, c == 1 ? "dependency" : "dependencies", ast_str_buffer(list));
+ } else {
+ module_load_error("Error loading module '%s': %s\n", resource_in, dlerror_msg);
+ }
+
+loaded_error:
+ ast_free(list);
+ unload_dynamic_module(mod);
+
+ return NULL;
+
+error_return:
ast_free(mod);
return NULL;
@@ -584,12 +1012,11 @@
So 'const char *dlerror_msg' seems to only be used for logging purposes.
Should I give the proposed change a spin?
I'm off to bed now. I would get back to you tomorrow evening.
Again thank you all, much appreciated!
With kind regards,
Seb
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
