Summary: | Incomplete rollback of dynamic linker state on linking failure | ||
---|---|---|---|
Product: | glibc | Reporter: | Florian Weimer <fweimer> |
Component: | dynamic-link | Assignee: | Florian Weimer <fweimer> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | carlos, elvstone, github, lion, woodard, zibeon |
Priority: | P2 | Flags: | fweimer:
security-
|
Version: | unspecified | ||
Target Milestone: | 2.31 | ||
See Also: |
https://bugzilla.redhat.com/show_bug.cgi?id=1395758 https://bugzilla.redhat.com/show_bug.cgi?id=1500128 https://sourceware.org/bugzilla/show_bug.cgi?id=24259 https://bugzilla.redhat.com/show_bug.cgi?id=1624941 https://bugzilla.redhat.com/show_bug.cgi?id=1410154 |
||
Host: | Target: | ||
Build: | Last reconfirmed: | ||
Attachments: |
a fairly simple reproducer
Honor NODELETE only after relocation |
Description
Florian Weimer
2016-11-18 13:16:20 UTC
*** Bug 22280 has been marked as a duplicate of this bug. *** Created attachment 10522 [details]
a fairly simple reproducer
As a case in point here is a reproducer that I whipped up based upon a customer report.
[ben@Mustang dl-bug]$ make all
cc -g -c -o main.o main.c
cc -g -o main main.o -ldl
cc -g -c -fpic a.c
cc -g -c -fpic d.c
cc -g -fpic -shared -Wl,-z,nodelete -o libd.so d.o
cc -g -c -fpic e.c
cc -g -fpic -shared -o libe.so e.o
cc -g -fpic -shared -o liba.so a.o -L. -ld -le
cc -g -c -fpic b.c
cc -g -fpic -shared -o libb.so b.o -L. -ld
[ben@Mustang dl-bug]$ make run
LD_LIBRARY_PATH=. ./main
d_fn x=12
inside b_fn
rm libe.so
LD_LIBRARY_PATH=. ./main
Could not open liba.so - libe.so: cannot open shared object file: No such file or directory
make: *** [Makefile:38: run] Segmentation fault (core dumped)
Note that libd.so is marked NODELETE
So when main dlopen's liba.so which needs on libd.so and libe.so because libe.so is missing, the load of liba.so fails. This is expected. However, when libb.so is loaded which also needs libd.so the application crashes because the relocations haven't been done.
[ben@Mustang dl-bug]$ LD_LIBRARY_PATH=. LD_DEBUG=reloc,files ./main 2> foo
d_fn x=12
inside b_fn
[ben@Mustang dl-bug]$ egrep file\|reloc foo
10901: file=libdl.so.2 [0]; needed by ./main [0]
10901: file=libdl.so.2 [0]; generating link map
10901: file=libc.so.6 [0]; needed by ./main [0]
10901: file=libc.so.6 [0]; generating link map
10901: relocation processing: /lib64/libc.so.6
10901: relocation processing: /lib64/libdl.so.2
10901: relocation processing: ./main (lazy)
10901: relocation processing: /lib64/ld-linux-x86-64.so.2
10901: file=liba.so [0]; dynamically loaded by ./main [0]
10901: file=liba.so [0]; generating link map
10901: file=libd.so [0]; needed by ./liba.so [0]
10901: file=libd.so [0]; generating link map
10901: file=libe.so [0]; needed by ./liba.so [0]
10901: file=libe.so [0]; generating link map
10901: relocation processing: ./libe.so
10901: relocation processing: ./libd.so
10901: relocation processing: ./liba.so
10901: opening file=./liba.so [0]; direct_opencount=1
10901: file=libb.so [0]; dynamically loaded by ./main [0]
10901: file=libb.so [0]; generating link map
10901: relocation processing: ./libb.so
10901: opening file=./libb.so [0]; direct_opencount=1
vs.
[ben@Mustang dl-bug]$ rm libe.so
[ben@Mustang dl-bug]$ LD_LIBRARY_PATH=. LD_DEBUG=reloc,files ./main 2> foo
Segmentation fault (core dumped)
[ben@Mustang dl-bug]$ egrep file\|reloc foo
10965: file=libdl.so.2 [0]; needed by ./main [0]
10965: file=libdl.so.2 [0]; generating link map
10965: file=libc.so.6 [0]; needed by ./main [0]
10965: file=libc.so.6 [0]; generating link map
10965: relocation processing: /lib64/libc.so.6
10965: relocation processing: /lib64/libdl.so.2
10965: relocation processing: ./main (lazy)
10965: relocation processing: /lib64/ld-linux-x86-64.so.2
10965: file=liba.so [0]; dynamically loaded by ./main [0]
10965: file=liba.so [0]; generating link map
10965: file=libd.so [0]; needed by ./liba.so [0]
10965: file=libd.so [0]; generating link map
10965: file=libe.so [0]; needed by ./liba.so [0]
10965: file=./liba.so [0]; destroying link map
Could not open liba.so - libe.so: cannot open shared object file: No such file or directory
10965: file=libb.so [0]; dynamically loaded by ./main [0]
10965: file=libb.so [0]; generating link map
10965: relocation processing: ./libb.so
Note on the failing case the relocations are never done on libd.so
It is my opinion that the NODELETE flag should not be honored at this early stage. The reason for the NODELETE flag is that the library may have side effects that are irreversible. However, because the library has not been relocated, it cannot even have had its constructor run. Therefore, its ability to cause irreversible side effects are practically nil. Therefore it is safe to remove it as if the NODELETE flag had not been set. Earlier testing with my reproducer demonstrated that without the -Wl,nodelete command line option, the problem does not manifest. Therefore, we have a case where we do not have a NODELETE flag that works correctly and we have a case where we do honor the NODELETE flag which crashes. Therefore it seems to make sense that the NODELETE flag only takes effect after the relocations have been done. Or maybe only after the library's constructor has been run. It is only then, that the library could have made a change which that would not permit it from being deleted. *** Bug 23810 has been marked as a duplicate of this bug. *** Created attachment 11463 [details]
Honor NODELETE only after relocation
At our side, this bug manifest itself as a "libgcc_s.so.1 must be installed for pthread_cancel to work" message.
I agree with Ben that we shuold not honor NODELETE too early. The attached patch makes our production use-case work. I might have time to add a test case too in the following days.
The master branch has been updated by H.J. Lu <hjl@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=d0093c5cefb7f7a4143f3bb03743633823229cc6 commit d0093c5cefb7f7a4143f3bb03743633823229cc6 Author: H.J. Lu <hjl.tools@gmail.com> Date: Mon Jul 1 12:23:10 2019 -0700 Call _dl_open_check after relocation [BZ #24259] This is a workaround for [BZ #20839] which doesn't remove the NODELETE object when _dl_open_check throws an exception. Move it after relocation in dl_open_worker to avoid leaving the NODELETE object mapped without relocation. [BZ #24259] * elf/dl-open.c (dl_open_worker): Call _dl_open_check after relocation. * sysdeps/x86/Makefile (tests): Add tst-cet-legacy-5a, tst-cet-legacy-5b, tst-cet-legacy-6a and tst-cet-legacy-6b. (modules-names): Add tst-cet-legacy-mod-5a, tst-cet-legacy-mod-5b, tst-cet-legacy-mod-5c, tst-cet-legacy-mod-6a, tst-cet-legacy-mod-6b and tst-cet-legacy-mod-6c. (CFLAGS-tst-cet-legacy-5a.c): New. (CFLAGS-tst-cet-legacy-5b.c): Likewise. (CFLAGS-tst-cet-legacy-mod-5a.c): Likewise. (CFLAGS-tst-cet-legacy-mod-5b.c): Likewise. (CFLAGS-tst-cet-legacy-mod-5c.c): Likewise. (CFLAGS-tst-cet-legacy-6a.c): Likewise. (CFLAGS-tst-cet-legacy-6b.c): Likewise. (CFLAGS-tst-cet-legacy-mod-6a.c): Likewise. (CFLAGS-tst-cet-legacy-mod-6b.c): Likewise. (CFLAGS-tst-cet-legacy-mod-6c.c): Likewise. ($(objpfx)tst-cet-legacy-5a): Likewise. ($(objpfx)tst-cet-legacy-5a.out): Likewise. ($(objpfx)tst-cet-legacy-mod-5a.so): Likewise. ($(objpfx)tst-cet-legacy-mod-5b.so): Likewise. ($(objpfx)tst-cet-legacy-5b): Likewise. ($(objpfx)tst-cet-legacy-5b.out): Likewise. (tst-cet-legacy-5b-ENV): Likewise. ($(objpfx)tst-cet-legacy-6a): Likewise. ($(objpfx)tst-cet-legacy-6a.out): Likewise. ($(objpfx)tst-cet-legacy-mod-6a.so): Likewise. ($(objpfx)tst-cet-legacy-mod-6b.so): Likewise. ($(objpfx)tst-cet-legacy-6b): Likewise. ($(objpfx)tst-cet-legacy-6b.out): Likewise. (tst-cet-legacy-6b-ENV): Likewise. * sysdeps/x86/tst-cet-legacy-5.c: New file. * sysdeps/x86/tst-cet-legacy-5a.c: Likewise. * sysdeps/x86/tst-cet-legacy-5b.c: Likewise. * sysdeps/x86/tst-cet-legacy-6.c: Likewise. * sysdeps/x86/tst-cet-legacy-6a.c: Likewise. * sysdeps/x86/tst-cet-legacy-6b.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-5.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-5a.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-5b.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-5c.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-6.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-6a.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-6b.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-6c.c: Likewise. I think I have a patch for this. Patch posted: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/471 The release/2.28/master branch has been updated by DJ Delorie <dj@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=0d3905b110000463775b3fb189213833acaebf81 commit 0d3905b110000463775b3fb189213833acaebf81 Author: H.J. Lu <hjl.tools@gmail.com> Date: Mon Jul 1 12:23:10 2019 -0700 Call _dl_open_check after relocation [BZ #24259] This is a workaround for [BZ #20839] which doesn't remove the NODELETE object when _dl_open_check throws an exception. Move it after relocation in dl_open_worker to avoid leaving the NODELETE object mapped without relocation. [BZ #24259] * elf/dl-open.c (dl_open_worker): Call _dl_open_check after relocation. * sysdeps/x86/Makefile (tests): Add tst-cet-legacy-5a, tst-cet-legacy-5b, tst-cet-legacy-6a and tst-cet-legacy-6b. (modules-names): Add tst-cet-legacy-mod-5a, tst-cet-legacy-mod-5b, tst-cet-legacy-mod-5c, tst-cet-legacy-mod-6a, tst-cet-legacy-mod-6b and tst-cet-legacy-mod-6c. (CFLAGS-tst-cet-legacy-5a.c): New. (CFLAGS-tst-cet-legacy-5b.c): Likewise. (CFLAGS-tst-cet-legacy-mod-5a.c): Likewise. (CFLAGS-tst-cet-legacy-mod-5b.c): Likewise. (CFLAGS-tst-cet-legacy-mod-5c.c): Likewise. (CFLAGS-tst-cet-legacy-6a.c): Likewise. (CFLAGS-tst-cet-legacy-6b.c): Likewise. (CFLAGS-tst-cet-legacy-mod-6a.c): Likewise. (CFLAGS-tst-cet-legacy-mod-6b.c): Likewise. (CFLAGS-tst-cet-legacy-mod-6c.c): Likewise. ($(objpfx)tst-cet-legacy-5a): Likewise. ($(objpfx)tst-cet-legacy-5a.out): Likewise. ($(objpfx)tst-cet-legacy-mod-5a.so): Likewise. ($(objpfx)tst-cet-legacy-mod-5b.so): Likewise. ($(objpfx)tst-cet-legacy-5b): Likewise. ($(objpfx)tst-cet-legacy-5b.out): Likewise. (tst-cet-legacy-5b-ENV): Likewise. ($(objpfx)tst-cet-legacy-6a): Likewise. ($(objpfx)tst-cet-legacy-6a.out): Likewise. ($(objpfx)tst-cet-legacy-mod-6a.so): Likewise. ($(objpfx)tst-cet-legacy-mod-6b.so): Likewise. ($(objpfx)tst-cet-legacy-6b): Likewise. ($(objpfx)tst-cet-legacy-6b.out): Likewise. (tst-cet-legacy-6b-ENV): Likewise. * sysdeps/x86/tst-cet-legacy-5.c: New file. * sysdeps/x86/tst-cet-legacy-5a.c: Likewise. * sysdeps/x86/tst-cet-legacy-5b.c: Likewise. * sysdeps/x86/tst-cet-legacy-6.c: Likewise. * sysdeps/x86/tst-cet-legacy-6a.c: Likewise. * sysdeps/x86/tst-cet-legacy-6b.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-5.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-5a.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-5b.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-5c.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-6.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-6a.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-6b.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-6c.c: Likewise. (cherry picked from commit d0093c5cefb7f7a4143f3bb03743633823229cc6) The release/2.29/master branch has been updated by DJ Delorie <dj@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=5e1548a6d9c5c544f13cb71f4462b3f38e87a3c6 commit 5e1548a6d9c5c544f13cb71f4462b3f38e87a3c6 Author: H.J. Lu <hjl.tools@gmail.com> Date: Mon Jul 1 12:23:10 2019 -0700 Call _dl_open_check after relocation [BZ #24259] This is a workaround for [BZ #20839] which doesn't remove the NODELETE object when _dl_open_check throws an exception. Move it after relocation in dl_open_worker to avoid leaving the NODELETE object mapped without relocation. [BZ #24259] * elf/dl-open.c (dl_open_worker): Call _dl_open_check after relocation. * sysdeps/x86/Makefile (tests): Add tst-cet-legacy-5a, tst-cet-legacy-5b, tst-cet-legacy-6a and tst-cet-legacy-6b. (modules-names): Add tst-cet-legacy-mod-5a, tst-cet-legacy-mod-5b, tst-cet-legacy-mod-5c, tst-cet-legacy-mod-6a, tst-cet-legacy-mod-6b and tst-cet-legacy-mod-6c. (CFLAGS-tst-cet-legacy-5a.c): New. (CFLAGS-tst-cet-legacy-5b.c): Likewise. (CFLAGS-tst-cet-legacy-mod-5a.c): Likewise. (CFLAGS-tst-cet-legacy-mod-5b.c): Likewise. (CFLAGS-tst-cet-legacy-mod-5c.c): Likewise. (CFLAGS-tst-cet-legacy-6a.c): Likewise. (CFLAGS-tst-cet-legacy-6b.c): Likewise. (CFLAGS-tst-cet-legacy-mod-6a.c): Likewise. (CFLAGS-tst-cet-legacy-mod-6b.c): Likewise. (CFLAGS-tst-cet-legacy-mod-6c.c): Likewise. ($(objpfx)tst-cet-legacy-5a): Likewise. ($(objpfx)tst-cet-legacy-5a.out): Likewise. ($(objpfx)tst-cet-legacy-mod-5a.so): Likewise. ($(objpfx)tst-cet-legacy-mod-5b.so): Likewise. ($(objpfx)tst-cet-legacy-5b): Likewise. ($(objpfx)tst-cet-legacy-5b.out): Likewise. (tst-cet-legacy-5b-ENV): Likewise. ($(objpfx)tst-cet-legacy-6a): Likewise. ($(objpfx)tst-cet-legacy-6a.out): Likewise. ($(objpfx)tst-cet-legacy-mod-6a.so): Likewise. ($(objpfx)tst-cet-legacy-mod-6b.so): Likewise. ($(objpfx)tst-cet-legacy-6b): Likewise. ($(objpfx)tst-cet-legacy-6b.out): Likewise. (tst-cet-legacy-6b-ENV): Likewise. * sysdeps/x86/tst-cet-legacy-5.c: New file. * sysdeps/x86/tst-cet-legacy-5a.c: Likewise. * sysdeps/x86/tst-cet-legacy-5b.c: Likewise. * sysdeps/x86/tst-cet-legacy-6.c: Likewise. * sysdeps/x86/tst-cet-legacy-6a.c: Likewise. * sysdeps/x86/tst-cet-legacy-6b.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-5.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-5a.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-5b.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-5c.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-6.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-6a.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-6b.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-6c.c: Likewise. (cherry picked from commit d0093c5cefb7f7a4143f3bb03743633823229cc6) The master branch has been updated by Florian Weimer <fw@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=f63b73814f74032c0e5d0a83300e3d864ef905e5 commit f63b73814f74032c0e5d0a83300e3d864ef905e5 Author: Florian Weimer <fweimer@redhat.com> Date: Wed Nov 13 15:44:56 2019 +0100 Remove all loaded objects if dlopen fails, ignoring NODELETE [BZ #20839] This introduces a “pending NODELETE” state in the link map, which is flipped to the persistent NODELETE state late in dlopen, via activate_nodelete. During initial relocation, symbol binding records pending NODELETE state only. dlclose ignores pending NODELETE state. Taken together, this results that a partially completed dlopen is rolled back completely because new NODELETE mappings are unloaded. Tested on x86_64-linux-gnu and i386-linux-gnu. Change-Id: Ib2a3d86af6f92d75baca65431d74783ee0dbc292 The master branch has been updated by Florian Weimer <fw@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=e37c2cf299b61ce18f62852f6c5624c27829b610 commit e37c2cf299b61ce18f62852f6c5624c27829b610 Author: Florian Weimer <fweimer@redhat.com> Date: Thu Oct 31 18:48:43 2019 +0100 Move _dl_open_check to its original place in dl_open_worker This reverts the non-test change from commit d0093c5cefb7f7a4143f ("Call _dl_open_check after relocation [BZ #24259]"), given that the underlying bug has been fixed properly in commit 61b74477fa7f63 ("Remove all loaded objects if dlopen fails, ignoring NODELETE [BZ #20839]"). Tested on x86-64-linux-gnu, with and without --enable-cet. Change-Id: I995a6cfb89f25d2b0cf5e606428c2a93eb48fc33 Fixed for glibc 2.31. > commit f63b73814f74032c0e5d0a83300e3d864ef905e5
> Author: Florian Weimer <fweimer@redhat.com>
> Date: Wed Nov 13 15:44:56 2019 +0100
>
> Remove all loaded objects if dlopen fails, ignoring NODELETE [BZ #20839]
This has caused breakage in qutebrowser. When attempting to start it the
following error is returned:
Inconsistency detected by ld.so: dl-open.c: 459: activate_nodelete: Assertion `!imap->l_init_called || imap->l_type != lt_loaded'
(In reply to zibeon from comment #14) > > commit f63b73814f74032c0e5d0a83300e3d864ef905e5 > > Author: Florian Weimer <fweimer@redhat.com> > > Date: Wed Nov 13 15:44:56 2019 +0100 > > > > Remove all loaded objects if dlopen fails, ignoring NODELETE [BZ #20839] > > This has caused breakage in qutebrowser. When attempting to start it the > following error is returned: > > Inconsistency detected by ld.so: dl-open.c: 459: activate_nodelete: > Assertion `!imap->l_init_called || imap->l_type != lt_loaded' The fix is being reviewed here: https://www.sourceware.org/ml/libc-alpha/2019-12/msg00098.html The master branch has been updated by Florian Weimer <fw@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=365624e2d2a342cdb693b4cc35d2312169959e28 commit 365624e2d2a342cdb693b4cc35d2312169959e28 Author: Florian Weimer <fweimer@redhat.com> Date: Fri Dec 13 10:18:24 2019 +0100 dlopen: Fix issues related to NODELETE handling and relocations The assumption behind the assert in activate_nodelete was wrong: Inconsistency detected by ld.so: dl-open.c: 459: activate_nodelete: Assertion `!imap->l_init_called || imap->l_type != lt_loaded' failed! (edit) It can happen that an already-loaded object that is in the local scope is promoted to NODELETE status, via binding to a unique symbol. Similarly, it is possible that such NODELETE promotion occurs to an already-loaded object from the global scope. This is why the loop in activate_nodelete has to cover all objects in the namespace of the new object. In do_lookup_unique, it could happen that the NODELETE status of an already-loaded object was overwritten with a pending NODELETE status. As a result, if dlopen fails, this could cause a loss of the NODELETE status of the affected object, eventually resulting in an incorrect unload. Fixes commit f63b73814f74032c0e5d0a83300e3d864ef905e5 ("Remove all loaded objects if dlopen fails, ignoring NODELETE [BZ #20839]"). (In reply to cvs-commit@gcc.gnu.org from comment #16) > commit 365624e2d2a342cdb693b4cc35d2312169959e28 > Author: Florian Weimer <fweimer@redhat.com> > Date: Fri Dec 13 10:18:24 2019 +0100 > > dlopen: Fix issues related to NODELETE handling and relocations I can confirm this fixed the specific issue I noted. Thank you |