Page MenuHomePhabricator

libhwasan initialisation include kernel syscall ABI relaxation
ClosedPublic

Authored by mmalcomson on Oct 10 2019, 8:32 AM.

Details

Summary

Until now AArch64 development has been on patched kernels that have an always
on relaxed syscall ABI where tagged pointers are accepted.
The patches that have gone into the mainline kernel rely on each process opting
in to this relaxed ABI.

This commit adds code to choose that ABI into __hwasan_init.

The idea has already been agreed with one of the hwasan developers
(http://lists.llvm.org/pipermail/llvm-dev/2019-September/135328.html).

The patch ignores failures of EINVAL for Android, since there are older versions of the Android kernel that don't require this prctl or even have the relevant values. Avoiding EINVAL will let the library run on them.

I've tested this on an AArch64 VM running a kernel that requires this
prctl, having compiled both with clang and gcc.

Diff Detail

Event Timeline

mmalcomson created this revision.Oct 10 2019, 8:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 10 2019, 8:32 AM
Herald added subscribers: llvm-commits, Restricted Project, kristof.beyls, srhines. · View Herald Transcript

Thank you!
The patches have been merged to the android common kernel just a few days ago, 4.14 and 4.19:
https://android.googlesource.com/kernel/common/+/690c4ca8a5715644370384672f24d95b042db74a

Pixel kernels in Q have an early version of the patch set without the prctl (i.e. the feature is always enabled). Future releases will require a prctl.
So please do a prctl on Android anyway, but ignore EINVAL.

mmalcomson edited the summary of this revision. (Show Details)

Run prctl syscall for Android, but ignore EINVAL failures.

NOTE: I don't believe this distinguishes between running on a kernel with with the tagged address ABI unconditional or running on a newer kernel or on a kernel with sysctl abi.tagged_addr_disabled=1 (https://android.googlesource.com/kernel/common/+/690c4ca8a5715644370384672f24d95b042db74a/Documentation/arm64/tagged-address-abi.rst)

I doubt this will be much of a concern -- there was already a requirement of having the correct Android kernel for things to work -- but am mentioning it for posterity.

Run prctl syscall for Android, but ignore EINVAL failures.

NOTE: I don't believe this distinguishes between running on a kernel with with the tagged address ABI unconditional or running on a newer kernel or on a kernel with sysctl abi.tagged_addr_disabled=1 (https://android.googlesource.com/kernel/common/+/690c4ca8a5715644370384672f24d95b042db74a/Documentation/arm64/tagged-address-abi.rst)

This is a good point. It appears that PR_GET_TAGGED_ADDR_CTRL works even when abi.tagged_addr_disabled=1, we can use it to tell these two cases apart, but there is not a lot we could do with that information.

A real test would be invoking a random syscall with a tagged pointer, ex. uname(). I don't think we need to go that far, but leaving it up to you.

compiler-rt/lib/hwasan/hwasan.cpp
359

Please move it to InitInstrumentation to handle __hwasan_init_static, too.

compiler-rt/lib/hwasan/hwasan_linux.cpp
158

This needs to be internal_prctl because prctl implementation in libc can be built with hwasan.

Run prctl syscall for Android, but ignore EINVAL failures.

NOTE: I don't believe this distinguishes between running on a kernel with with the tagged address ABI unconditional or running on a newer kernel or on a kernel with sysctl abi.tagged_addr_disabled=1 (https://android.googlesource.com/kernel/common/+/690c4ca8a5715644370384672f24d95b042db74a/Documentation/arm64/tagged-address-abi.rst)

This is a good point. It appears that PR_GET_TAGGED_ADDR_CTRL works even when abi.tagged_addr_disabled=1, we can use it to tell these two cases apart

Ah! So it does!
I'd apparently not updated my kernel since this was added -- thanks for the catch!

I've used that to add a slightly nicer error message and implemented the suggested changes.

eugenis accepted this revision.Oct 15 2019, 2:22 PM

LGTM
Thank you!
Do you want me to submit this change for you?

This revision is now accepted and ready to land.Oct 15 2019, 2:22 PM

LGTM
Thank you!
Do you want me to submit this change for you?

Yes, thanks! That would be great!

eugenis updated this revision to Diff 225507.Oct 17 2019, 1:28 PM
eugenis edited the summary of this revision. (Show Details)

Add (uptr) cast to -1 to silence "comparison of integers of different signs" warning.
Apply clang-format.

This revision was automatically updated to reflect the committed changes.