[PATCH v2 0/4] KVM: arm64: UBSAN at EL2

Mostafa Saleh posted 4 patches 7 months, 3 weeks ago
arch/arm64/include/asm/esr.h     | 5 +++++
arch/arm64/kernel/traps.c        | 4 ++--
arch/arm64/kvm/handle_exit.c     | 6 ++++++
arch/arm64/kvm/hyp/nvhe/Makefile | 6 ++++++
arch/x86/kernel/traps.c          | 2 +-
include/linux/ubsan.h            | 6 +++---
lib/Kconfig.ubsan                | 9 +++++++++
lib/ubsan.c                      | 8 +++++---
scripts/Makefile.ubsan           | 5 ++++-
9 files changed, 41 insertions(+), 10 deletions(-)
[PATCH v2 0/4] KVM: arm64: UBSAN at EL2
Posted by Mostafa Saleh 7 months, 3 weeks ago
Many of the sanitizers the kernel supports are disabled when running
in EL2 with nvhe/hvhe/proctected modes, some of those are easier
(and makes more sense) to integrate than others.
Last year, kCFI support was added in [1]

This patchset adds support for UBSAN in EL2.
UBSAN can run in 2 modes:
  1) “Normal” (CONFIG_UBSAN_TRAP=n): In this mode the compiler will
  do the UBSAN checks and insert some function calls in case of
  failures, it can provide more information(ex: what is the value of
  the out of bound) about the failures through those function arguments,
  and those functions(implemented in lib/ubsan.c) will print a report with
  such errors.

  2) Trap (CONFIG_UBSAN_TRAP=y): This is a minimal mode, where similarly,
  the compiler will do the checks, but instead of doing function calls,
  it would do a “brk #imm” (for ARM64) with a unique code with the failure
  type, but without any extra information (ex: only print the out-bound line
  but not the index)

For nvhe/hvhe/proctected modes, #2 would be suitable, as there is no way to
print reports from EL2, so similarly to kCFI(even with permissive) it would
cause the hypervisor to panic.

But that means that for EL2 we need to compile the code with the same options
as used by “CONFIG_UBSAN_TRAP” independently from the kernel config.

This patch series adds a new KCONFIG for ARM64 to choose to enable UBSAN
separately for the modes mentioned.

The same logic decoding the kernel UBSAN is reused, so the messages from
the hypervisor will look similar as:
[   29.215332] kvm [190]: nVHE hyp UBSAN: array index out of bounds at: [<ffff8000811f2344>] __kvm_nvhe_handle___pkvm_init_vm+0xa8/0xac!

In this patch set, the same UBSAN options(for check types) are used for both
EL1/EL2, although a case can be made to have separate options (leading to
totally separate CFLAGS) if we want EL2 to be compiled with stricter checks
for something as protected mode.
However, re-using the current flags, makes code re-use easier for
report_ubsan_failure() and  Makefile.ubsan

[1] https://lore.kernel.org/all/20240610063244.2828978-1-ptosi@google.com/

Changes from v1:
- https://lore.kernel.org/all/20250416180440.231949-1-smostafa@google.com/
- Collected Kees Acked-By
- Rename CFLAGS flag to CFLAGS_UBSAN_TRAP
- Small comment fix

Mostafa Saleh (4):
  arm64: Introduce esr_is_ubsan_brk()
  ubsan: Remove regs from report_ubsan_failure()
  KVM: arm64: Introduce CONFIG_UBSAN_KVM_EL2
  KVM: arm64: Handle UBSAN faults

 arch/arm64/include/asm/esr.h     | 5 +++++
 arch/arm64/kernel/traps.c        | 4 ++--
 arch/arm64/kvm/handle_exit.c     | 6 ++++++
 arch/arm64/kvm/hyp/nvhe/Makefile | 6 ++++++
 arch/x86/kernel/traps.c          | 2 +-
 include/linux/ubsan.h            | 6 +++---
 lib/Kconfig.ubsan                | 9 +++++++++
 lib/ubsan.c                      | 8 +++++---
 scripts/Makefile.ubsan           | 5 ++++-
 9 files changed, 41 insertions(+), 10 deletions(-)

-- 
2.49.0.967.g6a0df3ecc3-goog
Re: [PATCH v2 0/4] KVM: arm64: UBSAN at EL2
Posted by Marc Zyngier 7 months, 2 weeks ago
On Wed, 30 Apr 2025 16:27:07 +0000, Mostafa Saleh wrote:
> Many of the sanitizers the kernel supports are disabled when running
> in EL2 with nvhe/hvhe/proctected modes, some of those are easier
> (and makes more sense) to integrate than others.
> Last year, kCFI support was added in [1]
> 
> This patchset adds support for UBSAN in EL2.
> UBSAN can run in 2 modes:
>   1) “Normal” (CONFIG_UBSAN_TRAP=n): In this mode the compiler will
>   do the UBSAN checks and insert some function calls in case of
>   failures, it can provide more information(ex: what is the value of
>   the out of bound) about the failures through those function arguments,
>   and those functions(implemented in lib/ubsan.c) will print a report with
>   such errors.
> 
> [...]

