[1/4] elf: Disallow the empty symbol name

Message ID CAMe9rOq9G-5s6vm0BMTHiBDiRAjoBqH_F4XzSdb03s5rwe_GAg@mail.gmail.com
State New
Headers
Series [1/4] elf: Disallow the empty symbol name |

Commit Message

H.J. Lu Sept. 23, 2025, 2:15 a.m. UTC
  Reject the empty symbol name, "".

PR ld/33456
* elflink.c (elf_link_add_object_symbols): Disallow the empty
symbol name.
  

Comments

Alan Modra Sept. 23, 2025, 11:20 a.m. UTC | #1
On Tue, Sep 23, 2025 at 10:15:51AM +0800, H.J. Lu wrote:
> Reject the empty symbol name, "".
> 
> PR ld/33456
> * elflink.c (elf_link_add_object_symbols): Disallow the empty
> symbol name.

OK.
  
Jan Beulich Sept. 23, 2025, 2:34 p.m. UTC | #2
On 23.09.2025 13:20, Alan Modra wrote:
> On Tue, Sep 23, 2025 at 10:15:51AM +0800, H.J. Lu wrote:
>> Reject the empty symbol name, "".
>>
>> PR ld/33456
>> * elflink.c (elf_link_add_object_symbols): Disallow the empty
>> symbol name.
> 
> OK.

Where exactly is it stated that zero-length symbols aren't allowed?
gas allows me to make such a symbol. It isn't consistent though: It
doesn't allow me to make such a symbol global, for example. Imo as
long as such symbols aren't forbidden, this should be at most a
warning.

Jan
  
H.J. Lu Sept. 23, 2025, 8:21 p.m. UTC | #3
On Tue, Sep 23, 2025 at 10:34 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 23.09.2025 13:20, Alan Modra wrote:
> > On Tue, Sep 23, 2025 at 10:15:51AM +0800, H.J. Lu wrote:
> >> Reject the empty symbol name, "".
> >>
> >> PR ld/33456
> >> * elflink.c (elf_link_add_object_symbols): Disallow the empty
> >> symbol name.
> >
> > OK.
>
> Where exactly is it stated that zero-length symbols aren't allowed?
> gas allows me to make such a symbol. It isn't consistent though: It
> doesn't allow me to make such a symbol global, for example. Imo as
> long as such symbols aren't forbidden, this should be at most a
> warning.
>
> Jan

name here is a global symbol, which can't be an empty string.
I will update my commit message with

elf: Disallow the empty global symbol name

Reject the empty global symbol name, "".

        PR ld/33456
        * elflink.c (elf_link_add_object_symbols): Disallow the empty
        global symbol name.
  
Alan Modra Sept. 25, 2025, 1:22 a.m. UTC | #4
On Wed, Sep 24, 2025 at 04:21:33AM +0800, H.J. Lu wrote:
> On Tue, Sep 23, 2025 at 10:34 PM Jan Beulich <jbeulich@suse.com> wrote:
> >
> > On 23.09.2025 13:20, Alan Modra wrote:
> > > On Tue, Sep 23, 2025 at 10:15:51AM +0800, H.J. Lu wrote:
> > >> Reject the empty symbol name, "".
> > >>
> > >> PR ld/33456
> > >> * elflink.c (elf_link_add_object_symbols): Disallow the empty
> > >> symbol name.
> > >
> > > OK.
> >
> > Where exactly is it stated that zero-length symbols aren't allowed?
> > gas allows me to make such a symbol. It isn't consistent though: It
> > doesn't allow me to make such a symbol global, for example. Imo as
> > long as such symbols aren't forbidden, this should be at most a
> > warning.
> >
> > Jan
> 
> name here is a global symbol, which can't be an empty string.
> I will update my commit message with
> 
> elf: Disallow the empty global symbol name
> 
> Reject the empty global symbol name, "".
> 
>         PR ld/33456
>         * elflink.c (elf_link_add_object_symbols): Disallow the empty
>         global symbol name.

This needs tweaking.  When I gave the OK for this patch, I'd seen that
you were changing global syms (locals can have empty names of course),
and thought that a global sym with name "" was useless:  We match
symbols by name when linking, obviously.  The problem is that STT_LOOS
thru STT_HIPROC symbol types might reasonably have empty names.

I'll commit the following after running another testsuite run.  It
fixes
sparc64-linux-gnu  +FAIL: selective2
sparc64-linux-gnu  +FAIL: selective3

	PR ld/33456
	* elflink.c (elf_link_add_object_symbols): Move new check later
	to give the backend add_symbol_hook a chance to remove symbols
	with empty names.

diff --git a/bfd/elflink.c b/bfd/elflink.c
index 0a0456177c2..5c8b822e36a 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -5096,13 +5096,6 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
       if (name == NULL)
 	goto error_free_vers;
 
-      if (name[0] == '\0')
-	{
-	  _bfd_error_handler (_("%pB: corrupt symbol table"), abfd);
-	  bfd_set_error (bfd_error_bad_value);
-	  goto error_free_vers;
-	}
-
       if (isym->st_shndx == SHN_COMMON
 	  && (abfd->flags & BFD_PLUGIN) != 0)
 	{
@@ -5146,6 +5139,13 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
 	    continue;
 	}
 
