Re: [PATCH RFC v1 23/26] kmsan: unpoisoning buffers from devices etc.

[Alexander Potapenko <glider@google.com>]

On Fri, Oct 18, 2019 at 6:22 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Fri, Oct 18, 2019 at 11:43:01AM +0200, glider@google.com wrote:
> > When data is copied to memory from a device KMSAN should treat it as
> > initialized. In most cases it's enough to just unpoison the buffer that
> > is known to come from a device.
> > In the case with __do_page_cache_readahead() and bio_copy_user_iov() we
> > have to mark the whole pages as ignored by KMSAN, as it's not obvious
> > where these pages are read again.
>
> ...
>
> > +++ b/mm/filemap.c
> > @@ -18,6 +18,7 @@
> >  #include <linux/uaccess.h>
> >  #include <linux/capability.h>
> >  #include <linux/kernel_stat.h>
> > +#include <linux/kmsan-checks.h>
> >  #include <linux/gfp.h>
> >  #include <linux/mm.h>
> >  #include <linux/swap.h>
> > @@ -2810,6 +2811,8 @@ static struct page *do_read_cache_page(struct address_space *mapping,
> >               page = wait_on_page_read(page);
> >               if (IS_ERR(page))
> >                       return page;
> > +             /* Assume all pages in page cache are initialized. */
> > +             kmsan_unpoison_shadow(page_address(page), PAGE_SIZE);
>
> Why would you do that?  The page cache already keeps track of which
> pages are initialised -- the PageUptodate flag is set on them.  Indeed,
> just adding a kmsan call to SetPageUptodate and __SetPageUptodate would
> probably be a very straightforward way of handling things, and probably
> means you can get rid of a lot of these other calls.
This seems like a very good thing to do, I'll definitely try that.
I however noticed that __SetPageUptodate is used when copying pages,
not during disk I/O. Is that really so?

We basically need the following behavior: if a device writes to a
page, the contents of that page are considered initialized.
However when the kernel copies one page to another, we must explicitly
copy the source shadow page to the destination.

On a related note, there seems to be a PG_dirty bit that indicates the
page is to be flushed to disk.
What's the best place to check such pages for being initialized, so
that we can also report writes of uninitialized data to the disk?


-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg