Re: [PATCH v2 08/10] ceph: convert page cleanup loop in writepages_finish() to folios

Matthew Wilcox <[email protected]>
Newsgroups org.kernel.vger.ceph-devel,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel,org.kvack.linux-mm
Message-ID <[email protected]>
On Wed, Aug 05, 2026 at 02:33:34AM +0300, Tal Zussman wrote:
> >  		for (j = 0; j < num_pages; j++) {
> > +			struct folio *folio;
> > +
> >  			fscrypt_finalize_bounce_page(&osd_data->pages[j]);
> > -			page = osd_data->pages[j];
> > -			BUG_ON(!page);
> > -			WARN_ON(!PageUptodate(page));
> > +			folio = page_folio(osd_data->pages[j]);
> > +			BUG_ON(!folio);
> > +			WARN_ON(!folio_test_uptodate(folio));
> 
> Sashiko complains:
> 
> "Since page_folio() invokes _compound_head() which dereferences the page
> pointer, won't this cause a NULL pointer dereference before reaching the
> BUG_ON(!folio) check if osd_data->pages[j] is actually NULL?
> The previous code checked the page pointer before any dereference occurred.
> Would it be safer to check the array element for NULL before passing it to
> page_folio()?"
> 
> The BUG_ON can just be removed, as fscrypt_finalize_bounce_page() cannot take
> or set its argument to NULL as constructed, so the check was unnecessary
> to begin with.

Heh, Sashiko beat me to it ;-)

I'd agree, the BUG_ON was always unnecessary, and I was going to
recommend just deleting it.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.