Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] x86/kcov: instrumenting apic/ crashes selftest() on FRED timer entry --
commit c1145245b36a2aa7f3b165ab0f5b126b2c810470 Author: Yunseong Kim <[email protected]> x86/kcov: allow instrumenting noisy objects for early boot coverage This commit switches several x86 Makefile exclusions from hardcoded n to $(kcov-noisy) so that CONFIG_KCOV_INSTRUMENT_NOISY=y can collect early boot coverage from arch/x86/kernel/, arch/x86/kernel/apic/, and arch/x86/lib/. > diff --git a/arch/x86/kernel/apic/Makefile b/arch/x86/kernel/apic/Makefile > index 581db89477f92..146cbd766c924 100644 > --- a/arch/x86/kernel/apic/Makefile > +++ b/arch/x86/kernel/apic/Makefile > @@ -5,7 +5,7 @@ > > # Leads to non-deterministic coverage that is not a function of syscall > inputs. > # In particular, smp_apic_timer_interrupt() is called in random places. > -KCOV_INSTRUMENT := n > +KCOV_INSTRUMENT := $(kcov-noisy) [Severity: High] Does instrumenting arch/x86/kernel/apic/apic.o cause a NULL pointer dereference during boot when CONFIG_KCOV_INSTRUMENT_NOISY=y and CONFIG_KCOV_SELFTEST=y on FRED-enabled x86_64 systems? In arch/x86/kernel/apic/apic.c, DEFINE_IDTENTRY_SYSVEC(sysvec_apic_timer_interrupt) expands via arch/x86/include/asm/idtentry.h: void fred_##func(struct pt_regs *regs) { instr_##func (regs); } Unlike the IDT entry wrapper sysvec_apic_timer_interrupt(), fred_sysvec_apic_timer_interrupt() is not marked noinstr or __no_sanitize_coverage. When apic.o is instrumented with KCOV, fred_sysvec_apic_timer_interrupt() and the inlined instr_sysvec_apic_timer_interrupt() -> run_sysvec_on_irqstack_cond() -> call_on_irqstack_cond() call into __sanitizer_cov_trace_pc() before irq_enter_rcu() adds HARDIRQ_OFFSET to preempt_count (and again after irq_exit_rcu() removes HARDIRQ_OFFSET). During boot, after rest_init() -> early_kcov_stop() clears init_task.kcov_area to NULL, device_initcall(kcov_init) invokes selftest() in kernel/kcov.c: 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); While selftest() spins waiting for timer interrupts to advance jiffies with current->kcov_area == NULL, the first LAPIC timer interrupt under FRED enters fred_sysvec_apic_timer_interrupt() while in_task() in check_kcov_mode() is still true, reaching __sanitizer_cov_trace_pc() in kernel/kcov.c: area = t->kcov_area; /* The first 64-bit word is the number of subsequent PCs. */ pos = READ_ONCE(area[0]) + 1; and dereferencing NULL. In addition, during normal task KCOV tracing on FRED systems, if a LAPIC timer interrupt preempts a traced task inside __sanitizer_cov_trace_pc() or write_comp_data() between reading area[0] and writing back the updated count, can the nested KCOV calls in fred_sysvec_apic_timer_interrupt() have their area[0] updates overwritten when the interrupted context resumes and stores its stale index? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