+      if (name[0] == '\0')
+	{
+	  _bfd_error_handler (_("%pB: corrupt symbol table"), abfd);
+	  bfd_set_error (bfd_error_bad_value);
+	  goto error_free_vers;
+	}
+
       /* Sanity check that all possibilities were handled.  */
       if (sec == NULL)
 	abort ();
  
Jan Beulich Sept. 25, 2025, 6:03 a.m. UTC | #5
On 25.09.2025 03:22, Alan Modra wrote:
> On Wed, Sep 24, 2025 at 04:21:33AM +0800, H.J. Lu wrote:
>> On Tue, Sep 23, 2025 at 10:34 PM Jan Beulich <jbeulich@suse.com> wrote:
>>>
>>> On 23.09.2025 13:20, Alan Modra wrote:
>>>> On Tue, Sep 23, 2025 at 10:15:51AM +0800, H.J. Lu wrote:
>>>>> Reject the empty symbol name, "".
>>>>>
>>>>> PR ld/33456
>>>>> * elflink.c (elf_link_add_object_symbols): Disallow the empty
>>>>> symbol name.
>>>>
>>>> OK.
>>>
>>> Where exactly is it stated that zero-length symbols aren't allowed?
>>> gas allows me to make such a symbol. It isn't consistent though: It
>>> doesn't allow me to make such a symbol global, for example. Imo as
>>> long as such symbols aren't forbidden, this should be at most a
>>> warning.
>>
>> name here is a global symbol, which can't be an empty string.

Why? Where's this stated?

>> I will update my commit message with
>>
>> elf: Disallow the empty global symbol name
>>
>> Reject the empty global symbol name, "".
>>
>>         PR ld/33456
>>         * elflink.c (elf_link_add_object_symbols): Disallow the empty
>>         global symbol name.
> 
> This needs tweaking.  When I gave the OK for this patch, I'd seen that
> you were changing global syms (locals can have empty names of course),
> and thought that a global sym with name "" was useless:  We match
> symbols by name when linking, obviously.

Yes, yet still: A single such symbol could exist in the name space. Unless
I've overlooked something in the spec.

Jan

>  The problem is that STT_LOOS
> thru STT_HIPROC symbol types might reasonably have empty names.
> 
> I'll commit the following after running another testsuite run.  It
> fixes
> sparc64-linux-gnu  +FAIL: selective2
> sparc64-linux-gnu  +FAIL: selective3
> 
> 	PR ld/33456
> 	* elflink.c (elf_link_add_object_symbols): Move new check later
> 	to give the backend add_symbol_hook a chance to remove symbols
> 	with empty names.
> 
> diff --git a/bfd/elflink.c b/bfd/elflink.c
> index 0a0456177c2..5c8b822e36a 100644
> --- a/bfd/elflink.c
> +++ b/bfd/elflink.c
> @@ -5096,13 +5096,6 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
>        if (name == NULL)
>  	goto error_free_vers;
>  
> -      if (name[0] == '\0')
> -	{
> -	  _bfd_error_handler (_("%pB: corrupt symbol table"), abfd);
> -	  bfd_set_error (bfd_error_bad_value);
> -	  goto error_free_vers;
> -	}
> -
>        if (isym->st_shndx == SHN_COMMON
>  	  && (abfd->flags & BFD_PLUGIN) != 0)
>  	{
> @@ -5146,6 +5139,13 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
>  	    continue;
>  	}
>  
> +      if (name[0] == '\0')
> +	{
> +	  _bfd_error_handler (_("%pB: corrupt symbol table"), abfd);
> +	  bfd_set_error (bfd_error_bad_value);
> +	  goto error_free_vers;
> +	}
> +
>        /* Sanity check that all possibilities were handled.  */
>        if (sec == NULL)
>  	abort ();
> 
>
  
Alan Modra Sept. 25, 2025, 11:58 a.m. UTC | #6
On Thu, Sep 25, 2025 at 08:03:31AM +0200, Jan Beulich wrote:
> Yes, yet still: A single such symbol could exist in the name space. Unless
> I've overlooked something in the spec.

You do have a point, but I'll only take it seriously if you can
generate such a symbol in an ELF object file using gas.
  

Patch

From 3c3fab00d18ef3303e143e44aef88a8f60f1f904 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 23 Sep 2025 10:05:14 +0800
Subject: [PATCH 1/4] elf: Disallow the empty symbol name

Reject the empty symbol name, "".

	PR ld/33456
	* elflink.c (elf_link_add_object_symbols): Disallow the empty
	symbol name.

Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
---
 bfd/elflink.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/bfd/elflink.c b/bfd/elflink.c
index b04b017747f..66982f82b94 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -5096,6 +5096,13 @@  elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
       if (name == NULL)
 	goto error_free_vers;
 
+      if (name[0] == '\0')
+	{
+	  _bfd_error_handler (_("%pB: corrupt symbol table"), abfd);
+	  bfd_set_error (bfd_error_bad_value);
+	  goto error_free_vers;
+	}
+
       if (isym->st_shndx == SHN_COMMON
 	  && (abfd->flags & BFD_PLUGIN) != 0)
 	{
-- 
2.51.0