On Mon, Nov 25, 2019 at 5:05 PM Robin Murphy <robin.murphy@xxxxxxx> wrote: > > On 22/11/2019 11:26 am, glider@xxxxxxxxxx wrote: > > KMSAN doesn't allow treating adjacent memory pages as such, if they were > > allocated by different alloc_pages() calls. > > ext4_mpage_readpages() however does so: adjacent pages end up being passed > > together to dma_direct_map_sg(). > > Urgh, there are definitely more places where physically-contiguous pages > are coalesced into single scatterlist entries - see > sg_alloc_table_from_pages() for instance. I wouldn't be surprised if > there are further open-coded versions hiding out in various other > drivers/subsystems too. Unless I've misunderstood, this seems like quite > an invasive limitation :( You're right. Other places haven't fired off so far, so I was just unaware of them, but I'm anticipating more. There are two possible solutions to that problem: 1. Allocate shadow and origin pages at fixed offset from the kernel page. This is what we already do for vmalloc, but not for page_alloc(), as it turned out to be quite hard. Ideas on how to implement this approach are still welcome, because it'll simplify the rest of the KMSAN runtime a lot. 2. Make all accesses touching non-contiguous pages access dummy shadow pages instead, so that such accesses don't produce any uninitialized values. This is quite controversial, as it may prevent true positives from being reported. > Robin. > > > To prevent this, jump directly to the buffer_head-based read function in > > KMSAN builds. > > > > Signed-off-by: Alexander Potapenko <glider@xxxxxxxxxx> > > Cc: "Theodore Ts'o" <tytso@xxxxxxx> > > Cc: Andreas Dilger <adilger.kernel@xxxxxxxxx> > > Cc: Vegard Nossum <vegard.nossum@xxxxxxxxxx> > > Cc: Dmitry Vyukov <dvyukov@xxxxxxxxxx> > > Cc: linux-mm@xxxxxxxxx > > --- > > > > Change-Id: I54ae8af536626a988e6398ff18a06c179b0ce034 > > --- > > fs/ext4/readpage.c | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/fs/ext4/readpage.c b/fs/ext4/readpage.c > > index a30b203fa461..a3bb9e3ce5de 100644 > > --- a/fs/ext4/readpage.c > > +++ b/fs/ext4/readpage.c > > @@ -252,6 +252,17 @@ int ext4_mpage_readpages(struct address_space *mapping, > > if (page_has_buffers(page)) > > goto confused; > > > > +#if defined(CONFIG_KMSAN) > > + /* > > + * The following code may treat adjacent pages allocated > > + * separately as a bigger contiguous allocation. > > + * KMSAN doesn't allow this, as the corresponding metadata > > + * pages may be separated. > > + * Skip this complex logic for KMSAN builds. > > + */ > > + goto confused; > > +#endif > > + > > block_in_file = (sector_t)page->index << (PAGE_SHIFT - blkbits); > > last_block = block_in_file + nr_pages * blocks_per_page; > > last_block_in_file = (ext4_readpage_limit(inode) + > > -- 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
