| Message ID | 20191220184955.223741-41-glider@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add KernelMemorySanitizer infrastructure | expand |
On Fri, Dec 20, 2019 at 07:49:53PM +0100, glider@google.com 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(). > To prevent this, jump directly to the buffer_head-based read function in > KMSAN builds. > > Signed-off-by: Alexander Potapenko <glider@google.com> > Cc: "Theodore Ts'o" <tytso@mit.edu> > Cc: Andreas Dilger <adilger.kernel@dilger.ca> > Cc: Vegard Nossum <vegard.nossum@oracle.com> > Cc: Dmitry Vyukov <dvyukov@google.com> > Cc: Marco Elver <elver@google.com> > Cc: Andrey Konovalov <andreyknvl@google.com> > Cc: linux-mm@kvack.org > --- > > v4: > - use IS_ENABLED, as requested by Marco Elver > > Change-Id: I54ae8af536626a988e6398ff18a06c179b0ce034 > --- > fs/ext4/readpage.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/fs/ext4/readpage.c b/fs/ext4/readpage.c > index fef7755300c3..1e46ddf7a854 100644 > --- a/fs/ext4/readpage.c > +++ b/fs/ext4/readpage.c > @@ -252,6 +252,16 @@ int ext4_mpage_readpages(struct address_space *mapping, > if (page_has_buffers(page)) > goto confused; > > + /* > + * 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. > + */ > + if (IS_ENABLED(CONFIG_KMSAN)) > + goto confused; > + > 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) + > -- Why is ext4 special here? Wouldn't other filesystems need this too? Is it possible the problem is the page merging logic in bio_add_page()? Maybe we need something like: diff --git a/block/bio.c b/block/bio.c index a5d75f6bf4c7e..9449a1e571ee7 100644 --- a/block/bio.c +++ b/block/bio.c @@ -646,6 +646,8 @@ static inline bool page_is_mergeable(const struct bio_vec *bv, *same_page = ((vec_end_addr & PAGE_MASK) == page_addr); if (!*same_page && pfn_to_page(PFN_DOWN(vec_end_addr)) + 1 != page) return false; + if (!*same_page && IS_ENABLED(CONFIG_KMSAN)) + return false; return true; }
> Why is ext4 special here? Wouldn't other filesystems need this too? ext4 isn't special, it's just the FS we're using on syzbot. > Is it possible the problem is the page merging logic in bio_add_page()? > Maybe we need something like: > > diff --git a/block/bio.c b/block/bio.c > index a5d75f6bf4c7e..9449a1e571ee7 100644 > --- a/block/bio.c > +++ b/block/bio.c > @@ -646,6 +646,8 @@ static inline bool page_is_mergeable(const struct bio_vec *bv, > *same_page = ((vec_end_addr & PAGE_MASK) == page_addr); > if (!*same_page && pfn_to_page(PFN_DOWN(vec_end_addr)) + 1 != page) > return false; > + if (!*same_page && IS_ENABLED(CONFIG_KMSAN)) > + return false; > return true; > } > This one appears to work, thanks! I'll pick it for v5.
diff --git a/fs/ext4/readpage.c b/fs/ext4/readpage.c index fef7755300c3..1e46ddf7a854 100644 --- a/fs/ext4/readpage.c +++ b/fs/ext4/readpage.c @@ -252,6 +252,16 @@ int ext4_mpage_readpages(struct address_space *mapping, if (page_has_buffers(page)) goto confused; + /* + * 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. + */ + if (IS_ENABLED(CONFIG_KMSAN)) + goto confused; + 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) +
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(). To prevent this, jump directly to the buffer_head-based read function in KMSAN builds. Signed-off-by: Alexander Potapenko <glider@google.com> Cc: "Theodore Ts'o" <tytso@mit.edu> Cc: Andreas Dilger <adilger.kernel@dilger.ca> Cc: Vegard Nossum <vegard.nossum@oracle.com> Cc: Dmitry Vyukov <dvyukov@google.com> Cc: Marco Elver <elver@google.com> Cc: Andrey Konovalov <andreyknvl@google.com> Cc: linux-mm@kvack.org --- v4: - use IS_ENABLED, as requested by Marco Elver Change-Id: I54ae8af536626a988e6398ff18a06c179b0ce034 --- fs/ext4/readpage.c | 10 ++++++++++ 1 file changed, 10 insertions(+)