Project

General

Profile

Actions

Bug #14127

closed

ld(1) can double free when cleaning up

Added by Rich Lowe 4 months ago. Updated 3 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
tools - gate/build tools
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

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

Related to illumos gate - Feature #14090: ld(1) could use a normal allocatorClosedRich Lowe

Actions
Actions #1

Updated by Marcel Telka 4 months ago

Actions #2

Updated by Dan McDonald 4 months 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.

Actions #3

Updated by Rich Lowe 4 months 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

Actions #4

Updated by Rich Lowe 4 months 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
Actions #5

Updated by Electric Monk 4 months ago

  • Gerrit CR set to 1743
Actions #6

Updated by Electric Monk 3 months 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>

Actions

Also available in: Atom PDF