[committed,v2,1/2] elf: Accept absolute (SHN_ABS) symbols whose value is zero [BZ #23307]
diff mbox series

Message ID alpine.DEB.2.00.1806291651000.20622@tp.orcam.me.uk
State Accepted
Headers show
Series
  • [committed,v2,1/2] elf: Accept absolute (SHN_ABS) symbols whose value is zero [BZ #23307]
Related show

Commit Message

Maciej W. Rozycki June 29, 2018, 4:13 p.m. UTC
We have this condition in `check_match' (in elf/dl-lookup.c):

  if (__glibc_unlikely ((sym->st_value == 0 /* No value.  */
                         && stt != STT_TLS)
                        || ELF_MACHINE_SYM_NO_MATCH (sym)
                        || (type_class & (sym->st_shndx == SHN_UNDEF))))
    return NULL;

which causes all !STT_TLS symbols whose value is zero to be silently 
ignored in lookup.  This may make sense for regular symbols, however not 
for absolute (SHN_ABS) ones, where zero is like any value, there's no 
special meaning attached to it.

Consequently legitimate programs fail, for example taking the 
`elf/tst-absolute-sym' test case, substituting 0 for 0x55aa in 
`elf/tst-absolute-sym-lib.lds' and then trying to run the resulting 
program we get this:

$ .../elf/tst-absolute-sym
.../elf/tst-absolute-sym: symbol lookup error: .../elf/tst-absolute-sym-lib.so: undefined symbol: absolute
$ 

even though the symbol clearly is there:

$ readelf --dyn-syms .../elf/tst-absolute-sym-lib.so | grep '\babsolute\b'
     7: 00000000     0 NOTYPE  GLOBAL DEFAULT  ABS absolute
$ 

The check for the zero value has been there since forever or commit 
d66e34cd4234/08162fa88891 ("Implemented runtime dynamic linker to 
support ELF shared libraries.") dating back to May 2nd 1995, and the 
problem triggers regardless of commit e7feec374c63 ("elf: Correct 
absolute (SHN_ABS) symbol run-time calculation [BZ #19818]") being 
present or not.

Fix the issue then, by permitting `sym->st_value' to be 0 for SHN_ABS 
symbols in lookup.

	[BZ #23307]
	* elf/dl-lookup.c (check_match): Do not reject a symbol whose
	`st_value' is 0 if `st_shndx' is SHN_ABS.
	* elf/tst-absolute-zero.c: New file.
	* elf/tst-absolute-zero-lib.c: New file.
	* elf/tst-absolute-zero-lib.lds: New file.
	* elf/Makefile (tests): Add `tst-absolute-zero'.
	(modules-names): Add `tst-absolute-zero-lib'.
	(LDLIBS-tst-absolute-zero-lib.so): New variable.
	($(objpfx)tst-absolute-zero-lib.so): New dependency.
	($(objpfx)tst-absolute-zero: New dependency.
---
Hi Florian,

> > Index: glibc/elf/tst-absolute-zero-lib.c
> > ===================================================================
> > --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> > +++ glibc/elf/tst-absolute-zero-lib.c	2018-06-18 18:12:40.102324540 +0100
> > @@ -0,0 +1,25 @@
> > +/* BZ #xxxxx absolute zero symbol calculation shared module.
> 
> Needs Bugzilla number.
> 
> > Index: glibc/elf/tst-absolute-zero.c
> > ===================================================================
> > --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> > +++ glibc/elf/tst-absolute-zero.c	2018-06-18 18:12:40.147611912 +0100
> > @@ -0,0 +1,38 @@
> > +/* BZ #xxxxx absolute zero symbol calculation main executable.
> 
> Likewise.

 Thank you for your review.  I have applied this change now, with these 
issues fixed.  I'm keeping BZ #23307 open for the outcome with 2/2.

  Maciej
---
 elf/Makefile                  |    8 ++++++--
 elf/dl-lookup.c               |    1 +
 elf/tst-absolute-zero-lib.c   |   25 +++++++++++++++++++++++++
 elf/tst-absolute-zero-lib.lds |    1 +
 elf/tst-absolute-zero.c       |   38 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 71 insertions(+), 2 deletions(-)

glibc-elf-shn-abs-zero.diff

Comments

Joseph Myers June 29, 2018, 5:41 p.m. UTC | #1
On Fri, 29 Jun 2018, Maciej W. Rozycki wrote:

> issues fixed.  I'm keeping BZ #23307 open for the outcome with 2/2.

That's not appropriate.  If a bug is resolved (which this is), it needs to 
be marked RESOLVED/FIXED with target milestone set, so that the proper 
list of fixed bugs is generated for 2.28 even if there is no further 
progress on the question of ABI marking.

This does not rule out having a *new* bug opened for the ABI marking 
question, but the original bug should be marked as fixed to make sure we 
get a proper list of fixed bugs in NEWS for 2.28.
Maciej W. Rozycki June 29, 2018, 6:05 p.m. UTC | #2
On Fri, 29 Jun 2018, Joseph Myers wrote:

> > issues fixed.  I'm keeping BZ #23307 open for the outcome with 2/2.
> 
> That's not appropriate.  If a bug is resolved (which this is), it needs to 
> be marked RESOLVED/FIXED with target milestone set, so that the proper 
> list of fixed bugs is generated for 2.28 even if there is no further 
> progress on the question of ABI marking.

 OK, done.

  Maciej

Patch
diff mbox series

Index: glibc/elf/Makefile
===================================================================
--- glibc.orig/elf/Makefile	2018-06-18 18:12:19.911942887 +0100
+++ glibc/elf/Makefile	2018-06-18 18:12:40.092259983 +0100
@@ -186,7 +186,7 @@  tests += restest1 preloadtest loadfail m
 	 tst-tlsalign tst-tlsalign-extern tst-nodelete-opened \
 	 tst-nodelete2 tst-audit11 tst-audit12 tst-dlsym-error tst-noload \
 	 tst-latepthread tst-tls-manydynamic tst-nodelete-dlclose \
-	 tst-debug1 tst-main1 tst-absolute-sym tst-big-note
+	 tst-debug1 tst-main1 tst-absolute-sym tst-absolute-zero tst-big-note
 #	 reldep9
 tests-internal += loadtest unload unload2 circleload1 \
 	 neededtest neededtest2 neededtest3 neededtest4 \
@@ -273,7 +273,7 @@  modules-names = testobj1 testobj2 testob
 		tst-latepthreadmod $(tst-tls-many-dynamic-modules) \
 		tst-nodelete-dlclose-dso tst-nodelete-dlclose-plugin \
 		tst-main1mod tst-libc_dlvsym-dso tst-absolute-sym-lib \
-		tst-big-note-lib
+		tst-absolute-zero-lib tst-big-note-lib
 
 ifeq (yes,$(have-mtls-dialect-gnu2))
 tests += tst-gnu2-tls1
@@ -1456,6 +1456,10 @@  LDLIBS-tst-absolute-sym-lib.so = tst-abs
 $(objpfx)tst-absolute-sym-lib.so: $(LDLIBS-tst-absolute-sym-lib.so)
 $(objpfx)tst-absolute-sym: $(objpfx)tst-absolute-sym-lib.so
 
+LDLIBS-tst-absolute-zero-lib.so = tst-absolute-zero-lib.lds
+$(objpfx)tst-absolute-zero-lib.so: $(LDLIBS-tst-absolute-zero-lib.so)
+$(objpfx)tst-absolute-zero: $(objpfx)tst-absolute-zero-lib.so
+
 # Both the main program and the DSO for tst-libc_dlvsym need to link
 # against libdl.
 $(objpfx)tst-libc_dlvsym: $(libdl)
Index: glibc/elf/dl-lookup.c
===================================================================
--- glibc.orig/elf/dl-lookup.c	2018-06-17 09:06:30.202407070 +0100
+++ glibc/elf/dl-lookup.c	2018-06-18 18:12:36.138114433 +0100
@@ -76,6 +76,7 @@  check_match (const char *const undef_nam
   unsigned int stt = ELFW(ST_TYPE) (sym->st_info);
   assert (ELF_RTYPE_CLASS_PLT == 1);
   if (__glibc_unlikely ((sym->st_value == 0 /* No value.  */
+			 && sym->st_shndx != SHN_ABS
 			 && stt != STT_TLS)
 			|| ELF_MACHINE_SYM_NO_MATCH (sym)
 			|| (type_class & (sym->st_shndx == SHN_UNDEF))))
Index: glibc/elf/tst-absolute-zero-lib.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ glibc/elf/tst-absolute-zero-lib.c	2018-06-29 16:49:42.817333198 +0100
@@ -0,0 +1,25 @@ 
+/* BZ #23307 absolute zero symbol calculation shared module.
+   Copyright (C) 2018 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+extern char absolute;
+
+void *
+get_absolute (void)
+{
+  return &absolute;
+}
Index: glibc/elf/tst-absolute-zero-lib.lds
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ glibc/elf/tst-absolute-zero-lib.lds	2018-06-18 18:12:40.129495568 +0100
@@ -0,0 +1 @@ 
+"absolute" = 0;
Index: glibc/elf/tst-absolute-zero.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ glibc/elf/tst-absolute-zero.c	2018-06-29 16:49:50.893494243 +0100
@@ -0,0 +1,38 @@ 
+/* BZ #23307 absolute zero symbol calculation main executable.
+   Copyright (C) 2018 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <support/check.h>
+#include <support/support.h>
+#include <support/test-driver.h>
+
+void *get_absolute (void);
+
+static int
+do_test (void)
+{
+  void *ref = (void *) 0;
+  void *ptr;
+
+  ptr = get_absolute ();
+  if (ptr != ref)
+    FAIL_EXIT1 ("Got %p, expected %p\n", ptr, ref);
+
+  return 0;
+}
+
+#include <support/test-driver.c>