[committed,v2,1/2] elf: Accept absolute (SHN_ABS) symbols whose value is zero [BZ #23307]
Commit Message
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
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.
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
===================================================================
@@ -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)
===================================================================
@@ -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))))
===================================================================
@@ -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;
+}
===================================================================
@@ -0,0 +1 @@
+"absolute" = 0;
===================================================================
@@ -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>