Page MenuHomeFreeBSD

riscv: add KASAN support
Needs ReviewPublic

Authored by zishun.yi.dev_gmail.com on Mon, Jun 1, 10:16 AM.
Tags
None
Referenced Files
F159457911: D57381.id179105.diff
Sun, Jun 14, 9:55 AM
F159438809: D57381.id179088.diff
Sun, Jun 14, 3:00 AM
Unknown Object (File)
Sat, Jun 13, 1:03 PM
Unknown Object (File)
Thu, Jun 11, 9:27 AM
Unknown Object (File)
Wed, Jun 10, 5:21 PM
Unknown Object (File)
Wed, Jun 10, 7:23 AM
Unknown Object (File)
Tue, Jun 9, 11:13 PM
Unknown Object (File)
Tue, Jun 9, 2:29 AM

Details

Reviewers
mhorne
markj
Group Reviewers
riscv
Summary

Introduce KASAN support for the riscv architecture. The implementation is similar to arm64.

Additionally, this patch refactors the generation of KASAN atomic interceptors in sys/kern/subr_asan.c:

  • The Problem: KASAN previously forced the declaration and generation of atomic interceptors for all widths unconditionally. This breaks on architectures like riscv that lack some sub-word atomic implementations.
  • The Solution: Instead of bloating the code with massive #ifdef blocks, this patch introduces macros (e.g., ARCH_SUPPORT_ATOMIC_*_WIDTH) to declare what the architecture supports, allowing us to conditionally generate these interceptors. The MI code defaults to declaring support for all atomic operations, while MD code can exclude unsupported operations by #undef and overriding the specific ARCH_SUPPORT_ATOMIC_*_WIDTH macros they lack.

LLVM PR: https://github.com/llvm/llvm-project/pull/202288

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 73732
Build 70615: arc lint + arc unit

Event Timeline

minsoochoo0122_proton.me added inline comments.
contrib/llvm-project/clang/lib/Driver/ToolChains/FreeBSD.cpp
485

Please create a pull request on llvm upstream and add the LLVM PR number to the commit message and summary.

sys/conf/kern.mk
276–277

This has been resolved since LLVM 20 (commit) and we recently MFVed LLVM 21. We can remove this line now. ping @dim

sys/riscv/include/asan.h
5

why?

  1. the year is wrong
  2. we changed preferred license format (see licensing policy)

Prevent KASAN from intercepting atomic functions that are unsupported by the
architecture. The specific changes can be summarized as follows:

  • Use ARCH_SUPPORT_ATOMIC_*_WIDTH x-macros to control the generation of interceptor functions in subr_asan.c.
  • Use #undef in machine/atomic.h to undefine the macros that are forcibly defined by sys/atomic_san.h but lack actual MD implementations.

Please let me know if there is a better way to implement this.

sys/riscv/include/asan.h
5

Oh nvm, this is from sys/arm64/asan.h. Sorry for my mistake, you can mark these as resolved.

sys/kern/subr_asan.c
655

These lines are too long and should be wrapped to 80 columns.

890

Could you please explain what you are changing in this file? The review description does not say anything, and this clearly isn't riscv-specific.

sys/riscv/include/atomic.h
90

Again, what's happening here?

sys/riscv/riscv/pmap.c
5492

This is missing a comment explaining where the constants come from.

5552

Indentation here is wrong, see style(9).

5568

Indentation here is wrong.

5634

Why wmb? Shouldn't all of these routines end with sfence_vma()?

sys/kern/subr_asan.c
890

Sorry, I should have provided more context in the review description. Here is the background:

  • riscv lacks many sub-word atomic implementations (at least prior to the Zabha extension).
  • Currently, atomic_san.h unconditionally adds atomic defines across all architectures to intercept these operations, even if an architecture like riscv doesn't actually support some of them.
  • Similarly, subr_kasan.c forces the implementation of all atomic interceptor functions for all architectures.

I initially tried adding these missing atomic implementations for riscv in D57379. However, jrtc27 pointed out that this could introduce performance regressions, and that the correct approach is to just have KASAN not intercept atomic functions that aren't implemented by the architecture.

Therefore, my implementation logic here is:

  • For declarations: machine/atomic.h should #undef the operations it does not support.
  • For interceptor generation (subr_kasan.c): One alternative was to wrap every generation site in an #ifdef
c
#ifdef atomic_add_8
ASAN_ATOMIC_FUNC_ADD(8, uint8_t);
#endif

However, this would lead to massive file bloat and fails to handle some cases (e.g., when atomic_* exists but atomic_*_acq does not).
To resolve this, I used X-macros. For example, use ARCH_SUPPORT_ATOMIC_ADD_WIDTH to declare the atomic_add width supported by the architecture.
The MI part defaults to supporting all atomic operations, while the MD part overrides the ones it lacks.

I'm not entirely sure if this is the most elegant solution, but it is the best method I could come up with so far.

sys/riscv/include/atomic.h
90

As explained above, the MI part defaults to assuming all atomic operations are supported. This MD section here overrides the width declarations for the operations that are not fully supported. This way, we don't need to modify the code for amd64 and arm64, since they already support almost all atomic operations.

Hi, glad to see this review.

I will take a detailed look next week, and I hope to test it out as well.

Hi, glad to see this review.

I will take a detailed look next week, and I hope to test it out as well.

Thanks for taking a look :)

Fixed hart boot hang on riscv smp caused by KASAN, by initializing the tp register earlier in the assembly stage.

zishun.yi.dev_gmail.com marked 3 inline comments as done.