Re: [PATCH] kfence: check kfence canary in panic and reboot

[Shaobo Huang <huangshaobo6@huawei.com>]

On Thu, 21 Apr 2022 10:50:10 +0200, Marco Elver wrote:
> On Thu, 21 Apr 2022 at 10:37, Shaobo Huang <huangshaobo6@huawei.com> wrote:
> [...]
> > > >  static int __init kfence_debugfs_init(void)
> > > >  {
> > > >     struct dentry *kfence_dir = debugfs_create_dir("kfence", NULL);
> > > > @@ -806,6 +832,8 @@ static void kfence_init_enable(void)
> > > >
> > > >     WRITE_ONCE(kfence_enabled, true);
> > > >     queue_delayed_work(system_unbound_wq, &kfence_timer, 0);
> > > > +   register_reboot_notifier(&kfence_check_canary_notifier);
> > > > +   atomic_notifier_chain_register(&panic_notifier_list, &kfence_check_canary_notifier);
> > >
> > > Executing this on panic is reasonable. However,
> > > register_reboot_notifier() tells me this is being executed on *every*
> > > reboot (not just panic). I think that's not what we want, because that
>> > may increase reboot latency depending on how many KFENCE objects we
> > > have. Is it possible to *only* do the check on panic?
> >
> > if oob occurs before reboot, reboot can also detect it, if not, the detection will be missing in this scenario.
> > reboot and panic are two scenarios of system reset, so I think both scenarios need to be added.
> 
> That doesn't quite answer my question, why do you want to run the
> check during normal reboot? As I understand it right now it will run
> on any normal reboot, and also on panics. I have concerns adding these
> checks to normal reboots because it may increase normal reboot
> latency, which we do not want.

as you said, the detection will indeed increase normal reboot latency, and the
detection of normal reboot is not very meaningful. considering the cost and benefit,
I agree with your suggestion to only detect in panic.

thanks,
ShaoBo Huang