[Crash-utility] [PATCH 1/2] Change functions in extensions/echo.c to be static
lijiang
lijiang at redhat.com
Thu Apr 15 05:33:03 UTC 2021
在 2021年04月02日 08:45, HAGIO KAZUHITO(萩尾 一仁) 写道:
> -----Original Message-----
>> If a same extension(Eg: extensions/trace.so) with two different names are loaded by
>> "extend" command twice, it sometimes segfaults crash.
>>
>> It's because crash uses RTLD_NOW|RTLD_GLOBAL flags of dlopen to load an extension.
>> RTDL_GLOBAL will make symbols defined by this shared object available for
>> symbol resolution of subsequently loaded shared objects. So symbols in subsequently
>> loaded extensions are overwritten by the former loaded one with a same name.
>> Not only can it cause segfaults, but some other unexpected behaviours.
>
> The phenomenon of the first paragraph is an example and a rare situation if
> not on purpose, please let me change the order and generalize a bit more here:
> ---
> The crash utility uses RTLD_NOW|RTLD_GLOBAL flags ... with the same name.
>
> This can cause unexpected behaviors when loading two extension modules that
> have a symbol with the same name. For example, we can reproduce a segmentation
> violation by loading the current trace.so extension module with two different
> names.
> ---
>
> I'll edit when applying. Otherwise, the patch looks good to me.
This change looks good. For the [PATCH 1/2] Change functions in extensions/echo.c to be static
Acked-by: Lianbo Jiang <lijiang at redhat.com>
Thanks.
> Please wait for another ack.
>
> Acked-by: Kazuhito Hagio <k-hagio-ab at nec.com>
>
> Thanks,
> Kazu
>
>>
>> This patch changes functions within extensions/echo.c to be static, and documents
>> the issue in code comments, for extensions developers who takes echo.c as reference.
>>
>> Signed-off-by: Tao Liu <ltao at redhat.com>
>> ---
>> extensions/echo.c | 22 ++++++++++++++--------
>> 1 file changed, 14 insertions(+), 8 deletions(-)
>>
>> diff --git a/extensions/echo.c b/extensions/echo.c
>> index ff1e09a..f4bb88f 100644
>> --- a/extensions/echo.c
>> +++ b/extensions/echo.c
>> @@ -17,19 +17,25 @@
>>
>> #include "defs.h" /* From the crash source top-level directory */
>>
>> -void echo_init(void); /* constructor function */
>> -void echo_fini(void); /* destructor function (optional) */
>> +static void echo_init(void); /* constructor function */
>> +static void echo_fini(void); /* destructor function (optional) */
>>
>> -void cmd_echo(void); /* Declare the commands and their help data. */
>> -char *help_echo[];
>> +static void cmd_echo(void); /* Declare the commands and their help data. */
>> +static char *help_echo[];
>>
>> +/*
>> + * Please making the functions and global variables static within your
>> + * extension if you don't want to make them visiable to subsequently
>> + * loaded extensions. Otherwise, non-static symbols within 2 extensions and
>> + * have a same name can cause confliction.
>> + */
>> static struct command_table_entry command_table[] = {
>> { "echo", cmd_echo, help_echo, 0}, /* One or more commands, */
>> { NULL }, /* terminated by NULL, */
>> };
>>
>>
>> -void __attribute__((constructor))
>> +static void __attribute__((constructor))
>> echo_init(void) /* Register the command set. */
>> {
>> register_extension(command_table);
>> @@ -39,7 +45,7 @@ echo_init(void) /* Register the command set. */
>> * This function is called if the shared object is unloaded.
>> * If desired, perform any cleanups here.
>> */
>> -void __attribute__((destructor))
>> +static void __attribute__((destructor))
>> echo_fini(void) { }
>>
>>
>> @@ -49,7 +55,7 @@ echo_fini(void) { }
>> * other crash commands for usage of the myriad of utility routines available
>> * to accomplish what your task.
>> */
>> -void
>> +static void
>> cmd_echo(void)
>> {
>> int c;
>> @@ -94,7 +100,7 @@ cmd_echo(void)
>> *
>> */
>>
>> -char *help_echo[] = {
>> +static char *help_echo[] = {
>> "echo", /* command name */
>> "echoes back its arguments", /* short description */
>> "arg ...", /* argument synopsis, or " " if none */
>> --
>> 2.29.2
>
More information about the Crash-utility
mailing list