Skip to content

arch/arm64: Add Branch Target Identification#421

Closed
michpappas wants to merge 3 commits intounikraft:stagingfrom
michpappas:arm64_introduce_bti
Closed

arch/arm64: Add Branch Target Identification#421
michpappas wants to merge 3 commits intounikraft:stagingfrom
michpappas:arm64_introduce_bti

Conversation

@michpappas
Copy link
Copy Markdown
Member

Prerequisite checklist

  • Read the contribution guidelines regarding submitting new changes to the project;
  • Tested your changes against relevant architectures and platforms;
  • Ran the checkpatch.pl on your commit series before opening this PR;
  • Updated relevant documentation.

Base target

  • Architecture(s): [e.g. arm64]
  • Platform(s): [common]
  • Application(s): [N/A]

Additional configuration

This PR introduces a new Kconfig option, CONFIG_ARM64_FEAT_BTI. For details, see below.

Description of changes

FEAT_BTI is a hardware protection against JOP-like attacks.
To do that, it introduces:

  • The BTI instruction.
  • The GP field in Stage 1 PTEs.
  • The PSTATE.BTYPE field.

BTI instructions, aka landing pads, are placed by the compiler at branch targets. On runtime, branches that do not land on a BTI instruction trigger an Branch Target Exception.

The GP field indicates whether a page is guarded with BTI. This is allows backwards compatibilty, by disabling BTI on pages that contain non-BTI guarded code. Notice that BTI instructions on unguarded pages execute as NOP.

PSTATE.BTYPE encodes the type of an indirect jump, ie the branch instruction, the registers used to carry parameters, and whether the target page is guarded or or not. When an indirect branch is taken, the processor checks whether PSATE.BTYPE matches the type of the branch target, and on negative match it generates an Branch Target Exception. The purpose of this is to further limit the scope of possible gadgets among BTI protected branches. Notice that there are exceptions to this. For details see D5.4.4 in [2].

Architecture Support

Armv8.5-a introduces FEAT_BTI as a mandatory feature. This feature is only available in AArch64.

GCC Support

GCC-9 introduces support for Armv8.5-A. BTI is supported through the -mbranch-protection parameter.

The parameters passed to -mbranch-protection are interpreted as:

  • none: Disables all branch protections.
  • pac-ret: Enables PAuth for function returns on non-leaf functions. The +leaf modifier enables protection for leaf functions.
  • bti: Enables BTI.
  • standard: Enables all branch protections.

GCC-9 comes with an issue where under certain conditions incorrect BTI instructions are generated. Due of that issue, for BTI support in Unikraft we mandate GCC => 10.

Backwards Compatibility

BTI is implemented as hint instructions. Processors based on older architecture revisions will execute these as NOP.

Changes introduced in this PR

This PR adds BTI support on arm64-based platforms through a new Kconfig option CONFIG_ARM64_FEAT_BTI. Enabling this option instructs GCC to instrument branch targets with landing pads. The default PTE attributes of executable pages and blocks are updated to set the GP attribute when BTI is enabled.

Platform Requirements

As GCC only generates BTI instructions for C code, platforms need to make sure that assembly functions that are called through indirect branching are updated with landing pads before BTI support is enabled.

References

  1. Learn the architecture: Providing protection for complex software
  2. The Arm Architecture Reference Manual Armv8, for Armv8-A architecture profile

@michpappas michpappas requested a review from a team March 6, 2022 10:49
@michpappas michpappas changed the title Arm64 introduce bti arch/arm64: Add Branch Target Identificaiton Mar 6, 2022
@michpappas
Copy link
Copy Markdown
Member Author

This PR depends on #369

