akirilov-arm opened PR #4195 from fiber_cfi
to main
:
Now the fiber implementation on AArch64 authenticates function
return addresses and includes the relevant BTI instructions, except
on macOS.Copyright (c) 2022, Arm Limited.
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
akirilov-arm requested alexcrichton for a review on PR #4195.
akirilov-arm edited PR #4195 from fiber_cfi
to main
:
Now the fiber implementation on AArch64 authenticates function return addresses and includes the relevant BTI instructions, except on macOS.
This PR has been spun off from #3606 and fixes #3183.
alexcrichton submitted PR review.
alexcrichton submitted PR review.
alexcrichton created PR review comment:
Is the "FIXME" here referring to that theoretically we should add like a
FastAppleAarch64
convention or something like that? If that's the case it's definitely ok to leave this just as a comment for now, and perosnally I'd be ok leaving this as an implementation detial until someone otherwise proves that it's better to add the fast aarch64 variant.
alexcrichton created PR review comment:
Personally I work in assembly so little I end up needing a lot of comments to help guide me through what's happening here. I can sort of piece together what I think is happening here but would you be up for writing a longer-form comment at the top of this file (or somewhere appropriate) about the register authentication bits that are implemented here? (and then refer to that comment from this section)
alexcrichton created PR review comment:
To confirm, this is because the macos assembler doesn't have support for the
paciasp
instruction natively? (do you know if there's a version that does support it?)
alexcrichton created PR review comment:
Personally I'm pretty unfamiliar with all of this. Would you be up for adding a comment here to why this is necessary and perhaps an online doc if possible explaining the format of this section?
akirilov-arm submitted PR review.
akirilov-arm created PR review comment:
Yes, that's right. TBH currently the ABI implementation does not really distinguish between calling conventions, i.e. it generates essentially the same code in all cases (except for the unwinding information, as discussed here).
akirilov-arm submitted PR review.
akirilov-arm created PR review comment:
macOS differs in several details in the way it handles authenticated pointers (e.g. it uses the B key, i.e. the
PACIBSP
instruction in this case), particularly with respect to unwinding, and currently I am unable to tackle the implementation properly, so I have chosen to skip it for the time being. Note that we are actually using aHINT
instruction here (essentially a no-op), which is part of the base 64-bit architecture, so assemblers should not have any issue with it, even if they are really old;PACIASP
is just an alias for aHINT
with a particular immediate.
alexcrichton submitted PR review.
alexcrichton created PR review comment:
Oh wow sorry I completely misread "ifndef CFG_TARGET_OS_macos" as "if macos" which is entirely the opposite of what's happening here.
Otherwise those makes sense! Although the same question still somewhat applies, do general assemblers not recognize
paciasp
as an instruction? (or is there like some common assembler that only recently got support for such syntax?)
akirilov-arm submitted PR review.
akirilov-arm created PR review comment:
Arm actually maintains documentation on the support for various architectural features by the 2 major open source toolchains - here is the page for the GNU toolchain and here is the information for the LLVM one. So, consider Ubuntu 18.04 (LTS release, still supported) - it ships with version 2.30 of Binutils, so it does support pointer authentication (i.e. the
autiasp
,pacia1716
, andpaciasp
instructions), but it does not support BTI; I am not sure what the case is with Linux distributions with even longer support periods such as RHEL. I have decided to be conservative and use an approach that works as long as the assembler has AArch64 support at all, but I would not mind changing it.
cfallin submitted PR review.
cfallin created PR review comment:
At least for
bti
, if a currently supported Ubuntu can't assemble the instruction, IMHO it seems like we should probably avoid it for the time being -- we otherwise don't really have stringent version requirements on the system toolchain AFAIK, just on Rust, and I've seen plenty of old Ubuntu (and other distro) installs in production in various places.Could we
#define _bti hint #XX
and maybe the PAC instructions too for uniformity?(I'll defer to Alex on this though if he feels differently, he's far more qualified on toolchain questions generally)
akirilov-arm edited PR #4195 from fiber_cfi
to main
:
Now the fiber implementation on AArch64 authenticates function return addresses and includes the relevant BTI instructions, except on macOS, following the guidelines presented in the bytecodealliance/rfcs#17 RFC proposal.
This PR has been spun off from #3606 and fixes #3183.
alexcrichton submitted PR review.
alexcrichton created PR review comment:
Oh I'm mostly just curious I don't think any concrete changes are needed. The
// actual-instr
comments are clear enough to me what it's supposed to be. If there's some assembler that needs thehint ...
instead of the actual instruction seems fine to leave these in.
akirilov-arm updated PR #4195 from fiber_cfi
to main
.
alexcrichton submitted PR review.
alexcrichton merged PR #4195.
Last updated: Dec 23 2024 at 12:05 UTC