Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
MIME-Version: 1.0
References: <Yl/qa2w3q9kyXcQl@elver.google.com> <20220421083715.45380-1-huangshaobo6@huawei.com>
In-Reply-To: <20220421083715.45380-1-huangshaobo6@huawei.com>
From:   Marco Elver <elver@google.com>
Date:   Thu, 21 Apr 2022 10:50:10 +0200
Message-ID: <CANpmjNMAT_DaiOoz=k6Z13nVR_2A_5fck12h0JKQSmNQRSKwGg@mail.gmail.com>
Subject: Re: [PATCH] kfence: check kfence canary in panic and reboot
To:     Shaobo Huang <huangshaobo6@huawei.com>
Cc:     akpm@linux-foundation.org, chenzefeng2@huawei.com,
        dvyukov@google.com, glider@google.com, kasan-dev@googlegroups.com,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        nixiaoming@huawei.com, wangbing6@huawei.com,
        wangfangpeng1@huawei.com, young.liuyang@huawei.com,
        zengweilin@huawei.com, zhongjubin@huawei.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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.