Re: [PATCH v3 3/3] kasan: allow freed user page poisoning to be disabled with HW tags

[Peter Collingbourne <pcc@google.com>]

On Wed, May 26, 2021 at 3:45 AM Jann Horn <jannh@google.com> wrote:
>
> On Wed, May 26, 2021 at 12:07 AM Andrey Konovalov <andreyknvl@gmail.com> wrote:
> > On Wed, May 12, 2021 at 11:09 PM Peter Collingbourne <pcc@google.com> wrote:
> > > Poisoning freed pages protects against kernel use-after-free. The
> > > likelihood of such a bug involving kernel pages is significantly higher
> > > than that for user pages. At the same time, poisoning freed pages can
> > > impose a significant performance cost, which cannot always be justified
> > > for user pages given the lower probability of finding a bug. Therefore,
> > > make it possible to configure the kernel to disable freed user page
> > > poisoning when using HW tags via the new kasan.skip_user_poison_on_free
> > > command line option.
> >
> > So the potential scenario that would be undetectable with
> > kasan.skip_user_poison_on_free enabled is: 1) kernel allocates a user
> > page and maps it for userspace, 2) the page gets freed in the kernel,
> > 3) kernel accesses the page leading to a use-after-free. Is this
> > correct?

Yes, that's correct.

> > If bugs involving use-after-free accesses on user pages is something
> > that is extremely rare, perhaps we could just change the default and
> > avoid adding a command line switch.
> >
> > Jann, maybe you have an idea of how common something like this is or
> > have other inputs?
>
> GFP_USER is kind of a squishy concept, and if you grep around for it
> in the kernel tree, you can see it being used for all kinds of things
> - including SKBs in some weird ISDN driver, various types of BPF
> allocations, and so on. It's probably the wrong flag to hook if you
> want something that means "these pages will mostly be accessed from
> userspace".
>
> My guess is that what pcc@ is actually interested in are probably
> mainly anonymous pages, and to a lesser degree also page cache pages?
> Those use the more specific GFP_HIGHUSER_MOVABLE (which indicates that
> the kernel will usually not be holding any direct pointers to the page
> outside of rmap/pagecache logic, and that any kernel access to the
> pages will be using the kmap API).
>
> It's probably safe to assume that the majority of kernel bugs won't
> directly involve GFP_HIGHUSER_MOVABLE memory - that's probably mostly
> only going to happen if there are bugs in code that grabs pages with
> get_user_pages* and then kmap()s them, or if there's something broken
> in the pipe logic, or maybe an OOB issue in filesystem parsing code
> (?), or something like that.

This makes sense to me. The pages that I'm most interested in getting
this treatment are indeed the anonymous and, to a lesser extent,
pagecache pages.

The GFP_HIGHUSER_MOVABLE restrictions to me imply a lack of direct
kernel access more strongly than GFP_USER, as you point out. Therefore
I agree with Andrey that we probably don't need a switch for this and
can just change the behavior for GFP_HIGHUSER_MOVABLE. I've done so in
v4, although this required a preparatory patch to merge
__alloc_zeroed_user_highpage() and
alloc_zeroed_user_highpage_movable() so that we actually use the
GFP_HIGHUSER_MOVABLE constant for anonymous pages.

> Peter, is the plan to have this poisoning disabled in production? Is
> there an estimate on slow this is?

Yes, that is the plan. I don't think I'm at liberty to provide exact
numbers, but given that the previous patches mean that at allocation
time we only touch pages once whether or not KASAN is enabled, the
most significant remaining KASAN-associated source of overhead for
user pages is the deallocation time overhead that I'm eliminating in
this patch.

Peter