[PATCH] elf: handle NULL input to fatal_error

[PATCH] elf: handle NULL input to fatal_error

[Jiangfeng Xiao]

"dlopen_doit" may execute
"_dl_signal_error (0, NULL, NULL, ...)",
which cause a segmentation fault.

The call stack is as follows:

Program received signal SIGSEGV, Segmentation fault.
fatal_error (errcode=errcode@entry=0, objname=0x0, occasion=0x0,
    errstring=errstring@entry=0xf7c90518 "invalid mode parameter")
(gdb) bt
@0  fatal_error (errcode=errcode@entry=0, objname=0x0, occasion=0x0,
    errstring=errstring@entry=0xf7c90518 "invalid mode parameter")
@1  0xf7de5260 in __GI__dl_signal_error (errcode=0, objname=0x0, occation=0x0,
    errstring=0xf7c90518 "invalid mode parameter")
@2  0xf7d0e204 in dlopen_doit (a=a@entry=0xfffefa94)

When objname is NULL, referencing *objname accesses a null pointer.
Therefore, *objname is changed to objname.

After this bug is fixed, if objname is NULL, the "strlen"
in _dl_fatal_printf->_dl_debug_vdprintf will produce
another segmentation fault.

The call stack is as follows:

Program received signal SIGSEGV, Segmentation fault.
strlen () at ../sysdeps/arm/armv6t2/strlen.S:85
(gdb) bt
@0  strlen () at ../sysdeps/arm/armv6t2/strlen.S:85
@1  0xf7d7fd40 in _dl_debug_vdprintf (fd=2, tag_p=0, fmt=0xf7ab83ab "s%s%s%s%s\n", arg=...)
@2  0xf7d8006c in __GI__dl_fatal_printf (fmt=0xf7ab83a2 "%s: %s: %s%s%s%s%s\n")
@3  0xf7c0b204 in fatal_error (errcode@entry=0, objname=0x0, occasion=0x0,
    errstring=errstring@entry=0xf7ab6518 "invalid mode parameter")
@4  0xf7c0b258 in __GI__dl_signal_error (errcode=0, objname=0x0,
    occation=0x0 errstring=0xf7ab6518 "invalid mode parameter")
@5  0xf7b34204 in dlopen_doit (a=a@entry=0xff9f7434)

Therefore, null check are required for "objname" and "errstring".

Fixes: 2449ae7b2da24 ("ld.so: Introduce struct dl_exception")

Signed-off-by: Jiangfeng Xiao <xiaojiangfeng@huawei.com>
---
 elf/dl-catch.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/elf/dl-catch.c b/elf/dl-catch.c
index 2109516..05a41d1 100644
--- a/elf/dl-catch.c
+++ b/elf/dl-catch.c
@@ -83,8 +83,8 @@ fatal_error (int errcode, const char *objname, const char *occasion,
   _dl_fatal_printf ("%s: %s: %s%s%s%s%s\n",
 		    RTLD_PROGNAME,
 		    occasion ?: N_("error while loading shared libraries"),
-		    objname, *objname ? ": " : "",
-		    errstring, errcode ? ": " : "",
+		    objname ? objname : "", objname ? ": " : "",
+		    errstring ? errstring : "", errcode ? ": " : "",
 		    (errcode
 		     ? __strerror_r (errcode, buffer, sizeof buffer)
 		     : ""));
-- 
1.8.5.6

Re: [PATCH] elf: handle NULL input to fatal_error

[Andreas Schwab]

_dl_signal_error used to set objname to "" if it is null, it should
continue to do so (this has been removed in commit 2449ae7b2d).  It
still sanitizes errstring, so nothing needs to be done about that.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] elf: handle NULL input to fatal_error

[Jiangfeng Xiao]

On 2024/3/30 23:47, Andreas Schwab wrote:
> _dl_signal_error used to set objname to "" if it is null, it should
> continue to do so (this has been removed in commit 2449ae7b2d).  It
> still sanitizes errstring, so nothing needs to be done about that.
> 

