linux-kernel - Re: [PATCH v2] fs: add NULL check in drop

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAO4qAqJAgO_3tGmoTie2XpCJvHfbZawaOddzdibDkGB9+R+PQw@mail.gmail.com>
Date: Thu, 11 Dec 2025 18:20:56 +0530
From: Deepak Karn <dkarn@...hat.com>
To: Jan Kara <jack@...e.cz>
Cc: djwong@...nel.org, brauner@...nel.org, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, 
	syzbot+e07658f51ca22ab65b4e@...kaller.appspotmail.com, 
	syzkaller-bugs@...glegroups.com, viro@...iv.linux.org.uk, 
	David Howells <dhowells@...hat.com>, linux-afs@...ts.infradead.org
Subject: Re: [PATCH v2] fs: add NULL check in drop_buffers() to prevent null-ptr-deref

On Thu, Dec 11, 2025 at 2:46 PM Jan Kara <jack@...e.cz> wrote:
>
> On Wed 10-12-25 23:20:23, Deepak Karn wrote:
> > On Wed, Dec 10, 2025 at 10:54 PM Jan Kara <jack@...e.cz> wrote:
> > >
> > > On Wed 10-12-25 20:59:00, Deepak Karn wrote:
> > > > On Wed, Dec 10, 2025 at 3:25 PM Jan Kara <jack@...e.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@...e.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.
> > >
> > > Yes, I meant something like:
> > >
> > >         if (WARN_ON_ONCE(!folio_buffers(folio)))
> > >                 return true;
> > >
> > > in try_to_free_buffers().
> > >
> >
> > Yes this is what I am also going for, just a minor change that I was
> > going to return false.
> >
> > Reason:
> > From the flow it should return false whenever failed to free buffers /
> > operation couldn't complete. Do you think otherwise.
>
> Well, it's rather we should return false when the page cannot be freed
> because we were unable to free the buffers. When folio_buffers() are NULL,
> the page *can* be freed so we should be returning true in that case.
>
Thank you Jan for your response and clarifying this. I will submit an
updated patch.

Regards,
Deepakkumar Karn