[v2,1/2] elf: strdup() l_name if no realname [BZ #30100]

Message ID 20230211200436.2860281-1-stsp2@yandex.ru
State Superseded
Headers
Series [v2,1/2] elf: strdup() l_name if no realname [BZ #30100] |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

stsp Feb. 11, 2023, 8:04 p.m. UTC
  _dl_close_worker() has this code:
      /* This name always is allocated.  */
      free (imap->l_name);

But in that particular case, while indeed being allocated, l_name
doesn't point to the start of an allocation:
  new = (struct link_map *) calloc (sizeof (*new) + audit_space
                                    + sizeof (struct link_map *)
                                    + sizeof (*newname) + libname_len, 1);
  ...
  new->l_symbolic_searchlist.r_list = (struct link_map **) ((char *) (new + 1)
                                                            + audit_space);

  new->l_libname = newname
    = (struct libname_list *) (new->l_symbolic_searchlist.r_list + 1);
  newname->name = (char *) memcpy (newname + 1, libname, libname_len);
  ...
  new->l_name = (char *) newname->name + libname_len - 1;

It therefore cannot be freed separately.
Use strdup() as a simple fix.

Signed-off-by: Stas Sergeev <stsp2@yandex.ru>
---
 elf/dl-object.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Florian Weimer Feb. 12, 2023, 1:44 p.m. UTC | #1
* Stas Sergeev via Libc-alpha:

> _dl_close_worker() has this code:
>       /* This name always is allocated.  */
>       free (imap->l_name);
>
> But in that particular case, while indeed being allocated, l_name
> doesn't point to the start of an allocation:
>   new = (struct link_map *) calloc (sizeof (*new) + audit_space
>                                     + sizeof (struct link_map *)
>                                     + sizeof (*newname) + libname_len, 1);
>   ...
>   new->l_symbolic_searchlist.r_list = (struct link_map **) ((char *) (new + 1)
>                                                             + audit_space);
>
>   new->l_libname = newname
>     = (struct libname_list *) (new->l_symbolic_searchlist.r_list + 1);
>   newname->name = (char *) memcpy (newname + 1, libname, libname_len);
>   ...
>   new->l_name = (char *) newname->name + libname_len - 1;
>
> It therefore cannot be freed separately.
> Use strdup() as a simple fix.

The problematic adjustment should only happen for objects which cannot
be closed.  I think you see this in your dlmem case because it
introduces unnamed shared objects that can be closed.  It may be
better to change the dlmem interface to explicitly name the shared
objects because that assumption is likely widespread throughout the
system (even outside glibc, in debuggers, for example).
  

Patch

diff --git a/elf/dl-object.c b/elf/dl-object.c
index f1f2ec956c..c92daf37d1 100644
--- a/elf/dl-object.c
+++ b/elf/dl-object.c
@@ -122,7 +122,7 @@  _dl_new_object (char *realname, const char *libname, int type,
 #endif
     new->l_name = realname;
   else
-    new->l_name = (char *) newname->name + libname_len - 1;
+    new->l_name = __strdup ((char *) newname->name + libname_len - 1);
 
   new->l_type = type;
   /* If we set the bit now since we know it is never used we avoid