Thank you very much, it sounds good to me.

I'm going to submit a patch to sanitize objname in _dl_signal_error,
just like early code.

[PATCH] elf: sanitize objname in _dl_signal_error

[Jiangfeng Xiao]

"dlopen_doit" may execute
"_dl_signal_error (0, NULL, NULL, ...)",
which cause a segmentation fault.

The call stack is as follows:

Program received signal SIGSEGV, Segmentation fault.
fatal_error (errcode=errcode@entry=0, objname=0x0, occasion=0x0,
    errstring=errstring@entry=0xf7c90518 "invalid mode parameter")
(gdb) bt
@0  fatal_error (errcode=errcode@entry=0, objname=0x0, occasion=0x0,
    errstring=errstring@entry=0xf7c90518 "invalid mode parameter")
@1  0xf7de5260 in __GI__dl_signal_error (errcode=0, objname=0x0, occation=0x0,
    errstring=0xf7c90518 "invalid mode parameter")
@2  0xf7d0e204 in dlopen_doit (a=a@entry=0xfffefa94)

When objname is NULL, referencing *objname will accesses
a null pointer. As a result, a segmentation fault occurs.

_dl_signal_error used to set objname to "" if it is null,
it should continue to do so (this has been removed in commit 2449ae7b2d)

Fixes: 2449ae7b2da24 ("ld.so: Introduce struct dl_exception")

Suggested-by: Andreas Schwab <schwab@linux-m68k.org>
Link: https://public-inbox.org/libc-alpha/1711806052-117857-1-git-send-email-xiaojiangfeng@huawei.com/
Signed-off-by: Jiangfeng Xiao <xiaojiangfeng@huawei.com>
---
 elf/dl-catch.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/elf/dl-catch.c b/elf/dl-catch.c
