-
Notifications
You must be signed in to change notification settings - Fork 101
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
Check for a branded ELF note when OS/ABI is NONE #1344
base: master
Are you sure you want to change the base?
Conversation
Currently the auditor is only checking for the correct OS/ABI and producing a warning on FreeBSD when it doesn't match. This warning is incorrect on AArch64 though, as FreeBSD instead uses an ELF note to declare the OS/ABI. The change implemented here looks for such a note when the OS/ABI in the ELF header is NONE.
I haven't looked at the implementation yet, but I want to quickly answer the question
Yeah, the problem is that we don't know (or forgot? who knows) how libraries with the wrong OS/ABI field were generated on FreeBSD. An alternative should be to compile a simple dummy library (something like |
# On AArch64 and RISC-V, FreeBSD uses an ELF note section to identify itself rather | ||
# than OS/ABI in the ELF header. In that case, the OS/ABI will be generic Unix (NONE). | ||
# See https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=252490 and | ||
# https://github.com/freebsd/freebsd-src/blob/main/lib/csu/common/crtbrand.S |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# https://github.com/freebsd/freebsd-src/blob/main/lib/csu/common/crtbrand.S | |
# https://github.com/freebsd/freebsd-src/blob/3c95262007ef934c9e98b87460a48889bf42c1b9/lib/csu/common/crtbrand.S |
@@ -39,7 +64,9 @@ function platform_for_object(oh::ObjectHandle) | |||
end | |||
end | |||
|
|||
if oh.ei.osabi == ELF.ELFOSABI_LINUX || oh.ei.osabi == ELF.ELFOSABI_NONE | |||
if oh.ei.osabi == ELF.ELFOSABI_NONE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we just delete platform_for_object
since it appears to be unused?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I can if you'd like me to.
Ah rip, the patchelf we ship is old and doesn't have |
Sigh, never mind the test then, we can't upgrade patchelf because newer version is broken (JuliaPackaging/Yggdrasil#7728 (comment), caused by NixOS/patchelf#492) |
Currently the auditor is only checking for the correct OS/ABI and producing a warning on FreeBSD when it doesn't match. This warning is incorrect on AArch64 though, as FreeBSD instead uses an ELF note to declare the OS/ABI. The change implemented here looks for such a note when the OS/ABI in the ELF header is NONE.
Note that there are no tests added... I'd be interested in any suggestions for how best to test this given that the functions I modified are not currently tested, at least directly (AFAICT).