[PATCH v2] fs: add NULL check in drop_buffers() to prevent null-ptr-deref

[Deepak Karn]

On Wed, Dec 10, 2025 at 3:25 PM Jan Kara <jack at suse.cz> wrote:
>
> On Tue 09-12-25 22:00:04, Deepak Karn wrote:
> > On Tue, Dec 9, 2025 at 4:48 PM Jan Kara <jack at suse.cz> wrote:
> > >
> > > On Tue 09-12-25 01:43:33, Deepakkumar Karn wrote:
> > > > On Mon, 8 Dec 2025 11:30:24 -0800, Darrick J. Wong wrote:
> > > > > > drop_buffers() dereferences the buffer_head pointer returned by
> > > > > > folio_buffers() without checking for NULL. This leads to a null pointer
> > > > > > dereference when called from try_to_free_buffers() on a folio with no
> > > > > > buffers attached. This happens when filemap_release_folio() is called on
> > > > > > a folio belonging to a mapping with AS_RELEASE_ALWAYS set but without
> > > > > > release_folio address_space operation defined. In such case,
> > > >
> > > > > What user is that?  All the users of AS_RELEASE_ALWAYS in 6.18 appear to
> > > > > supply a ->release_folio.  Is this some new thing in 6.19?
> > > >
> > > > AFS directories SET AS_RELEASE_ALWAYS but have not .release_folio.
> > >
> > > AFAICS AFS sets AS_RELEASE_ALWAYS only for symlinks but not for
> > > directories? Anyway I agree AFS symlinks will have AS_RELEASE_ALWAYS but no
> > > .release_folio callback. And this looks like a bug in AFS because AFAICT
> > > there's no point in setting AS_RELEASE_ALWAYS when you don't have
> > > .release_folio callback. Added relevant people to CC.
> > >
> > >                                                                 Honza
> >
> > Thank you for your response Jan. As you suggested, the bug is in AFS.
> > Can we include this current defensive check in drop_buffers() and I can submit
> > another patch to handle that bug of AFS we discussed?
>
> I'm not strongly opposed to that (although try_to_free_buffers() would seem
> like a tad bit better place) but overall I don't think it's a great idea as
> it would hide bugs. But perhaps with WARN_ON_ONCE() (to catch sloppy
> programming) it would be a sensible hardening.
>

Thanks Jan for your response. As suggested, adding WARN_ON_ONCE()
will be more sensible.
I just wanted to clarify my understanding, you are suggesting adding
WARN_ON_ONCE()
in try_to_free_buffers() as this highlights the issue and also solves
the concern. If my
understanding is wrong please let me know, I will share the updated patch.

> Also I think mapping_set_release_always() should assert that
> mapping->a_ops->release_folio is non-NULL to catch the problem early (once
> you fix the AFS bug).
>

I will address this in another patch for AFS.

Regards,
Deepakkumar Karn