Re: PAC and BTI support on arm64
Hi!
On Sun, 2022-05-29 at 14:19:05 +0100, Wookey wrote:
> Discussion:
> The -mbranch-protection=standard option could be set either by dpkg or
> by gcc. I'm not quite sure how we decide which is most appropriate?
I think if the option has potential for breakage or for a performance
hit, then it tends to be better to enable them via dpkg hardening
flags, otherwise people building in Debian systems that are not
interested in the packaging defaults might get surprised.
> They could be an unconditional architecture default, or could be part
> of the dpkg hardening flags. It could be one hardening feature 'PACBTI',
> or separate 'PAC' and 'BTI' features (with corresponding logic to set
> the right flags).
> A notable aspect of this flag is that it is arch-specific. It should
> not be issued when targetting arches other than arm64 as it is not a
> valid flag there. dpkg-buildflags gets this right with below patch.
> The -fcf-protection option on amd64 has the same
> characteristic and indeed is pretty-much the same functionality (as
> BTI). (It might have been better if the gcc developers had had one
> option that added this type of branch protection on both
> architectures, and ignored it on other arches, but they didn't.) That
> option appears to have been set by default in Ubuntu 19.10 gcc
> targetting aarch64 (although it's a bit hard to tell as gcc dumpspecs
> is pretty cryptic, and I could be wrong.
If there's potential for needing to disable them (for whatever reason),
then adding them into their own feature would be better. But having a
feature that is arch-specific (not just that it does not currently
work on some arches), seems not ideal.
I think I'd probably want a new feature that can potentially be used
on other arches such as with -fcf-protection on amd64, yes.
Say hardening=+branch or perhaps branchprotection or similar.
> The hardening features seem to assume that they are all
> implemented/available on all architectures? I'm not sure if extra work
> would be needed for a hardening feature that only existed on one arch
> (so far) (PAC). (BTI exists on two arches). Perhaps all this is an
> argument for just turning it on by default in gcc instead?
The features can be easily shadowed for specific arches, if for
example they currently have no proper support for the flags. But
I'd rather abstract this than have a new feature per option denoting
similar protections for each arch.
> As I said, I'm not sure what our policy is on what goes in gcc default
> flags, dpkg default flags or dpkg feature flags. So I'm open to
> suggestions on the best/right way to implement this, then will prepare
> patches/file bugs.
I think the attached patch might do (it's still missing man page
update), assuming that the flags work on all compiler variables, and
the amd64 case could be disabled for now if there are concerns of
enabling both at the same time (it could be brought up later).
The main issue is that because most packages just do hardening=+all,
even adding such feature disabled by default, would imply pretty much
enabling it immediately, so we might as well just enable it by
default. But to do that, the usual step after having done an archive
rebuild (if it seemed appropriate) and which you already did, would be
to bring this up on the debian-devel mailing list. If there are no
objections after a bit, I'm fine adding the support on the next dpkg
release after that time.
> The only known compatibility risk is running modern binaries (built
> with PAC enabled) on modern hardware (ARMv8.3 or newer) on binaries
> (e.g. in an old chroot) built with gcc older than 7 (so that the
> tagged pointers are not recognised and properly masked), which might
> cause hangs/faults during exception unwinding, should some
> exception-causing error occur.
I'm not sure I understand this sentence above about “running modern
binaries … on binaries”? Did you mean run-time linked, or is this
related to the kernel? Or something else?
> Obviously this is a potentially intrusive change, despite the
> backwards-compatible design, so we have done a mass rebuild of the
> archive with this option set to see if there were any problems. Of
> 14371 packages there were 12 packages with build issues, 4 of those
> were trying to use -mbranch-protection=standard with an x86 compiler
> due to the simplistic way the flags were set for the test: echo -e
> "APPEND CFLAGS -mbranch-protection=standard\nAPPEND CXXFLAGS
> -mbranch-protection=standard"> /etc/dpkg/buildflags.conf (which
> applies to all arches, not just arm64)
That result looks pretty good TBH.
> I have implemented this in the Debian Vendor options, but actually it
> should probably be turned on everywhere unless some distro has a good
> reason not to. IIUC the debian settings are inherited unless overridden?
Currently the default settings cannot be modified by derivatives, but
I've got some code adding support for that. In any case see above
about making it possible to disable, and the «+all» case anyway.
Thanks,
Guillem
diff --git i/scripts/Dpkg/Vendor/Debian.pm w/scripts/Dpkg/Vendor/Debian.pm
index c293a99d6..7dcaab718 100644
--- i/scripts/Dpkg/Vendor/Debian.pm
+++ w/scripts/Dpkg/Vendor/Debian.pm
@@ -129,6 +129,7 @@ sub _add_build_flags {
format => 1,
relro => 1,
bindnow => 0,
+ branch => 1,
},
);
@@ -364,6 +365,11 @@ sub _add_build_flags {
# relro not implemented on ia64, hppa, avr32.
$use_feature{hardening}{relro} = 0;
}
+ if ($cpu !~ /^(?:amd64|arm64)$/) {
+ # On amd64 use -fcf-protection.
+ # On arm64 use -mbranch-protection=standard.
+ $use_feature{hardening}{branch} = 0;
+ }
# Mask features that might be influenced by other flags.
if ($opts_build->has('noopt')) {
@@ -430,6 +436,17 @@ sub _add_build_flags {
$flags->append('LDFLAGS', '-Wl,-z,now');
}
+ # Branch protection
+ if ($use_feature{hardening}{branch}) {
+ my $flag;
+ if ($cpu eq 'arm64') {
+ $flag = '-mbranch-protection=standard';
+ } elsif ($cpu eq 'amd64') {
+ $flag = '-fcf-protection';
+ }
+ $flags->append($_, $flag) foreach @compile_flags;
+ }
+
## Commit
# Set used features to their builtin setting if unset.
diff --git i/scripts/t/Dpkg_BuildFlags.t w/scripts/t/Dpkg_BuildFlags.t
index d2d3d7aee..47d675c49 100644
--- i/scripts/t/Dpkg_BuildFlags.t
+++ w/scripts/t/Dpkg_BuildFlags.t
@@ -55,6 +55,7 @@ my %known_features = (
) ],
hardening => [ qw(
bindnow
+ branch
format
fortify
pie
Reply to: