Bug #14127
closed
ld(1) can double free when cleaning up
100%
Description
With the introduction of #14090 we got an ld(1) that actually noticed allocator errors. Unfortunately, building graphviz for OI uncovered bugs in ld due to the greater checking libumem does. It appears that when cleaning up input files we double free and crash.
ld: warning: file /builds/richlowe/oi-userland/components/image/graphviz/build/i86/lib/cdt/.libs/libcdt.so: linked to ../../lib/cdt/.libs/libcdt.so: attempted multiple inclusion of file free(d083d0): double-free or invalid buffer stack trace: libumem.so.1'umem_err_recoverable+0xd3 libumem.so.1'process_free+0xa5 libumem.so.1'umem_malloc_free+0x1a libelf.so.1'elf_end+0x112 libld.so.4'ifl_list_cleanup+0x33 libld.so.4'ld32_ofl_cleanup+0x25 libld.so.4'ld32_main+0x69d ld'main+0xb6 ld'_start_crt+0x87 ld'_start+0x18
This is partly what we did this for, but we should fix it before re-introducing #14090
Related issues
Updated by Marcel Telka over 1 year ago
- Related to Feature #14090: ld(1) could use a normal allocator added
Updated by Dan McDonald over 1 year ago
For additional testing, I'd recommend contacting distro-runners with instructions to:
- Build their illumos with this and 14090.
- Install it on their build-the-world environments.
- Literally build their world on said environment.
And see what happens.
Updated by Rich Lowe over 1 year ago
There are two problems here.
The main one is a double free in elf_end(). If a member of an archive is ended and the refcount of the archive itself drops to 0, we loop around and free the archive as well, but do so incorrectly in such a manner that leaves us double-freeing the last part of the archive member.
Secondarily, we handle -z allextract and archives weirdly and close the archive as soon as possible (though this won't actually free anything, because of reference counts from the members). This means when we reach the elf_end of the last member, we trigger the first problem. It also somewhat unnecessarily complicates the cleanup to no real benefit (we finally free the archive only very slightly sooner than we would do so in the general case). The main thing we actually get out of this is the ability to use the archive elf handle being NULL as a flag to indicate that the archive has been allextract'ed.
I plan to fix the obvious double free, and also make cleanup of archives always take the same, simpler, path. And add a real boolean indicating that an archive was processed under allextract
Updated by Rich Lowe over 1 year ago
testing:
jperkin built all of pkgsrc using this link editor, with on core dumps from ld(1) and no suspicious new failures.
I have built pieces of openindiana that would previously have hit this bug with on issues.I believe a complete openindiana rebuild to be almost impossible (Dan), but will ask Andy about OmniOS and -extra
Updated by Electric Monk over 1 year ago
- Status changed from New to Closed
- % Done changed from 0 to 100
git commit b3403853e80914bd0aade9b5b605da4878078173
commit b3403853e80914bd0aade9b5b605da4878078173
Author: Richard Lowe <richlowe@richlowe.net>
Date: 2021-10-20T18:08:01.000Z
14127 ld(1) can double free when cleaning up
Reviewed by: Andy Fiddaman <andy@omnios.org>
Reviewed by: Robert Mustacchi <rm+illumos@fingolfin.org>
Approved by: Dan McDonald <danmcd@joyent.com>