Page MenuHomeFreeBSD

Add the KASAN runtime
ClosedPublic

Authored by markj on Mar 24 2021, 11:40 PM.

Details

Reviewers
None
Group Reviewers
manpages
Commits
R10:38da497a4dfc: 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
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Disable checking after a panic.

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

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
242

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
179–181

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

sys/sys/asan.h
65

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
121

That's correct, yes. Will fix.

sys/kern/subr_asan.c
179–181

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.

242

I think that's a good idea.

sys/sys/asan.h
65

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
242

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 R10:38da497a4dfc: Add the KASAN runtime (authored by markj). · Explain Why
This revision was automatically updated to reflect the committed changes.