Page MenuHomeFreeBSD
Differential D8681

EFI loaders: amd64 uses rela relocations
ClosedPublic
Actions

Authored by emaste on Nov 30 2016, 4:23 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Mar 4, 5:39 AM
Unknown Object (File)
Jan 13 2025, 9:49 PM
Unknown Object (File)
Dec 28 2024, 4:57 AM
Unknown Object (File)
Dec 9 2024, 6:05 AM
Unknown Object (File)
Nov 24 2024, 3:13 PM
Unknown Object (File)
Nov 19 2024, 10:01 PM
Unknown Object (File)
Oct 27 2024, 4:56 AM
Unknown Object (File)
Oct 27 2024, 4:55 AM
View All 74 Files
Subscribers
benno
imp

Details

Reviewers
jhb
andrew
Commits
rS309360: EFI loaders: parse rela relocations on amd64
Summary

Prior to this change the loader self relocation code interpreted amd64's rela relocations as if they were rel relocations, discarding the addend. This "worked" because GNU ld 2.17.50 stores the addend also at the relocation target, for some reason.

Other linkers, and possibly other versions of GNU ld, don't have this behaviour, so we must interpret relocations correctly.

Reported by: George Rimar

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

emaste updated this revision to Diff 22620.Nov 30 2016, 4:23 PM
emaste retitled this revision from to EFI loaders: amd64 uses rela relocations.
emaste updated this object.
emaste edited the test plan for this revision. (Show Details)
emaste added reviewers: andrew, jhb.
emaste added a subscriber: benno.
andrew added inline comments.Nov 30 2016, 4:25 PM
sys/boot/common/self_reloc.c
108 ↗(On Diff #22620)

Could we keep some of the comment? The code relies on being linked with a base address of 0 (at least on arm64).

emaste updated this revision to Diff 22622.Nov 30 2016, 4:47 PM

Restore comment about link address. It applies to all of the _RELATIVE relocation types.

emaste added a comment.Nov 30 2016, 4:51 PM

Also per conversation on IRC we should change this to either handle both relocation section types. Or, at least accept only whichever of DT_REL/DT_RELA is appropriate for the architecture. I intend to make that change soon.

emaste added a subscriber: imp.Nov 30 2016, 4:57 PM
andrew accepted this revision.Nov 30 2016, 5:21 PM
andrew edited edge metadata.

I'm happy, but someone who knows more about arm64 should check those bits.

This revision is now accepted and ready to land.Nov 30 2016, 5:21 PM
emaste updated this revision to Diff 22623.Nov 30 2016, 5:39 PM
emaste edited edge metadata.

Restore accidentally deleted #else

This revision now requires review to proceed.Nov 30 2016, 5:39 PM
emaste added a comment.Nov 30 2016, 7:41 PM
In D8681#179906, @andrew wrote:

I'm happy, but someone who knows more about arm64 should check those bits.

I assume you meant amd64 :) Hopefully @jhb is fine with this change (with the expectation that there's a future improvement as described above).

andrew accepted this revision.Dec 1 2016, 10:40 AM
andrew edited edge metadata.
In D8681#179942, @emaste wrote:
In D8681#179906, @andrew wrote:

I'm happy, but someone who knows more about arm64 should check those bits.

I assume you meant amd64 :)

Yes

This revision is now accepted and ready to land.Dec 1 2016, 10:40 AM
Closed by commit rS309360: EFI loaders: parse rela relocations on amd64 (authored by emaste). · Explain WhyDec 1 2016, 2:28 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
head/
sys/
boot/
common/
17 lines

Diff 22639

View Options

head/sys/boot/common/self_reloc.c

Loading...