diff mbox series

elf: sanitize objname in _dl_signal_error

Message ID 1711939509-1411-1-git-send-email-xiaojiangfeng@huawei.com
State New
Headers show
Series elf: sanitize objname in _dl_signal_error | expand

Commit Message

Jiangfeng Xiao April 1, 2024, 2:45 a.m. UTC
"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(+)

Comments

Adhemerval Zanella Netto April 1, 2024, 1:50 p.m. UTC | #1
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);
Jiangfeng Xiao April 2, 2024, 2:37 p.m. UTC | #2
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.
H.J. Lu April 2, 2024, 2:42 p.m. UTC | #3
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.
Jiangfeng Xiao April 2, 2024, 2:54 p.m. UTC | #4
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.
> 
> 
>
H.J. Lu April 2, 2024, 3 p.m. UTC | #5
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.
> >
> >
> >
Jiangfeng Xiao April 2, 2024, 3:06 p.m. UTC | #6
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.
H.J. Lu April 2, 2024, 3:08 p.m. UTC | #7
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.
Jiangfeng Xiao April 2, 2024, 3:21 p.m. UTC | #8
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.
> 
> 
>
Adhemerval Zanella Netto April 2, 2024, 3:50 p.m. UTC | #9
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.
diff mbox series

Patch

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);