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.