index 2109516..92f0654 100644
--- a/elf/dl-catch.c
+++ b/elf/dl-catch.c
@@ -117,6 +117,9 @@ _dl_signal_error (int errcode, const char *objname, const char *occasion,
   if (! errstring)
     errstring = N_("DYNAMIC LINKER BUG!!!");
 
+  if (! objname)
+    objname = "";
+
   if (lcatch != NULL)
     {
       _dl_exception_create (lcatch->exception, objname, errstring);
-- 
1.8.5.6

Re: [PATCH] elf: sanitize objname in _dl_signal_error

[Adhemerval Zanella Netto]

On 31/03/24 23:45, Jiangfeng Xiao wrote:
> "dlopen_doit" may execute
> "_dl_signal_error (0, NULL, NULL, ...)",
> which cause a segmentation fault.
> 
> The call stack is as follows:
> 
> Program received signal SIGSEGV, Segmentation fault.
> fatal_error (errcode=errcode@entry=0, objname=0x0, occasion=0x0,
>     errstring=errstring@entry=0xf7c90518 "invalid mode parameter")
> (gdb) bt
> @0  fatal_error (errcode=errcode@entry=0, objname=0x0, occasion=0x0,
>     errstring=errstring@entry=0xf7c90518 "invalid mode parameter")
> @1  0xf7de5260 in __GI__dl_signal_error (errcode=0, objname=0x0, occation=0x0,
>     errstring=0xf7c90518 "invalid mode parameter")
> @2  0xf7d0e204 in dlopen_doit (a=a@entry=0xfffefa94)
> 
> When objname is NULL, referencing *objname will accesses
> a null pointer. As a result, a segmentation fault occurs.
> 
> _dl_signal_error used to set objname to "" if it is null,
> it should continue to do so (this has been removed in commit 2449ae7b2d)
> 
> Fixes: 2449ae7b2da24 ("ld.so: Introduce struct dl_exception")

How did you trigger this issue, either from user provided ABI (dlfcn.h)
or some some internal usage (if any)? If this is a user-visible issue
it will require a bug report and a reproducer.

> 
> Suggested-by: Andreas Schwab <schwab@linux-m68k.org>
> Link: https://public-inbox.org/libc-alpha/1711806052-117857-1-git-send-email-xiaojiangfeng@huawei.com/
> Signed-off-by: Jiangfeng Xiao <xiaojiangfeng@huawei.com>
> ---
>  elf/dl-catch.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/elf/dl-catch.c b/elf/dl-catch.c
> index 2109516..92f0654 100644
> --- a/elf/dl-catch.c
> +++ b/elf/dl-catch.c
> @@ -117,6 +117,9 @@ _dl_signal_error (int errcode, const char *objname, const char *occasion,
>    if (! errstring)
>      errstring = N_("DYNAMIC LINKER BUG!!!");
>  
> +  if (! objname)
> +    objname = "";
> +
>    if (lcatch != NULL)
>      {
>        _dl_exception_create (lcatch->exception, objname, errstring);

Re: [PATCH] elf: sanitize objname in _dl_signal_error

[Jiangfeng Xiao]

On 2024/4/1 21:50, Adhemerval Zanella Netto wrote:


> How did you trigger this issue, either from user provided ABI (dlfcn.h)
> or some some internal usage (if any)? If this is a user-visible issue
> it will require a bug report and a reproducer.
> 

Thanks for your reply.


The following are my reproduction cases:

```
#include <dlfcn.h>

int main(void)
{
	(void)dlopen("not_exist.so", -1);

	return 0;
}

```

However, this case cannot be reproduced in a common environment.

I reproduced this issue in the arm32 environment.
Glibc in the environment is compiled using the Clang compiler.
The glibc version is 2.34. (The patches that supports Clang
compilation has been applied to this version)

I have not figured out why the lcatch variable
in the _dl_signal_error function is null.
As a result, the exception branch
fatal_error(0, NULL, NULL, NULL, "invalid mode parameter")
is executed.
Maybe my Clang compiler's compilation parameters
are not configured properly.

I can then be sure that if glibc is compiled by the GCC compiler,
it should not trigger this issue.

I don't think the glibc mainline branch will trigger this problem
because glibc has not officially promised to support Clang.
So I think I'd rather not submit a bug report first.

Re: [PATCH] elf: sanitize objname in _dl_signal_error

[H.J. Lu]

On Tue, Apr 2, 2024 at 7:38 AM Jiangfeng Xiao <xiaojiangfeng@huawei.com> wrote:
>
>
>
> On 2024/4/1 21:50, Adhemerval Zanella Netto wrote:
>
>
> > How did you trigger this issue, either from user provided ABI (dlfcn.h)
> > or some some internal usage (if any)? If this is a user-visible issue
> > it will require a bug report and a reproducer.
> >
>
> Thanks for your reply.
>
>
> The following are my reproduction cases:
>
> ```
> #include <dlfcn.h>
>
> int main(void)
> {
>         (void)dlopen("not_exist.so", -1);
>
>         return 0;
> }
>
> ```
>
> However, this case cannot be reproduced in a common environment.
>
> I reproduced this issue in the arm32 environment.
> Glibc in the environment is compiled using the Clang compiler.

Is it a Clang bug?

> The glibc version is 2.34. (The patches that supports Clang
> compilation has been applied to this version)
>
> I have not figured out why the lcatch variable
> in the _dl_signal_error function is null.
> As a result, the exception branch
> fatal_error(0, NULL, NULL, NULL, "invalid mode parameter")
> is executed.
> Maybe my Clang compiler's compilation parameters
> are not configured properly.
>
> I can then be sure that if glibc is compiled by the GCC compiler,
> it should not trigger this issue.
>
> I don't think the glibc mainline branch will trigger this problem
> because glibc has not officially promised to support Clang.
> So I think I'd rather not submit a bug report first.



-- 
H.J.

Re: [PATCH] elf: sanitize objname in _dl_signal_error

[Jiangfeng Xiao]

On 2024/4/2 22:42, H.J. Lu wrote:
> On Tue, Apr 2, 2024 at 7:38 AM Jiangfeng Xiao <xiaojiangfeng@huawei.com> wrote:
>>
>>
>>
>> On 2024/4/1 21:50, Adhemerval Zanella Netto wrote:
>>
>>
>>> How did you trigger this issue, either from user provided ABI (dlfcn.h)
>>> or some some internal usage (if any)? If this is a user-visible issue
>>> it will require a bug report and a reproducer.
>>>
>>
>> Thanks for your reply.
>>
>>
>> The following are my reproduction cases:
>>
>> ```
>> #include <dlfcn.h>
>>
>> int main(void)
>> {
>>         (void)dlopen("not_exist.so", -1);
>>
>>         return 0;
>> }
>>
>> ```
>>
>> However, this case cannot be reproduced in a common environment.
>>
>> I reproduced this issue in the arm32 environment.
>> Glibc in the environment is compiled using the Clang compiler.
> 
> Is it a Clang bug?
> 

Maybe. However, the glibc code may reference null pointers,
which should be fixed.

>> The glibc version is 2.34. (The patches that supports Clang
>> compilation has been applied to this version)
>>
>> I have not figured out why the lcatch variable
>> in the _dl_signal_error function is null.
>> As a result, the exception branch
>> fatal_error(0, NULL, NULL, NULL, "invalid mode parameter")
>> is executed.
>> Maybe my Clang compiler's compilation parameters
>> are not configured properly.
>>
>> I can then be sure that if glibc is compiled by the GCC compiler,
>> it should not trigger this issue.
>>
>> I don't think the glibc mainline branch will trigger this problem
>> because glibc has not officially promised to support Clang.
>> So I think I'd rather not submit a bug report first.
> 
> 
> 

Re: [PATCH] elf: sanitize objname in _dl_signal_error

[H.J. Lu]

On Tue, Apr 2, 2024 at 7:54 AM Jiangfeng Xiao <xiaojiangfeng@huawei.com> wrote:
>
>
>
> On 2024/4/2 22:42, H.J. Lu wrote:
> > On Tue, Apr 2, 2024 at 7:38 AM Jiangfeng Xiao <xiaojiangfeng@huawei.com> wrote:
> >>
> >>
> >>
> >> On 2024/4/1 21:50, Adhemerval Zanella Netto wrote:
> >>
> >>
> >>> How did you trigger this issue, either from user provided ABI (dlfcn.h)
> >>> or some some internal usage (if any)? If this is a user-visible issue
> >>> it will require a bug report and a reproducer.
> >>>
> >>
> >> Thanks for your reply.
> >>
> >>
> >> The following are my reproduction cases:
> >>
> >> ```
> >> #include <dlfcn.h>
> >>
> >> int main(void)
> >> {
> >>         (void)dlopen("not_exist.so", -1);
> >>
> >>         return 0;
> >> }
> >>
> >> ```
> >>
> >> However, this case cannot be reproduced in a common environment.
> >>
> >> I reproduced this issue in the arm32 environment.
> >> Glibc in the environment is compiled using the Clang compiler.
> >
> > Is it a Clang bug?
> >
>
> Maybe. However, the glibc code may reference null pointers,
> which should be fixed.

Not if a null pointer should never happen.

> >> The glibc version is 2.34. (The patches that supports Clang
> >> compilation has been applied to this version)
> >>
> >> I have not figured out why the lcatch variable
> >> in the _dl_signal_error function is null.
> >> As a result, the exception branch
> >> fatal_error(0, NULL, NULL, NULL, "invalid mode parameter")
> >> is executed.
> >> Maybe my Clang compiler's compilation parameters
> >> are not configured properly.
> >>
> >> I can then be sure that if glibc is compiled by the GCC compiler,
> >> it should not trigger this issue.
> >>
> >> I don't think the glibc mainline branch will trigger this problem
> >> because glibc has not officially promised to support Clang.
> >> So I think I'd rather not submit a bug report first.
> >
> >
> >



-- 
H.J.

Re: [PATCH] elf: sanitize objname in _dl_signal_error

[Jiangfeng Xiao]

On 2024/4/2 23:00, H.J. Lu wrote:

>>>
>>
>> Maybe. However, the glibc code may reference null pointers,
>> which should be fixed.
> 
> Not if a null pointer should never happen.
> 
If the fatal_error(0, NULL, NULL, NULL, "invalid mode parameter")
code in the _dl_signal_error function will never be executed,
delete the redundant code. If it is possible, even if the
possibility is very low, it should be fixed.

Re: [PATCH] elf: sanitize objname in _dl_signal_error

[H.J. Lu]

On Tue, Apr 2, 2024 at 8:06 AM Jiangfeng Xiao <xiaojiangfeng@huawei.com> wrote:
>
>
>
> On 2024/4/2 23:00, H.J. Lu wrote:
>
> >>>
> >>
> >> Maybe. However, the glibc code may reference null pointers,
> >> which should be fixed.
> >
> > Not if a null pointer should never happen.
> >
> If the fatal_error(0, NULL, NULL, NULL, "invalid mode parameter")

Please open a glibc bug to track it.

> code in the _dl_signal_error function will never be executed,
> delete the redundant code. If it is possible, even if the
> possibility is very low, it should be fixed.



-- 
H.J.

Re: [PATCH] elf: sanitize objname in _dl_signal_error

[Jiangfeng Xiao]

On 2024/4/2 23:08, H.J. Lu wrote:
> On Tue, Apr 2, 2024 at 8:06 AM Jiangfeng Xiao <xiaojiangfeng@huawei.com> wrote:
>>
>>
>>
>> On 2024/4/2 23:00, H.J. Lu wrote:
>>
>>>>>
>>>>
>>>> Maybe. However, the glibc code may reference null pointers,
>>>> which should be fixed.
>>>
>>> Not if a null pointer should never happen.
>>>
>> If the fatal_error(0, NULL, NULL, NULL, "invalid mode parameter")
> 
> Please open a glibc bug to track it.
> 

All right, I've submitted a bug report.
https://sourceware.org/bugzilla/show_bug.cgi?id=31596

I will try to track this issue and update the progress.

>> code in the _dl_signal_error function will never be executed,
>> delete the redundant code. If it is possible, even if the
>> possibility is very low, it should be fixed.
> 
> 
> 

Re: [PATCH] elf: sanitize objname in _dl_signal_error

[Adhemerval Zanella Netto]

On 02/04/24 11:42, H.J. Lu wrote:
> On Tue, Apr 2, 2024 at 7:38 AM Jiangfeng Xiao <xiaojiangfeng@huawei.com> wrote:
>>
>>
>>
>> On 2024/4/1 21:50, Adhemerval Zanella Netto wrote:
>>
>>
>>> How did you trigger this issue, either from user provided ABI (dlfcn.h)
>>> or some some internal usage (if any)? If this is a user-visible issue
>>> it will require a bug report and a reproducer.
>>>
>>
>> Thanks for your reply.
>>
>>
>> The following are my reproduction cases:
>>
>> ```
>> #include <dlfcn.h>
>>
>> int main(void)
>> {
>>         (void)dlopen("not_exist.so", -1);
>>
>>         return 0;
>> }
>>
>> ```
>>
>> However, this case cannot be reproduced in a common environment.
>>
>> I reproduced this issue in the arm32 environment.
>> Glibc in the environment is compiled using the Clang compiler.
> 
> Is it a Clang bug?

This does seems to be a code generation issue, I just tested by azanella/clang
branch with both clang 16 (official package) and clang master and there is not
issue with dlopen non existent files.