Page MenuHomeFreeBSD

Add the KASAN runtime
ClosedPublic

Authored by markj on Mar 24 2021, 11:40 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 17, 6:05 AM
Unknown Object (File)
Thu, Apr 11, 5:33 PM
Unknown Object (File)
Mar 12 2024, 7:47 AM
Unknown Object (File)
Mar 12 2024, 7:47 AM
Unknown Object (File)
Mar 12 2024, 7:47 AM
Unknown Object (File)
Mar 12 2024, 7:47 AM
Unknown Object (File)
Mar 12 2024, 7:47 AM
Unknown Object (File)
Mar 12 2024, 7:36 AM

Details

Reviewers
None
Group Reviewers
manpages
Commits
rG38da497a4dfc: Add the KASAN runtime
Summary

This was ported from NetBSD (except for the man page).

The runtime implements a number of functions defined by the compiler
ABI. These are prefixed by asan. The compiler emits calls to
asan_load*() and __asan_store*() around memory accesses, and the
runtime consults the shadow map to determine whether a given access is
valid. This is good for catching use-after-frees and array overruns.

kasan_mark() is called by various kernel allocators to update state in
the shadow map. Updates to those allocators will come in subsequent
commits.

The runtime also defines various interceptors. Some low-level routines
are implemented in assembly and are thus not amenable to compiler
instrumentation. To handle this, the runtime implements these routines
on behalf of the rest of the kernel. The sanitizer implementation
validates memory accesses manually before handing off to the real
implementation.

This port reuses some prior work by andrew@ and Costin Carabas.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 38058
Build 34947: arc lint + arc unit

Event Timeline

Disable checking after a panic.

arichardson added inline comments.
share/man/man9/kasan.9
122

The code parameter is missing here. From reading the rest of the diff 0 seems to be valid, everything else (partially) invalid?

sys/kern/subr_asan.c
243

I wonder if a symbolic constant would be nicer than 0? E.g. kasan_mark(addr, size, size, KASAN_VALID)

andrew added inline comments.
sys/kern/subr_asan.c
180–182

Could this cause possible KASAN recursion if there's a problem in one of these functions?

sys/sys/asan.h
66

It might be useful to define these as do {} while(0) to make them safe to use in all contexts.

share/man/man9/kasan.9
122

That's correct, yes. Will fix.

sys/kern/subr_asan.c
180–182

In principle, yes. We could add a per-thread flag to disable checking while collecting and printing the stack. I'd be inclined to leave it for now until this actually proves to be a problem.

243

I think that's a good idea.

sys/sys/asan.h
66

I always thought that was unnecessary. Is the point to avoid compiler warnings about empty statements?

markj marked an inline comment as done.

Man page updates.

sys/kern/subr_asan.c
243

I thought about this some more and now prefer the current scheme. If the two size parameters are equal, code is unused, and using a plain 0 makes that a bit more clear IMO. Otherwise we should split kasan_mark() into two functions.

gbe added a subscriber: gbe.

LGTM from the manpage perspective.

This revision was not accepted when it landed; it landed in state Needs Review.Apr 13 2021, 9:42 PM
Closed by commit rG38da497a4dfc: Add the KASAN runtime (authored by markj). · Explain Why
This revision was automatically updated to reflect the committed changes.