Add DEBUG_POISON_POINTER

[mjg]

Summary

commit 6ccee9c10b06465f44da97e4b10d57200ff87e69 (HEAD -> poison_ptr4)
Author: Mateusz Guzik <[e-mail]>
Date:   Tue Nov 28 15:23:25 2023 +0000

    Add DEBUG_POISON_POINTER
    
    If you have a pointer which you know points to stale data, you can
    fill it with junk so that dereference later will trap.g
    
    Reviewed by:
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
    Differential Revision:  https://reviews.freebsd.org/D40946

Sample usage is nameidata, which leaks entirely to the caller and where use of stale pointers is very much a possibility. For example accessing ni_dvp when WANTPARENT nor LOCKPARENT were specified.

I don't care about the name or the specific implementation, feel free to roll your own. What I do care about is the functionality.

Test Plan

verified that this:

diff --git a/sys/kern/vfs_lookup.c b/sys/kern/vfs_lookup.c
index 1f62cf3e6652..8eb63eb3dc37 100644

• a/sys/kern/vfs_lookup.c

+++ b/sys/kern/vfs_lookup.c
@@ -1240,7 +1240,13 @@ vfs_lookup(struct nameidata *ndp)
#endif

ndp->ni_dvp = dp;
ndp->ni_vp = NULL;
ASSERT_VOP_LOCKED(dp, "lookup");

+
+ DEBUG_POISON_POINTER(dp);
+ printf("%s: read off dp %p\n", func, dp);
+ printf("%s: deref dp->v_mount %p\n", func, dp->v_mount);
+

/*
 * If we have a shared lock we may need to upgrade the lock for the
 * last operation.

without kasan: prints the first line, traps on the other
with kasan: it traps on the first printf

[kib]

No guarantee exists that your value is trapping, and e.g. it was often not on i386.

The value should be MD. On amd64 non-canonical pointer is much better then anything that can be actually mapped into kernel, so the usual 0xdeadc0de pattern is excellent in this regard. For i386, perhaps 0 is the best value now, but your 0xffffe000 is also mostly fine. No idea about other arches.

[kib]

In fact, on arches where it is not immediately obvious what to use, you could use unmapped_buf (see vfs_bio.c), which already does exactly that for bufs. Still, on amd64 non-canonical value is better.

[mjg]

• use unmapped_buf
• add kasan support

[markj]

sys/sys/asan.h

68 ↗

(On Diff #124460)

Consumers should use kasan_mark() directly, there's no need for KASAN to provide this symbol.

[mjg]

• remove kasan_poison_pointer

[mjg]

ping?

[mjg]

ping

[kib]

I am confused by the current patch. Where is DEBUG_POISON_POINTER_VALUE provided?

[mjg]

• correct diff

[mjg]

First of all my apologies, this somehow fell through the cracks after i pinged.

Trying to use unmapped_buf directly was running into trouble due to __read_mostly annotation. I could not remove it, and including headers to make it available resulted in numerous build failures.

Thus the issue is dodged with a new var.