Re: [PATCH v2] kfence: enable check kfence canary in panic via boot param

[Shaobo Huang <huangshaobo6@huawei.com>]

On Sun, 24 Apr 2022 15:31:42 +0200, Marco Elver <elver@google.com> wrote:
> On Sun, 24 Apr 2022 at 13:00, Shaobo Huang <huangshaobo6@huawei.com> wrote:
> >
> > From: huangshaobo <huangshaobo6@huawei.com>
> >
> > when writing out of bounds to the red zone, it can only be
> > detected at kfree. However, the system may have been reset
> > before freeing the memory, which would result in undetected
> > oob. Therefore, it is necessary to detect oob behavior in
> > panic. Since only the allocated mem call stack is available,
> > it may be difficult to find the oob maker. Therefore, this
> > feature is disabled by default and can only be enabled via
> > boot parameter.
> 
> This description is still not telling the full story or usecase. The
> story goes something like:
> """
> Out-of-bounds accesses that aren't caught by a guard page will result
> in corruption of canary memory. In pathological cases, where an object
> has certain alignment requirements, an out-of-bounds access might
> never be caught by the guard page. Such corruptions, however, are only
> detected on kfree() normally. If the bug causes the kernel to panic
> before kfree(), KFENCE has no opportunity to report the issue. Such
> corruptions may also indicate failing memory or other faults.
> 
> To provide some more information in such cases, add the option to
> check canary bytes on panic. This might help narrow the search for the
> panic cause; but, due to only having the allocation stack trace, such
> reports are difficult to use to diagnose an issue alone. In most
> cases, such reports are inactionable, and is therefore an opt-in
> feature (disabled by default).
> """
> 
> Please feel free to copy or take pieces above to complete the commit message.
>
> [...]
> >  #include <linux/slab.h>
> >  #include <linux/spinlock.h>
> >  #include <linux/string.h>
> > +#include <linux/notifier.h>
> > +#include <linux/panic_notifier.h>
> 
> Please keep these includes sorted alphabetically.
> 
> [...]
> > +/* If true, check kfence canary in panic. */
> 
> It should be "on panic". E.g. "If true, check all canary bytes on panic."
> 
> [...]
> > +/* === Panic Notifier ====================================================== */
> 
> Blank line between /* === ... */ and function.

thank you so much for your suggestion!

thanks,
ShaoBo Huang