@unikraft-bot unikraft-bot added arch/arm arch/arm64 arch/x86_64 area/arch Unikraft Architecture area/include Part of include/uk area/plat Unikraft Patform lang/c Issues or PRs to do with C/C++ plat/common Common to all platforms plat/kvm Unikraft for KVM labels Mar 6, 2022
@unikraft-bot unikraft-bot requested a review from rene March 6, 2022 11:25
@michpappas michpappas force-pushed the arm64_introduce_bti branch 2 times, most recently from 6b53415 to a3f9d02 Compare March 6, 2022 16:19
@razvand razvand added this to the v0.9.0 (Hyperion) milestone Mar 9, 2022
@razvand razvand requested review from razvanvirtan and removed request for a team and rene March 9, 2022 11:31
@michpappas michpappas force-pushed the arm64_introduce_bti branch from a3f9d02 to 34fad74 Compare March 9, 2022 21:01
* int32_t smcc_psci_hvc_call(uint32_t, uint64_t, uint64_t, uint64_t);
*/
ENTRY(smcc_psci_hvc_call)
bti j
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These instructions should be conditional to CONFIG_ARM64_FEAT_BTI. Otherwise will break the build for older compilers (i.e. gcc < 9).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Actually this commit can be removed if PSCI calls are migrated to use the SMCCC API. @razvanvirtan, the BTI PR is scheduled for this release. Are you planning to include the PSCI change on this release too? If not I can push a update on this PR with @rene's suggestion.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michpappas I have created the PSCI change PR: #428
From what I see, this PR (the BTI one) is planed for the next release (0.9), so I think the PSCI one can be included in that release too.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@razvanvirtan awesome, thanks a lot 🚀 I'll look at it this weekend. @razvand could we schedule this one for 0.9 too?

