Page MenuHomeFreeBSD

Fix PowerPC64 ELFv1-specific problem in __elf_phdr_match_addr() leading to crash in threaded programs that unload libraries.

Authored by on Nov 28 2018, 1:25 AM.



sfs reported a python crash in IRC earlier that ended up being due to an oversight in __elf_phdr_match_addr().

Due to __elf_phdr_match_addr() limiting its search to PF_X sections, on the PPC64 ELFv1 ABI, it was never matching function pointers properly.

This meant that libthr was never cleaning up its atfork list in __pthread_cxa_finalize(), so if a library with an atfork handler was unloaded, libthr would crash on the next fork.

Normally, the null pointer check it does before calling the handler would avoid this crash, but, due to PPC64 ELFv1 using function descriptors instead of raw function pointers, a null check against the pointer itself is insufficient, as the pointer itself was not null, it was just pointing at a function descriptor that had been zeroed. (Which is an ABI violation.)

Calling a zeroed function descriptor on PPC64 ELFv1 causes a jump to address 0 with a zeroed r2 and r11.

Test Plan

Build devel/libsoup on powerpc64. After this patch, it should no longer crash in the middle of the build.

Diff Detail

rS FreeBSD src repository
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

kib added inline comments.Nov 28 2018, 9:11 AM
58 ↗(On Diff #51239)

Leave only the || (ph->p_flags & PF_X) == 0 expression under the #ifdef. PT_LOAD test should be not doubled. edited the test plan for this revision. (Show Details)

Replaced the comment block with one that jhibbits wrote that's worded a lot better than mine.

kib accepted this revision.Dec 1 2018, 10:05 AM
kib added inline comments.
65 ↗(On Diff #51453)

Add a blank line before this if(). Style requires it after the end of code with the block comment.

This revision is now accepted and ready to land.Dec 1 2018, 10:05 AM
This revision was automatically updated to reflect the committed changes.