Applied to next, thanks!

[1/4] arm64: Introduce esr_is_ubsan_brk()
      commit: dc1fd37a7f501731e488c1c6f86b2f591632a4ad
[2/4] ubsan: Remove regs from report_ubsan_failure()
      commit: d683a8561889c1813fe2ad6082769c91e3cb71b3
[3/4] KVM: arm64: Introduce CONFIG_UBSAN_KVM_EL2
      commit: 61b38f7591fb434fce326c1d686a9793c7f418bc
[4/4] KVM: arm64: Handle UBSAN faults
      commit: 446692759b0732ef2d9a93b7e6730aa762ccf0ab

Cheers,

	M.
-- 
Without deviation from the norm, progress is not possible.


Re: [PATCH v2 0/4] KVM: arm64: UBSAN at EL2
Posted by Kees Cook 7 months, 3 weeks ago
On Wed, Apr 30, 2025 at 04:27:07PM +0000, Mostafa Saleh wrote:
> Many of the sanitizers the kernel supports are disabled when running
> in EL2 with nvhe/hvhe/proctected modes, some of those are easier
> (and makes more sense) to integrate than others.
> Last year, kCFI support was added in [1]
> 
> This patchset adds support for UBSAN in EL2.

This touches both UBSAN and arm64 -- I'm happy to land this via the
hardening tree, but I expect the arm64 folks would rather take it via
their tree. What would people like to have happen?

-Kees

-- 
Kees Cook
Re: [PATCH v2 0/4] KVM: arm64: UBSAN at EL2
Posted by Marc Zyngier 7 months, 2 weeks ago
On Wed, 30 Apr 2025 19:32:23 +0100,
Kees Cook <kees@kernel.org> wrote:
> 
> On Wed, Apr 30, 2025 at 04:27:07PM +0000, Mostafa Saleh wrote:
> > Many of the sanitizers the kernel supports are disabled when running
> > in EL2 with nvhe/hvhe/proctected modes, some of those are easier
> > (and makes more sense) to integrate than others.
> > Last year, kCFI support was added in [1]
> > 
> > This patchset adds support for UBSAN in EL2.
> 
> This touches both UBSAN and arm64 -- I'm happy to land this via the
> hardening tree, but I expect the arm64 folks would rather take it via
> their tree. What would people like to have happen?

FWIW, I have now taken this in kvmarm/next. A stable branch is
available at [1] for anyone to pull and resolve potential conflicts.

Thanks,

	M.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/ubsan-el2

-- 
Without deviation from the norm, progress is not possible.
Re: [PATCH v2 0/4] KVM: arm64: UBSAN at EL2
Posted by Kees Cook 7 months, 2 weeks ago
On Wed, May 07, 2025 at 11:35:38AM +0100, Marc Zyngier wrote:
> On Wed, 30 Apr 2025 19:32:23 +0100,
> Kees Cook <kees@kernel.org> wrote:
> > 
> > On Wed, Apr 30, 2025 at 04:27:07PM +0000, Mostafa Saleh wrote:
> > > Many of the sanitizers the kernel supports are disabled when running
> > > in EL2 with nvhe/hvhe/proctected modes, some of those are easier
> > > (and makes more sense) to integrate than others.
> > > Last year, kCFI support was added in [1]
> > > 
> > > This patchset adds support for UBSAN in EL2.
> > 
> > This touches both UBSAN and arm64 -- I'm happy to land this via the
> > hardening tree, but I expect the arm64 folks would rather take it via
> > their tree. What would people like to have happen?
> 
> FWIW, I have now taken this in kvmarm/next. A stable branch is
> available at [1] for anyone to pull and resolve potential conflicts.

Thanks!


-- 
Kees Cook
Re: [PATCH v2 0/4] KVM: arm64: UBSAN at EL2
Posted by Marc Zyngier 7 months, 2 weeks ago
On Wed, 30 Apr 2025 19:32:23 +0100,
Kees Cook <kees@kernel.org> wrote:
> 
> On Wed, Apr 30, 2025 at 04:27:07PM +0000, Mostafa Saleh wrote:
> > Many of the sanitizers the kernel supports are disabled when running
> > in EL2 with nvhe/hvhe/proctected modes, some of those are easier
> > (and makes more sense) to integrate than others.
> > Last year, kCFI support was added in [1]
> > 
> > This patchset adds support for UBSAN in EL2.
> 
> This touches both UBSAN and arm64 -- I'm happy to land this via the
> hardening tree, but I expect the arm64 folks would rather take it via
> their tree. What would people like to have happen?

I don't mind either way, but in any case I'd like a stable branch with
that code so that I can merge it if any conflict occurs in -next.

Alternatively, I can take it via the kvmarm tree, and publish a stable
branch for anyone to pick and resolve conflicts ahead of the merge
window.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.