@@ -0,0 +1,35 @@
/* SPDX-License-Identifier: BSD-3-Clause */
/*
* Copyright (c) YYYY, Copyright Holder. All rights reserved.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, fix copyright information.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this is intentional, because these headers are empty, so I think it doesn't make sense to grant the copyright to myself. The idea was that the copyright should be updated by whoever adds content. This is also what I do in the next commit where I add come content to arch/arm/arm64/include/uk/asm/compile.h and grant the copyright to myself.

@@ -0,0 +1,35 @@
/* SPDX-License-Identifier: BSD-3-Clause */
/*
* Copyright (c) YYYY, Copyright Holder. All rights reserved.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, fix copyright information.

@@ -0,0 +1,35 @@
/* SPDX-License-Identifier: BSD-3-Clause */
/*
* Copyright (c) YYYY, Copyright Holder. All rights reserved.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, fix copyright information.

@michpappas michpappas force-pushed the arm64_introduce_bti branch from 34fad74 to 6acd153 Compare March 19, 2022 15:02
@michpappas
Copy link
Copy Markdown
Member Author

Updated PR with a fix for checkpatch

@michpappas michpappas force-pushed the arm64_introduce_bti branch from 6acd153 to 1ddd550 Compare April 16, 2022 17:00
@michpappas
Copy link
Copy Markdown
Member Author

Changes in this update:

@michpappas michpappas changed the title arch/arm64: Add Branch Target Identificaiton arch/arm64: Add Branch Target Identification Apr 16, 2022
@michpappas
Copy link
Copy Markdown
Member Author

michpappas commented Apr 16, 2022

@razvand, @razvanvirtan, @rene, @vladandrew this PR now depends on #428. I have also added this commit in this series to facilitate testing. Will rebase once #428 is merged.

Copy link
Copy Markdown
Member

@StefanJum StefanJum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!
Should be rebased now that #428 is merged.

Reviewed-by: Stefan Jumarea stefanjumarea02@gmail.com

BTI (FEAT_BTI) is a hardware protection against JOP-like attacks. When
BTI is enabled the compiler generates bti guard instructions, aka landing
pads, on branch targets. On runtime, branches that do not land on a bti
instruction trigger a Branch Target Exception.

The expected branch type of an indirect jump is encoded in PSTATE.BTYPE.
This further limits the scope of possible targets in BTI guarded pages.

Pages guarded by BTI are marked by setting the GP field in their
corresponding PTE. This is allows backwards compatibility, by disabling BTI
on pages that contain non-BTI guarded code. Notice that BTI instructions on
unguarded pages execute as NOP.

FEAT_BTI is introduced as a mandatory feature in Armv8.5. As BTI is
implemented in the Hint space, this feature is backwards compatible with
systems based on older architecture revisions.

This commit adds BTI support on arm64-based platforms through a new Kconfig
option CONFIG_ARM64_FEAT_BTI. When enabled, the build system passes GCC the
appropriate flags to enable BTI. It further updates default PTE attributes
of executable pages are updated to set the GP attribute, when BTI is
enabled.

As GCC only generates BTI instructions for C code, platforms need to make
sure that assembly functions that are called through indirect branching are
updated with landing pads before BTI support is enabled.

Signed-off-by: Michalis Pappas <mpappas@fastmail.fm>
Currently _libkvmplat_start() switches to the runtime stack via a
trampoline before passing control to ukboot. This appears to be
unnecessary and introduces complications when BTI is enabled.
Migrate all logic to _libkvmplat_start() to facilitate BTI.

Signed-off-by: Michalis Pappas <mpappas@fastmail.fm>
Update assembly implementation of SMCCC conduits with the necessary
PAuth and BTI instructions. This fixes an issue where any code that
invokes an SMCCC conduit will cause a Data Abort, due to the lack
of branch protection instructions. That will bring the system into
an infinite loop, as `system_off()` invokes a PSCI call, yet again
via an SMCCC conduit.

Signed-off-by: Michalis Pappas <mpappas@fastmail.fm>
@michpappas michpappas force-pushed the arm64_introduce_bti branch from 1ddd550 to 3e5e3ee Compare June 4, 2022 10:04
@michpappas
Copy link
Copy Markdown
Member Author

Rebased to staging.

@unikraft-bot
Copy link
Copy Markdown
Member

Checkpatch passed

Beep boop! I ran Unikraft's checkpatch.pl support script on your pull request and it all looks good!

SHA commit checkpatch
7fed583 arch/arm64: plat/common: Add Branch Target Identification
ff98962 plat/kvm/arm: Simplify _libkvmplat_start()
3e5e3ee plat/common/arm: Add PAuth / BTI instructions to SMCCC conduits

Copy link
Copy Markdown

@rene rene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michpappas , looks good to me.

Reviewed-by: Renê de Souza Pinto rene@renesp.com.br

Copy link
Copy Markdown
Member

@vladandrew vladandrew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great addition to Unikraft!

Approved-by: Vlad Badoiu vlad_andrei.badoiu@upb.ro

unikraft-bot pushed a commit that referenced this pull request Jun 8, 2022
Currently _libkvmplat_start() switches to the runtime stack via a
trampoline before passing control to ukboot. This appears to be
unnecessary and introduces complications when BTI is enabled.
Migrate all logic to _libkvmplat_start() to facilitate BTI.

Signed-off-by: Michalis Pappas <mpappas@fastmail.fm>
Reviewed-by: Stefan Jumarea <stefanjumarea02@gmail.com>
Reviewed-by: Renê de Souza Pinto <rene@renesp.com.br>
Approved-by: Vlad Badoiu <vlad_andrei.badoiu@upb.ro>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Closes: #421
unikraft-bot pushed a commit that referenced this pull request Jun 8, 2022
Update assembly implementation of SMCCC conduits with the necessary
PAuth and BTI instructions. This fixes an issue where any code that
invokes an SMCCC conduit will cause a Data Abort, due to the lack
of branch protection instructions. That will bring the system into
an infinite loop, as `system_off()` invokes a PSCI call, yet again
via an SMCCC conduit.

Signed-off-by: Michalis Pappas <mpappas@fastmail.fm>
Reviewed-by: Stefan Jumarea <stefanjumarea02@gmail.com>
Reviewed-by: Renê de Souza Pinto <rene@renesp.com.br>
Approved-by: Vlad Badoiu <vlad_andrei.badoiu@upb.ro>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Closes: #421
@unikraft-bot unikraft-bot added the ci/merged Merged by CI label Jun 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch/arm arch/arm64 arch/x86_64 area/arch Unikraft Architecture area/include Part of include/uk area/plat Unikraft Patform arm/smcc ci/merged Merged by CI lang/c Issues or PRs to do with C/C++ plat/common Common to all platforms plat/kvm Unikraft for KVM

Projects

Status: ✅ Done!

Development

Successfully merging this pull request may close these issues.

7 participants