elf: Fix handling of symbol versions which hash to zero (bug 29190)
Checks
Context |
Check |
Description |
redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
success
|
Build passed
|
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
success
|
Test passed
|
linaro-tcwg-bot/tcwg_glibc_build--master-arm |
success
|
Build passed
|
linaro-tcwg-bot/tcwg_glibc_check--master-arm |
fail
|
Patch failed to apply
|
redhat-pt-bot/TryBot-32bit |
success
|
Build for i686
|
Commit Message
This was found through code inspection. No application impact is
known.
Tested on i686-linux-gnu and x86_64-linux.gnu.
Joseph, do we need to adjust the hash collision tests after this fix
goes in?
Thanks,
Florian
---
elf/Makefile | 26 +++++++++++++++++
elf/dl-lookup.c | 22 ++++++++++----
elf/dl-version.c | 7 +++++
elf/tst-version-hash-zero-linkmod.c | 22 ++++++++++++++
elf/tst-version-hash-zero-linkmod.map | 7 +++++
elf/tst-version-hash-zero-mod.c | 20 +++++++++++++
elf/tst-version-hash-zero-mod.map | 13 +++++++++
elf/tst-version-hash-zero-refmod.c | 23 +++++++++++++++
elf/tst-version-hash-zero.c | 55 +++++++++++++++++++++++++++++++++++
9 files changed, 189 insertions(+), 6 deletions(-)
base-commit: a402cae36d95a2141703df324b5de5b581868c5c
Comments
On Thu, 16 Jan 2025, Florian Weimer wrote:
> This was found through code inspection. No application impact is
> known.
>
> Tested on i686-linux-gnu and x86_64-linux.gnu.
>
> Joseph, do we need to adjust the hash collision tests after this fix
> goes in?
The tests should work fine as-is (they avoid testing symbol versions that
hash to 0 because of this issue), but it would also be reasonable to
expand tst-hash-collision3 to include symbol versions that hash to 0,
given this fix.
Am Donnerstag, 16. Januar 2025, 11:58:10 Mitteleuropäische Normalzeit schrieb Florian Weimer:
> This was found through code inspection. No application impact is
> known.
>
> Tested on i686-linux-gnu and x86_64-linux.gnu.
>
I guess it's time to re-visit this now for 2.42 ?
* Andreas K. Huettel:
> Am Donnerstag, 16. Januar 2025, 11:58:10 Mitteleuropäische Normalzeit schrieb Florian Weimer:
>> This was found through code inspection. No application impact is
>> known.
>>
>> Tested on i686-linux-gnu and x86_64-linux.gnu.
>>
>
> I guess it's time to re-visit this now for 2.42 ?
Indeed. Adhemerval marked it as Accepted in Patchwork, I think, but
I don't think it's been reviewed.
Thanks,
Florian
On 10/02/25 14:30, Florian Weimer wrote:
> * Andreas K. Huettel:
>
>> Am Donnerstag, 16. Januar 2025, 11:58:10 Mitteleuropäische Normalzeit schrieb Florian Weimer:
>>> This was found through code inspection. No application impact is
>>> known.
>>>
>>> Tested on i686-linux-gnu and x86_64-linux.gnu.
>>>
>>
>> I guess it's time to re-visit this now for 2.42 ?
>
> Indeed. Adhemerval marked it as Accepted in Patchwork, I think, but
> I don't think it's been reviewed.
I think I have marked it wrongly as accepted, I will try to review this.
@@ -497,6 +497,7 @@ tests += \
tst-unique2 \
tst-unwind-ctor \
tst-unwind-main \
+ tst-version-hash-zero \
unload3 \
unload4 \
unload5 \
@@ -1033,6 +1034,9 @@ modules-names += \
tst-unique2mod1 \
tst-unique2mod2 \
tst-unwind-ctor-lib \
+ tst-version-hash-zero-linkmod \
+ tst-version-hash-zero-mod \
+ tst-version-hash-zero-refmod \
unload2dep \
unload2mod \
unload3mod1 \
@@ -3392,3 +3396,25 @@ $(objpfx)tst-nolink-libc-2: $(objpfx)tst-nolink-libc.o
-Wl,--dynamic-linker=$(objpfx)ld.so
$(objpfx)tst-nolink-libc-2.out: $(objpfx)tst-nolink-libc-2 $(objpfx)ld.so
$< > $@ 2>&1; $(evaluate-test)
+
+$(objpfx)tst-version-hash-zero.out: \
+ $(objpfx)tst-version-hash-zero-mod.so \
+ $(objpfx)tst-version-hash-zero-refmod.so
+$(objpfx)tst-version-hash-zero-mod.so: $(objpfx)tst-version-hash-zero-mod.os \
+ tst-version-hash-zero-mod.map
+ $(LINK.o) -shared -o $@ $(LDFLAGS.so) $< \
+ -Wl,--version-script=tst-version-hash-zero-mod.map
+# The run-time test module tst-version-hash-zero-refmod.so is linked
+# to a stub module, tst-version-hash-zero-linkmod.so, to produce an
+# expected relocation error.
+$(objpfx)tst-version-hash-zero-refmod.so: \
+ $(objpfx)tst-version-hash-zero-linkmod.so
+$(objpfx)tst-version-hash-zero-linkmod.so:\
+ $(objpfx)tst-version-hash-zero-linkmod.os \
+ tst-version-hash-zero-linkmod.map
+ $(LINK.o) -shared -o $@ $(LDFLAGS.so) $< \
+ -Wl,--version-script=tst-version-hash-zero-linkmod.map \
+ -Wl,--soname=tst-version-hash-zero-mod.so
+$(objpfx)tst-version-hash-zero-refmod.so: \
+ $(objpfx)tst-version-hash-zero-linkmod.so
+tst-version-hash-zero-refmod.so-no-z-defs = yes
@@ -100,12 +100,22 @@ check_match (const char *const undef_name,
/* We can match the version information or use the
default one if it is not hidden. */
ElfW(Half) ndx = verstab[symidx] & 0x7fff;
- if ((map->l_versions[ndx].hash != version->hash
- || strcmp (map->l_versions[ndx].name, version->name))
- && (version->hidden || map->l_versions[ndx].hash
- || (verstab[symidx] & 0x8000)))
- /* It's not the version we want. */
- return NULL;
+ if (map->l_versions[ndx].hash == version->hash
+ && strcmp (map->l_versions[ndx].name, version->name) == 0)
+ /* This is an exact version match. Return the symbol below. */
+ ;
+ else
+ {
+ if (!version->hidden
+ && map->l_versions[ndx].name[0] == '\0'
+ && (verstab[symidx] & 0x8000) == 0
+ && (*num_versions)++ == 0)
+ /* This is the global default version. Store it as a
+ fallback match. */
+ *versioned_sym = sym;
+
+ return NULL;
+ }
}
}
else
@@ -357,6 +357,13 @@ _dl_check_map_versions (struct link_map *map, int verbose, int trace_mode)
ent = (ElfW(Verdef) *) ((char *) ent + ent->vd_next);
}
}
+
+ /* The empty string has ELF hash zero. This avoids a NULL check
+ before the version string comparison in check_match in
+ dl-lookup.c. */
+ for (unsigned int i = 0; i < map->l_nversions; ++i)
+ if (map->l_versions[i].name == NULL)
+ map->l_versions[i].name = "";
}
/* When there is a DT_VERNEED entry with libc.so on DT_NEEDED, issue
new file mode 100644
@@ -0,0 +1,22 @@
+/* Stub module for linking tst-version-hash-zero-refmod.so.
+ Copyright (C) 2025 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; see the file COPYING.LIB. If
+ not, see <https://www.gnu.org/licenses/>. */
+
+/* The version script assigns a different symbol version for the stub
+ module. Loading the module with the incorrect version is expected
+ to fail. */
+#include "tst-version-hash-zero-mod.c"
new file mode 100644
@@ -0,0 +1,7 @@
+Base {
+ local: *;
+};
+
+OTHER_VERSION {
+ global: global_variable;
+} Base;
new file mode 100644
@@ -0,0 +1,20 @@
+/* Test module with a zero version symbol hash.
+ Copyright (C) 2025 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; see the file COPYING.LIB. If
+ not, see <https://www.gnu.org/licenses/>. */
+
+/* The symbol version is assigned by version script. */
+int global_variable;
new file mode 100644
@@ -0,0 +1,13 @@
+Base {
+ local: *;
+};
+
+/* Define the version so that tst-version-hash-zero-refmod.so passes
+ the initial symbol version check. */
+OTHER_VERSION {
+} Base;
+
+/* This version string hashes to zero. */
+PPPPPPPPPPPP {
+ global: global_variable;
+} Base;
new file mode 100644
@@ -0,0 +1,23 @@
+/* Test module that triggers a relocation failure in tst-version-hash-zero.
+ Copyright (C) 2025 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; see the file COPYING.LIB. If
+ not, see <https://www.gnu.org/licenses/>. */
+
+/* This is bound to global_variable@@OTHER_VERSION via
+ tst-version-hash-zero-linkmod.so, but at run time, only
+ global_variable@PPPPPPPPPPPP exists. */
+extern int global_variable;
+int *pointer_variable = &global_variable;
new file mode 100644
@@ -0,0 +1,55 @@
+/* Symbols with version hash zero should not match any version (bug 29190).
+ Copyright (C) 2025 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; see the file COPYING.LIB. If
+ not, see <https://www.gnu.org/licenses/>. */
+
+#include <support/check.h>
+#include <support/xdlfcn.h>
+#include <stddef.h>
+#include <string.h>
+
+static int
+do_test (void)
+{
+ void *handle = xdlopen ("tst-version-hash-zero-mod.so", RTLD_NOW);
+
+ /* This used to crash because some struct r_found_version entries
+ with hash zero did not have valid version strings. */
+ TEST_VERIFY (xdlvsym (handle, "global_variable", "PPPPPPPPPPPP") != NULL);
+
+ /* Consistency check. */
+ TEST_VERIFY (xdlsym (handle, "global_variable")
+ == xdlvsym (handle, "global_variable", "PPPPPPPPPPPP"));
+
+ /* This symbol version is supposed to be missing. */
+ TEST_VERIFY (dlvsym (handle, "global_variable", "OTHER_VERSION") == NULL);
+
+ /* tst-version-hash-zero-refmod.so references
+ global_variable@@OTHER_VERSION and is expected to fail to load.
+ dlvsym sets the hidden flag during lookup. Relocation does not,
+ so this exercises a different failure case. */
+ TEST_VERIFY (dlopen ("tst-version-hash-zero-refmod.so", RTLD_NOW) == NULL);
+ const char *message = dlerror ();
+ if (strstr (message,
+ ": undefined symbol: global_variable, version OTHER_VERSION")
+ == NULL)
+ FAIL_EXIT1 ("unexpected dlopen failure: %s", message);
+
+ xdlclose (handle);
+ return 0;
+}
+
+#include <support/test-driver.c>