diff mbox series

Fix getting return address in elf/tst-audit28.c.

Message ID 20230713131348.985999-1-stli@linux.ibm.com
State New
Headers show
Series Fix getting return address in elf/tst-audit28.c. | expand

Commit Message

Stefan Liebler July 13, 2023, 1:13 p.m. UTC 
Starting with commit 1bcfe0f732066ae5336b252295591ebe7e51c301, the
test was enhanced and the object for __builtin_return_address (0)
is searched with _dl_find_object.

Unfortunately on e.g. s390 (31bit), a postprocessing step is needed
as the highest bit has to be masked out.  This can be done with
__builtin_extract_return_addr.

Without this postprocessing, _dl_find_object returns with -1 and the
content of dlfo is invalid, which may lead to segfaults in basename.
Therefore those checks are now only done on success.
---
 elf/tst-auditmod28.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

Comments

Stefan Liebler July 19, 2023, 7:06 a.m. UTC | #1 
Ping
On 13.07.23 15:13, Stefan Liebler wrote:
> Starting with commit 1bcfe0f732066ae5336b252295591ebe7e51c301, the
> test was enhanced and the object for __builtin_return_address (0)
> is searched with _dl_find_object.
> 
> Unfortunately on e.g. s390 (31bit), a postprocessing step is needed
> as the highest bit has to be masked out.  This can be done with
> __builtin_extract_return_addr.
> 
> Without this postprocessing, _dl_find_object returns with -1 and the
> content of dlfo is invalid, which may lead to segfaults in basename.
> Therefore those checks are now only done on success.
> ---
>  elf/tst-auditmod28.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/elf/tst-auditmod28.c b/elf/tst-auditmod28.c
> index f6dfbbe202..35180208ba 100644
> --- a/elf/tst-auditmod28.c
> +++ b/elf/tst-auditmod28.c
> @@ -73,12 +73,19 @@ la_version (unsigned int current)
>  
>    /* Check _dl_find_object.  */
>    struct dl_find_object dlfo;
> -  TEST_COMPARE (_dl_find_object (__builtin_return_address (0), &dlfo), 0);
> -  /* "ld.so" is seen with --enable-hardcoded-path-in-tests.  */
> -  if (strcmp (basename (dlfo.dlfo_link_map->l_name), "ld.so") != 0)
> -    TEST_COMPARE_STRING (basename (dlfo.dlfo_link_map->l_name), LD_SO);
> -  TEST_COMPARE (_dl_find_object (dlsym (handle, "environ"), &dlfo), 0);
> -  TEST_COMPARE_STRING (basename (dlfo.dlfo_link_map->l_name), LIBC_SO);
> +  void *ret_addr = __builtin_extract_return_addr (__builtin_return_address (0));
> +  int ret_dl_find_object =_dl_find_object (ret_addr, &dlfo);
> +  TEST_COMPARE (ret_dl_find_object, 0);
> +  if (ret_dl_find_object == 0)
> +    {
> +      /* "ld.so" is seen with --enable-hardcoded-path-in-tests.  */
> +      if (strcmp (basename (dlfo.dlfo_link_map->l_name), "ld.so") != 0)
> +	TEST_COMPARE_STRING (basename (dlfo.dlfo_link_map->l_name), LD_SO);
> +    }
> +  ret_dl_find_object = _dl_find_object (dlsym (handle, "environ"), &dlfo);
> +  TEST_COMPARE (ret_dl_find_object, 0);
> +  if (ret_dl_find_object == 0)
> +    TEST_COMPARE_STRING (basename (dlfo.dlfo_link_map->l_name), LIBC_SO);
>    TEST_COMPARE (_dl_find_object ((void *) 1, &dlfo), -1);
>    TEST_COMPARE (_dl_find_object ((void *) -1, &dlfo), -1);
>
Florian Weimer July 19, 2023, 9:15 a.m. UTC | #2 
* Stefan Liebler:

