[PATCH] ld.so: Decorate BSS mappings

[Adhemerval Zanella Netto]

On 22/01/25 11:32, Petr Malat wrote:
> Hi!
> On Tue, Jan 21, 2025 at 02:56:33PM -0300, Adhemerval Zanella Netto wrote:
>> On 21/01/25 06:25, Petr Malat wrote:
>>> Decorate BSS mappings with [anon: .bss <file>], for example
>>> [anon: .bss /lib/libc.so.6]. The string ".bss" is already used
>>> by bionic so use the same, but add the filename as well.
>>>
>>> Signed-off-by: Petr Malat <oss@malat.biz>
>>> ---
>>>  elf/dl-map-segments.h | 20 ++++++++++++++++++++
>>>  1 file changed, 20 insertions(+)
>>>
>>> diff --git a/elf/dl-map-segments.h b/elf/dl-map-segments.h
>>> index 203b6c7b0b..cf6be4a783 100644
>>> --- a/elf/dl-map-segments.h
>>> +++ b/elf/dl-map-segments.h
>>> @@ -18,6 +18,7 @@
>>>     <https://www.gnu.org/licenses/>.  */
>>>
>>>  #include <dl-load.h>
>>> +#include <setvmaname.h>
>>>
>>>  /* Map a segment and align it properly.  */
>>>
>>> @@ -182,12 +183,31 @@ _dl_map_segments (struct link_map *l, int fd,
>>>            if (zeroend > zeropage)
>>>              {
>>>                /* Map the remaining zero pages in from the zero fill FD.  */
>>> +              char bssname[80] = " .bss";
>>
>> I think we should keep how we present other mappings, but prepending the
>> ' glibc: '.
> 
> This is a mapping set up by ld.so and felt like using glibc may make somebody
> think it was set up by libc, but in general I have no strict opinion in this.
> Bionic uses just ".bss".

But ld.so here is glibc, since the idea of prepending is to advertise that
it was done by the libc (including any helper library) instead of by the program
or any other library (although we can't really enforce anything here).

> 
> 
>> We define ANON_VMA_NAME_MAX_LEN on allocatestack.c, and now it is used in a
>> different place I think we should move it to setvmaname.h and use it instead
>> of '80'.
> 
> ACK
> 
> 
>>>                caddr_t mapat;
>>>                mapat = __mmap ((caddr_t) zeropage, zeroend - zeropage,
>>>                                c->prot, MAP_ANON|MAP_PRIVATE|MAP_FIXED,
>>>                                -1, 0);
>>>                if (__glibc_unlikely (mapat == MAP_FAILED))
>>>                  return DL_MAP_SEGMENTS_ERROR_MAP_ZERO_FILL;
>>> +              if (l->l_name && *l->l_name)
>>> +                {
>>> +                  int i = strlen(bssname), j = 0;
>>> +                  bssname[i++] = ' ';
>>> +                  if (strlen(l->l_name) > sizeof(bssname) - i - 1)
>>> +                    j = strlen(l->l_name) - (sizeof(bssname) - i - 1);
>>> +                  for (; l->l_name[j]; i++, j++)
>>> +                    {
>>> +                      char ch = l->l_name[j];
>>> +                      /* Replace non-printable characters and \, `, $, [ and ] */
>>> +                      if (ch <= 0x1f || ch >= 0x7f || strchr("\\`$[]", ch))
>>> +                        ch = '!';
>>> +                      bssname[i] = ch;
>>> +                    }
>>> +                  bssname[i] = 0;
>>> +                }
>>> +              __set_vma_name ((void*)zeropage, zeroend - zeropage, bssname);
>>
>> I wonder if showing the path of the link_map makes sense here, on some systems
>> (NIX for instance) the limited name size limitation will makes this information
>> moot. Since this code will already clamp the path if this is too long, maybe
>> just preset the libname, like '[anon: glibc: .bss libc.so.6]'
> 
> I like the full path, because it nicely matches other (file) mappings from
> the same library. What about using the full path if it fits and if not,
> using just the file name?

Sounds reasonable.