Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] kcov: NULL deref in __sanitizer_cov_trace_pc() during selftest() --
commit 2f2cd36073f62980c1305efe82a4239b11c076f5 Author: Yunseong Kim <[email protected]> kcov: collect early boot coverage of init_task This commit adds CONFIG_EARLY_KCOV to record code coverage of init_task from right after early parameter parsing in start_kernel() until the start of rest_init(), and exposes the collected PCs via /sys/kernel/debug/early_kcov. It also introduces CONFIG_KCOV_INSTRUMENT_NOISY and a kcov-noisy kbuild helper to instrument core subsystems normally excluded due to coverage noise. > diff --git a/kernel/Makefile b/kernel/Makefile > index 4ff92e24680c5..b9e4f06e02ad0 100644 > --- a/kernel/Makefile > +++ b/kernel/Makefile > @@ -27,14 +27,14 @@ endif > > # Prevents flicker of uninteresting __do_softirq()/__local_bh_disable_ip() > # in coverage traces. > -KCOV_INSTRUMENT_softirq.o := n > +KCOV_INSTRUMENT_softirq.o := $(kcov-noisy) [ ... ] > diff --git a/kernel/locking/Makefile b/kernel/locking/Makefile > index cee1901d4cff8..9041cbbec9aae 100644 > --- a/kernel/locking/Makefile > +++ b/kernel/locking/Makefile > @@ -1,7 +1,7 @@ > # SPDX-License-Identifier: GPL-2.0 > # Any varying coverage in these files is non-deterministic > # and is generally not a function of system call inputs. > -KCOV_INSTRUMENT := n > +KCOV_INSTRUMENT := $(kcov-noisy) [ ... ] > diff --git a/kernel/rcu/Makefile b/kernel/rcu/Makefile > index 0cfb009a99b9f..fd23c1dad641b 100644 > --- a/kernel/rcu/Makefile > +++ b/kernel/rcu/Makefile > @@ -1,7 +1,7 @@ > # SPDX-License-Identifier: GPL-2.0 > # Any varying coverage in these files is non-deterministic > # and is generally not a function of system call inputs. > -KCOV_INSTRUMENT := n > +KCOV_INSTRUMENT := $(kcov-noisy) [ ... ] > diff --git a/kernel/sched/Makefile b/kernel/sched/Makefile > index b1f1a367034ff..4b4db5cca9b08 100644 > --- a/kernel/sched/Makefile > +++ b/kernel/sched/Makefile > @@ -9,7 +9,7 @@ ccflags-y += $(call cc-disable-warning, > unused-but-set-variable) > > # These files are disabled because they produce non-interesting flaky > coverage > # that is not a function of syscall inputs. E.g. involuntary context > switches. > -KCOV_INSTRUMENT := n > +KCOV_INSTRUMENT := $(kcov-noisy) [ ... ] > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index 6759087154edd..40bf654ec7515 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -2233,6 +2233,51 @@ config KCOV_INSTRUMENT_ALL > filesystem fuzzing with AFL) then you will want to enable coverage > for more specific subsets of files, and should say n here. > [ ... ] > +config KCOV_INSTRUMENT_NOISY > + bool "Also instrument code excluded only for noisy coverage" > + depends on EARLY_KCOV && KCOV_INSTRUMENT_ALL > + help > + Some code (scheduler, RCU, locking, page and slab allocators, > + rbtree, stack unwinding, ...) is excluded from KCOV instrumentation > + because its coverage is not a function of system call inputs and > + only adds noise to coverage-guided fuzzing. Code that is excluded > + because it cannot be instrumented safely is not affected. > + > + Say y to instrument the noisy code as well, so that the early boot > + coverage shows the complete execution path. This degrades the > + coverage signal for syscall fuzzing, so say n for fuzzing kernels. [Severity: High] Can enabling CONFIG_KCOV_INSTRUMENT_NOISY trigger a NULL pointer dereference in __sanitizer_cov_trace_pc() during kcov_init() -> selftest(), as well as record interrupt entry and exit PCs into task coverage buffers? In kernel/kcov.c, check_kcov_mode() filters out interrupt context using in_task(), which relies on HARDIRQ_OFFSET being set in preempt_count(). However, several functions in kernel/softirq.c, kernel/sched/core.c, kernel/locking/lockdep.c, and kernel/rcu/ run on every hardware interrupt either before HARDIRQ_OFFSET is added or after it is subtracted, while in_task() is still true: - In kernel/softirq.c, irq_enter_rcu() and irq_enter() are not marked noinstr or __no_sanitize_coverage, so the compiler inserts a call to __sanitizer_cov_trace_pc() at function entry before __irq_enter_raw() adds HARDIRQ_OFFSET to preempt_count(). Likewise, __irq_exit_rcu() (inlined into irq_exit_rcu() and irq_exit()) calls preempt_count_sub(HARDIRQ_OFFSET) before finishing execution. - When CONFIG_DEBUG_PREEMPT=y or CONFIG_TRACE_PREEMPT_TOGGLE=y, __irq_enter_raw() calls preempt_count_add(HARDIRQ_OFFSET) in kernel/sched/core.c, which is instrumented at entry before __preempt_count_add() updates preempt_count(). - On interrupt exit after HARDIRQ_OFFSET is cleared, irqentry_exit_to_kernel_mode() / irqentry_exit_cond_resched() can call trace_hardirqs_on() -> lockdep_hardirqs_on_prepare() in kernel/locking/lockdep.c, rcu_irq_exit_check_preempt() in kernel/rcu/, and preempt_schedule_irq() -> __schedule() in kernel/sched/core.c (before prepare_task_switch() sets KCOV_IN_CTXSW and after kcov_finish_switch() clears it). When CONFIG_KCOV_SELFTEST=y is also enabled (for example in allyesconfig builds), kcov_init() calls selftest() at device_initcall time with current->kcov_area == NULL and current->kcov_size == 0: kernel/kcov.c:selftest() { ... start = jiffies; WRITE_ONCE(current->kcov_mode, KCOV_MODE_TRACE_PC); while ((jiffies - start) * MSEC_PER_SEC / HZ < 300) ; WRITE_ONCE(current->kcov_mode, 0); } When a timer interrupt fires during that loop, the instrumented interrupt entry/exit path invokes __sanitizer_cov_trace_pc() while in_task() is true: kernel/kcov.c:__sanitizer_cov_trace_pc() { ... t = current; if (!check_kcov_mode(KCOV_MODE_TRACE_PC, t)) return; area = t->kcov_area; /* The first 64-bit word is the number of subsequent PCs. */ pos = READ_ONCE(area[0]) + 1; if (likely(pos < t->kcov_size)) { ... } Because area[0] is read before checking t->kcov_size, dereferencing area when t->kcov_area is NULL crashes the kernel on boot. Should these interrupt entry/exit helpers be marked __no_sanitize_coverage (or noinstr), or should CONFIG_KCOV_SELFTEST depend on !KCOV_INSTRUMENT_NOISY? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
