Re: [PATCH 2/3] md: Use folio APIs in free_page()

[Matthew Wilcox <willy@infradead.org>]

On Mon, Sep 14, 2026 at 04:31:29AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Memory leak of internal bitmap pages and uninitialized file bitmap pages during teardown or error handling due to an early return in free_buffers().
> --

Yes, I thought the early return looked weird.  I just believed that I
hadn't looked into the code enough to understand why it looked weird but
was right.  Since the AI says it's wrong, I'm happy to believe it's wrong.
It's probably not really a high seveirty problem because most RAIDs are
set up once and then never removed, so the leak doesn't matter.

The other thing that looks weird is that this code believes the b_this_page
list is NULL terminated.  Most loops over "all BHs attached to a folio"
have a circular list.  However, that is established by link_dev_buffers()
which is not called by md-bitmap.  So it's correct, even though weird.

The good news is this is an easy fix:

@@ -536,9 +536,6 @@ static void free_buffers(struct page *page)
        struct folio *folio = page_folio(page);
        struct buffer_head *bh = folio_detach_private(folio);

-       if (!bh)
-               return;
-
        while (bh) {
                struct buffer_head *next = bh->b_this_page;
                free_buffer_head(bh);

I can submit a new vrsion, or Zi can just pick this up.

> commit d68f524234e61e6567e52a27e09b9d2546ab54af
> Author: Matthew Wilcox (Oracle) <willy@infradead.org>
> 
> md: Use folio APIs in free_page()
> 
> This commit converts the page to a folio, removing some of the last uses of
> a few page APIs and replacing two calls to compound_head() with one.
> 
> > diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> > index 5f1637f974c15..7b42db87d536d 100644
> > --- a/drivers/md/md-bitmap.c
> > +++ b/drivers/md/md-bitmap.c
> > @@ -533,19 +533,18 @@ static void write_file_page(struct bitmap *bitmap, struct page *page, int wait)
> >  
> >  static void free_buffers(struct page *page)
> >  {
> > -	struct buffer_head *bh;
> > +	struct folio *folio = page_folio(page);
> > +	struct buffer_head *bh = folio_detach_private(folio);
> >  
> > -	if (!PagePrivate(page))
> > +	if (!bh)
> >  		return;
> 
> [Severity: High]
> This is a pre-existing issue, but this early return skips the folio_put()
> required to drop the base allocation reference for pages that do not have
> buffers attached.
> 
> For internal bitmaps, pages are allocated via alloc_page() with a refcount
> of 1 in md_bitmap_storage_alloc():
> 
> drivers/md/md-bitmap.c:md_bitmap_storage_alloc() {
> 	...
> 	store->filemap[pnum] = alloc_page(GFP_KERNEL|__GFP_ZERO);
> 	...
> }
> 
> During MD array teardown or failure, md_bitmap_file_unmap() calls
> free_buffers() on these pages:
> 
> drivers/md/md-bitmap.c:md_bitmap_file_unmap() {
> 	...
> 	free_buffers(map[pages]);
> 	...
> }
> 
> Because internal bitmap pages don't have buffers attached, (!bh) evaluates
> to true, triggering the early return and completely bypassing the folio_put()
> at the end of the function.
> 
> This permanently leaks the pages allocated for the internal bitmap's filemap
> and superblock.
> 
> Does this early return need to be updated so that folio_put() is still
> called for folios without buffers?
> 
> > -	bh = page_buffers(page);
> >  	while (bh) {
> >  		struct buffer_head *next = bh->b_this_page;
> >  		free_buffer_head(bh);
> >  		bh = next;
> >  	}
> > -	detach_page_private(page);
> > -	put_page(page);
> > +	folio_put(folio);
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260913-remove-pg_private-v4-0-848550f7574e@nvidia.com?part=2