kern: array bounds sanitizer exploit mitigation

[CalANDif]

The UBSan array bounds sanitizer adds instrumentation to ensure indexes into fixed size arrays are within bounds. If the array access is out of bounds, a platform dependent trap instruction will be executed, preventing potential information leaks and memory corruption avenues from occurring in the first place - effectively improving the memory safety of the FreeBSD kernel.

This PR is more so meant for discussion around if there is interest in implementing this.

The array bounds sanitizer is light weight (compare, jump if greater than, with the branch target a trap instruction) , adding instrumentation only when it cannot prove statically that the
access would remain in bounds. By using a trap instruction, along with preventing the trap instructions from merging, existing debugging facilities can be used to identify which specific array access was out of bounds. The expected impact for standard kernel operation is very minimal, due to existing KUBSAN builds and fixing undefined behaviours. In the future, or perhaps for production builds, the trap instructions can be merged together to reduce .text space usage.

Along with addressing a particular class of vulnerabilities at the compiler level, this change will be beneficial for kernel developers as well, allowing them to identify mistakes earlier in the development process, hopefully saving them time and effort earlier on. Furthermore, it does not require source code modification or annotation to work, unlike https://wiki.freebsd.org/BoundsSafety

Two previous CVE related commits that would have been identified and blocked by array bounds instrumentation are:

9049b13: CVE-2014-0998, Keeps the VT_WAITACTIVE window selector unsigned, preventing large user values from becoming negative subscripts into fixed vd_windows[VT_MAXWINDOWS] storage.
af438ac: CVE-2024-42416, Rejects SCSI service actions of 32 or greater before indexing the fixed second dimension of ctl_cmd_table.

Additionally, Codex identified in the last 5 years of commits there were ~33 commits that mentioned fixing fixed size array issues, some of the more interesting ones to me are:

47e9c81: Limits decoded AUTH_SYS supplemental groups to XU_NGROUPS, preventing writes beyond the fixed struct xucred group array.
411c28b: Starts hashalloc()'s reverse scan at nitems(primes) - 1 instead of indexing the fixed primes table one past its end.
4ece799: Rejects INVALID_EVTCHN before deriving indices into fixed Xen shared-info event-channel bit arrays.

Indicating how it could have identified potential security issues, along with assisting developers earlier.

print_trap_stats.txt shows the files, functions, and lines where instrumentation was inserted to ensure no out of bounds fixed array accesses occurr. There were 15,387 inserted array bounds checks across 10,281 files / 5,652 functions. print_trap_stats.sh can reproduce that output as needed. If there is interest in merging this, I can take a look at options to reduce the volume of inserted checks.

For those who wish to play with this change, At the time of writing, a small modification is required in sys/kern/vfs_bio.c to address an out of bounds issue. Apart from this, the kernel will boot fine, and is able to run regression tests without issue.

diff --git a/sys/kern/vfs_bio.c b/sys/kern/vfs_bio.c
index cb45ca7c75b7..5f83f812f430 100644
--- a/sys/kern/vfs_bio.c
+++ b/sys/kern/vfs_bio.c
@@ -2032,16 +2032,13 @@ bd_flushall(struct bufdomain *bd)
 static void
 bq_insert(struct bufqueue *bq, struct buf *bp, bool unlock)
 {
-       struct bufdomain *bd;
-
        if (bp->b_qindex != QUEUE_NONE)
                panic("bq_insert: free buffer %p onto another queue?", bp);

-       bd = bufdomain(bp);
        if (bp->b_flags & B_AGE) {
                /* Place this buf directly on the real queue. */
                if (bq->bq_index == QUEUE_CLEAN)
-                       bq = bd->bd_cleanq;
+                       bq = bufdomain(bp)->bd_cleanq;
                BQ_LOCK(bq);
                TAILQ_INSERT_HEAD(&bq->bq_queue, bp, b_freelist);
        } else {
@@ -2054,6 +2051,9 @@ bq_insert(struct bufqueue *bq, struct buf *bp, bool unlock)
        bp->b_subqueue = bq->bq_subqueue;

        if (bp->b_qindex == QUEUE_CLEAN) {
+               struct bufdomain *bd;
+
+               bd = bufdomain(bp);
                /*
                 * Flush the per-cpu queue and notify any waiters.
                 *

Submitted as part of the OpenAI Patch The Planet effort by Calif.io

[github-actions]

Thank you for taking the time to contribute to FreeBSD!

There is an issue that needs to be resolved:

• Missing Signed-off-by lines (df0f869)

Note

Please review CONTRIBUTING.md, then update and push your branch again.

[markjdb]

Thanks! I submitted the buffer cache patch for review: https://reviews.freebsd.org/D59381