Re: [PATCH] kfence: add kfence.fault parameter

[Marco Elver]

On Tue, 3 Mar 2026 at 12:20, Alexander Potapenko <glider@xxxxxxxxxx> 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.