> Starting with commit 1bcfe0f732066ae5336b252295591ebe7e51c301, the
> test was enhanced and the object for __builtin_return_address (0)
> is searched with _dl_find_object.
>
> Unfortunately on e.g. s390 (31bit), a postprocessing step is needed
> as the highest bit has to be masked out.  This can be done with
> __builtin_extract_return_addr.
>
> Without this postprocessing, _dl_find_object returns with -1 and the
> content of dlfo is invalid, which may lead to segfaults in basename.
> Therefore those checks are now only done on success.
> ---
>  elf/tst-auditmod28.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/elf/tst-auditmod28.c b/elf/tst-auditmod28.c
> index f6dfbbe202..35180208ba 100644
> --- a/elf/tst-auditmod28.c
> +++ b/elf/tst-auditmod28.c
> @@ -73,12 +73,19 @@ la_version (unsigned int current)
>  
>    /* Check _dl_find_object.  */
>    struct dl_find_object dlfo;
> -  TEST_COMPARE (_dl_find_object (__builtin_return_address (0), &dlfo), 0);
> -  /* "ld.so" is seen with --enable-hardcoded-path-in-tests.  */
> -  if (strcmp (basename (dlfo.dlfo_link_map->l_name), "ld.so") != 0)
> -    TEST_COMPARE_STRING (basename (dlfo.dlfo_link_map->l_name), LD_SO);
> -  TEST_COMPARE (_dl_find_object (dlsym (handle, "environ"), &dlfo), 0);
> -  TEST_COMPARE_STRING (basename (dlfo.dlfo_link_map->l_name), LIBC_SO);
> +  void *ret_addr = __builtin_extract_return_addr (__builtin_return_address (0));
> +  int ret_dl_find_object =_dl_find_object (ret_addr, &dlfo);
> +  TEST_COMPARE (ret_dl_find_object, 0);
> +  if (ret_dl_find_object == 0)
> +    {
> +      /* "ld.so" is seen with --enable-hardcoded-path-in-tests.  */
> +      if (strcmp (basename (dlfo.dlfo_link_map->l_name), "ld.so") != 0)
> +	TEST_COMPARE_STRING (basename (dlfo.dlfo_link_map->l_name), LD_SO);
> +    }
> +  ret_dl_find_object = _dl_find_object (dlsym (handle, "environ"), &dlfo);
> +  TEST_COMPARE (ret_dl_find_object, 0);
> +  if (ret_dl_find_object == 0)
> +    TEST_COMPARE_STRING (basename (dlfo.dlfo_link_map->l_name), LIBC_SO);
>    TEST_COMPARE (_dl_find_object ((void *) 1, &dlfo), -1);
>    TEST_COMPARE (_dl_find_object ((void *) -1, &dlfo), -1);

Based on the GCC documentation, this should be reasinable portable.

Reviewed-by: Florian Weimer <fweimer@redhat.com>

Thanks,
Florian
Stefan Liebler July 19, 2023, 11:20 a.m. UTC | #3 
On 19.07.23 11:15, Florian Weimer wrote:
> Reviewed-by: Florian Weimer <fweimer@redhat.com>
> 
> Thanks,
> Florian
> 
Committed.
Thanks for reviewing.
diff mbox series

Patch

diff --git a/elf/tst-auditmod28.c b/elf/tst-auditmod28.c
index f6dfbbe202..35180208ba 100644
--- a/elf/tst-auditmod28.c
+++ b/elf/tst-auditmod28.c
@@ -73,12 +73,19 @@  la_version (unsigned int current)
 
   /* Check _dl_find_object.  */
   struct dl_find_object dlfo;
-  TEST_COMPARE (_dl_find_object (__builtin_return_address (0), &dlfo), 0);
-  /* "ld.so" is seen with --enable-hardcoded-path-in-tests.  */
-  if (strcmp (basename (dlfo.dlfo_link_map->l_name), "ld.so") != 0)
-    TEST_COMPARE_STRING (basename (dlfo.dlfo_link_map->l_name), LD_SO);
-  TEST_COMPARE (_dl_find_object (dlsym (handle, "environ"), &dlfo), 0);
-  TEST_COMPARE_STRING (basename (dlfo.dlfo_link_map->l_name), LIBC_SO);
+  void *ret_addr = __builtin_extract_return_addr (__builtin_return_address (0));
+  int ret_dl_find_object =_dl_find_object (ret_addr, &dlfo);
+  TEST_COMPARE (ret_dl_find_object, 0);
+  if (ret_dl_find_object == 0)
+    {
+      /* "ld.so" is seen with --enable-hardcoded-path-in-tests.  */
+      if (strcmp (basename (dlfo.dlfo_link_map->l_name), "ld.so") != 0)
+	TEST_COMPARE_STRING (basename (dlfo.dlfo_link_map->l_name), LD_SO);
+    }
+  ret_dl_find_object = _dl_find_object (dlsym (handle, "environ"), &dlfo);
+  TEST_COMPARE (ret_dl_find_object, 0);
+  if (ret_dl_find_object == 0)
+    TEST_COMPARE_STRING (basename (dlfo.dlfo_link_map->l_name), LIBC_SO);
   TEST_COMPARE (_dl_find_object ((void *) 1, &dlfo), -1);
   TEST_COMPARE (_dl_find_object ((void *) -1, &dlfo), -1);