On Tue, Mar 3, 2026 at 4:23 PM Marco Elver <[email protected]> wrote: > > On Tue, 3 Mar 2026 at 12:20, Alexander Potapenko <[email protected]> wrote: > > > > > @@ -830,7 +835,8 @@ static void kfence_check_all_canary(void) > > > static int kfence_check_canary_callback(struct notifier_block *nb, > > > unsigned long reason, void *arg) > > > { > > > - kfence_check_all_canary(); > > > + if (READ_ONCE(kfence_enabled)) > > > + kfence_check_all_canary(); > > > > By the way, should we also check for kfence_enabled when reporting errors? > > Not sure, I think it might be redundant - I don't see a way we should > get to the reporting path if KFENCE is disabled. And if there > currently is a way to get there, we should check kfence_enabled before > (such as in this panic notifier now). > > > > @@ -1307,12 +1314,14 @@ bool kfence_handle_page_fault(unsigned long addr, > > > bool is_write, struct pt_regs > > > if (to_report) { > > > raw_spin_lock_irqsave(&to_report->lock, flags); > > > to_report->unprotected_page = unprotected_page; > > > - kfence_report_error(addr, is_write, regs, to_report, > > > error_type); > > > + fault = kfence_report_error(addr, is_write, regs, > > > to_report, error_type); > > > raw_spin_unlock_irqrestore(&to_report->lock, flags); > > > } else { > > > /* This may be a UAF or OOB access, but we can't be sure. > > > */ > > > - kfence_report_error(addr, is_write, regs, NULL, > > > KFENCE_ERROR_INVALID); > > > + fault = kfence_report_error(addr, is_write, regs, NULL, > > > KFENCE_ERROR_INVALID); > > > } > > > > > > + kfence_handle_fault(fault); > > > + > > > return kfence_unprotect(addr); /* Unprotect and let access > > > proceed. */ > > > > If kfence_handle_fault() oopses, kfence_unprotect() will never be > > called, is that the desired behavior? > > It is - consider multiple kernel threads running into the same OOB or > UAF. We should oops them all, otherwise this change is almost no > benefit. > > > > /* Require non-NULL meta, except if KFENCE_ERROR_INVALID. */ > > > if (WARN_ON(type != KFENCE_ERROR_INVALID && !meta)) > > > - return; > > > + return KFENCE_FAULT_NONE; > > > > We explicitly don't panic here; guess it should be fine... > > Yes - it's a KFENCE bug if we get here, the WARN is fine.
Reviewed-by: Alexander Potapenko <[email protected]>
