Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Better support relocating NOTE sections/segments #218

Merged
merged 1 commit into from Jul 31, 2020
Merged

Better support relocating NOTE sections/segments #218

merged 1 commit into from Jul 31, 2020

Conversation

delroth
Copy link
Contributor

@delroth delroth commented Jun 20, 2020

Fixes #217

SHT_NOTE sections can be mapped in memory by PT_NOTE segments. When
rewriting an SHT_NOTE, attempt to also rewrite a matching segment if it
exists.

Note that an ELF can contain multiple SHT_NOTE sections, and a given
PT_NOTE segment can theoretically map multiple contiguous sections. This
is not currently supported, but we fail loudly when encountering it.

Also fix (or more like hack around) alignment issues with note sections.
Keeping the original alignment value when possible is important because
it determines how the data within the section needs to be parsed.

This was referenced Jun 20, 2020
Closed
Closed
@delroth
Copy link
Contributor Author

delroth commented Jun 20, 2020

Hmm, looks like this won't be enough. The situation with multiple SHT_NOTE covered by a single PT_NOTE is common enough that apparently we hit it even in the test suite :-/

rewriting section '.note.ABI-tag' from offset 0x121c (size 32) to offset 0x578 (size 32)
patchelf: unsupported overlap of SHT_NOTE and PT_NOTE
FAIL no-rpath-amd64.sh (exit status: 1)

And indeed there is a single PT_NOTE covering both .note.ABI-tag and .note.gnu.build-id.

Not completely sure what the best way forward is -- maybe some kind of normalization step where we break up the PT_NOTE into multiple segments. I'll give that a shot unless someone comes up with a better idea.

@delroth
Better support relocating NOTE sections/segments
43a3348 
SHT_NOTE sections can be mapped in memory by PT_NOTE segments. When
rewriting an SHT_NOTE, attempt to also rewrite a matching segment if it
exists.

Note that an ELF can contain multiple SHT_NOTE sections, and a given
PT_NOTE segment can theoretically map multiple contiguous sections.
There are multiple ways this could be handled, the one picked here is to
pre-normalize PT_NOTE segments so that a multi-section segment gets
broken up into multiple separate segments instead.

Also fix (or more like hack around) alignment issues with note sections.
Keeping the original alignment value when possible is important because
it determines how the data within the section needs to be parsed.
@delroth
Copy link
Contributor Author

delroth commented Jun 20, 2020

I implemented the normalization. Tests now pass, and as far as I can tell in my limited testing everything seems to be working fine.

@flokli
Copy link

flokli commented Jul 8, 2020

@domenkozar, can you take a look at this? This should fix NixOS/nixpkgs#91145.

@domenkozar
Copy link
Member

I'm not familiar with the codebase, maybe @edolstra can take a look.

@Mic92
Copy link
Member

Mic92 commented Jul 27, 2020

Does any of the tests exercise this segment?

@Mic92
Copy link
Member

Mic92 commented Jul 31, 2020

I also added a regression test in #225

@Mic92
Copy link
Member

Mic92 commented Jul 31, 2020

Feel free to cherry pick.

@edolstra edolstra merged commit 43a3348 into NixOS:master Jul 31, 2020
@edolstra
Copy link
Member

Thanks, I've merged #225 which includes this one!

@Mic92 Mic92 mentioned this pull request Aug 2, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

patchelf doesn't update PT_NOTE segment when relocating SHT_NOTE sections
5 participants
@delroth @flokli @domenkozar @Mic92 @edolstra