* [PATCH v2] fs: add NULL check in drop_buffers() to prevent null-ptr-deref
@ 2025-12-08 19:03 Deepakkumar Karn
2025-12-08 19:30 ` Darrick J. Wong
0 siblings, 1 reply; 11+ messages in thread
From: Deepakkumar Karn @ 2025-12-08 19:03 UTC (permalink / raw)
To: Alexander Viro
Cc: Christian Brauner, Jan Kara, linux-fsdevel, linux-kernel,
syzbot+e07658f51ca22ab65b4e, syzkaller-bugs, Deepakkumar Karn
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,
folio_needs_release() returns true because of AS_RELEASE_ALWAYS flag,
the folio has no private buffer data, causing the try_to_free_buffers()
with a folio that has no buffers.
Adding NULL check for the buffer_head pointer and return false early if
no buffers are attached to the folio.
Reported-by: syzbot+e07658f51ca22ab65b4e@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=e07658f51ca22ab65b4e
Fixes: 6439476311a6 ("fs: Convert drop_buffers() to use a folio")
Signed-off-by: Deepakkumar Karn <dkarn@redhat.com>
---
fs/buffer.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/buffer.c b/fs/buffer.c
index 838c0c571022..fa5de0cdf540 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -2893,6 +2893,10 @@ drop_buffers(struct folio *folio, struct buffer_head **buffers_to_free)
struct buffer_head *head = folio_buffers(folio);
struct buffer_head *bh;
+ /* In cases of folio without buffer_head*/
+ if (!head)
+ return false;
+
bh = head;
do {
if (buffer_busy(bh))
--
2.52.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v2] fs: add NULL check in drop_buffers() to prevent null-ptr-deref 2025-12-08 19:03 [PATCH v2] fs: add NULL check in drop_buffers() to prevent null-ptr-deref Deepakkumar Karn @ 2025-12-08 19:30 ` Darrick J. Wong 2025-12-08 20:13 ` Deepakkumar Karn 0 siblings, 1 reply; 11+ messages in thread From: Darrick J. Wong @ 2025-12-08 19:30 UTC (permalink / raw) To: Deepakkumar Karn Cc: Alexander Viro, Christian Brauner, Jan Kara, linux-fsdevel, linux-kernel, syzbot+e07658f51ca22ab65b4e, syzkaller-bugs On Tue, Dec 09, 2025 at 12:33:07AM +0530, Deepakkumar Karn 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? --D > folio_needs_release() returns true because of AS_RELEASE_ALWAYS flag, > the folio has no private buffer data, causing the try_to_free_buffers() > with a folio that has no buffers. > > Adding NULL check for the buffer_head pointer and return false early if > no buffers are attached to the folio. > > Reported-by: syzbot+e07658f51ca22ab65b4e@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=e07658f51ca22ab65b4e > Fixes: 6439476311a6 ("fs: Convert drop_buffers() to use a folio") > Signed-off-by: Deepakkumar Karn <dkarn@redhat.com> > --- > fs/buffer.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/fs/buffer.c b/fs/buffer.c > index 838c0c571022..fa5de0cdf540 100644 > --- a/fs/buffer.c > +++ b/fs/buffer.c > @@ -2893,6 +2893,10 @@ drop_buffers(struct folio *folio, struct buffer_head **buffers_to_free) > struct buffer_head *head = folio_buffers(folio); > struct buffer_head *bh; > > + /* In cases of folio without buffer_head*/ > + if (!head) > + return false; > + > bh = head; > do { > if (buffer_busy(bh)) > -- > 2.52.0 > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] fs: add NULL check in drop_buffers() to prevent null-ptr-deref 2025-12-08 19:30 ` Darrick J. Wong @ 2025-12-08 20:13 ` Deepakkumar Karn 2025-12-09 11:18 ` Jan Kara 0 siblings, 1 reply; 11+ messages in thread From: Deepakkumar Karn @ 2025-12-08 20:13 UTC (permalink / raw) To: djwong Cc: brauner, dkarn, jack, linux-fsdevel, linux-kernel, syzbot+e07658f51ca22ab65b4e, syzkaller-bugs, viro 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] fs: add NULL check in drop_buffers() to prevent null-ptr-deref 2025-12-08 20:13 ` Deepakkumar Karn @ 2025-12-09 11:18 ` Jan Kara 2025-12-09 16:30 ` Deepak Karn 0 siblings, 1 reply; 11+ messages in thread From: Jan Kara @ 2025-12-09 11:18 UTC (permalink / raw) To: Deepakkumar Karn Cc: djwong, brauner, jack, linux-fsdevel, linux-kernel, syzbot+e07658f51ca22ab65b4e, syzkaller-bugs, viro, David Howells, linux-afs 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 -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] fs: add NULL check in drop_buffers() to prevent null-ptr-deref 2025-12-09 11:18 ` Jan Kara @ 2025-12-09 16:30 ` Deepak Karn 2025-12-10 9:55 ` Jan Kara 0 siblings, 1 reply; 11+ messages in thread From: Deepak Karn @ 2025-12-09 16:30 UTC (permalink / raw) To: Jan Kara Cc: djwong, brauner, linux-fsdevel, linux-kernel, syzbot+e07658f51ca22ab65b4e, syzkaller-bugs, viro, David Howells, linux-afs On Tue, Dec 9, 2025 at 4:48 PM Jan Kara <jack@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? Regards, Deepakkumar Karn ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] fs: add NULL check in drop_buffers() to prevent null-ptr-deref 2025-12-09 16:30 ` Deepak Karn @ 2025-12-10 9:55 ` Jan Kara 2025-12-10 15:29 ` Deepak Karn 0 siblings, 1 reply; 11+ messages in thread From: Jan Kara @ 2025-12-10 9:55 UTC (permalink / raw) To: Deepak Karn Cc: Jan Kara, djwong, brauner, linux-fsdevel, linux-kernel, syzbot+e07658f51ca22ab65b4e, syzkaller-bugs, viro, David Howells, linux-afs On Tue 09-12-25 22:00:04, Deepak Karn wrote: > On Tue, Dec 9, 2025 at 4:48 PM Jan Kara <jack@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. 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). Honza -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] fs: add NULL check in drop_buffers() to prevent null-ptr-deref 2025-12-10 9:55 ` Jan Kara @ 2025-12-10 15:29 ` Deepak Karn 2025-12-10 17:23 ` Jan Kara 0 siblings, 1 reply; 11+ messages in thread From: Deepak Karn @ 2025-12-10 15:29 UTC (permalink / raw) To: Jan Kara Cc: djwong, brauner, linux-fsdevel, linux-kernel, syzbot+e07658f51ca22ab65b4e, syzkaller-bugs, viro, David Howells, linux-afs On Wed, Dec 10, 2025 at 3:25 PM Jan Kara <jack@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@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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] fs: add NULL check in drop_buffers() to prevent null-ptr-deref 2025-12-10 15:29 ` Deepak Karn @ 2025-12-10 17:23 ` Jan Kara 2025-12-10 17:50 ` Deepak Karn 0 siblings, 1 reply; 11+ messages in thread From: Jan Kara @ 2025-12-10 17:23 UTC (permalink / raw) To: Deepak Karn Cc: Jan Kara, djwong, brauner, linux-fsdevel, linux-kernel, syzbot+e07658f51ca22ab65b4e, syzkaller-bugs, viro, David Howells, linux-afs On Wed 10-12-25 20:59:00, Deepak Karn wrote: > On Wed, Dec 10, 2025 at 3:25 PM Jan Kara <jack@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@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. Yes, I meant something like: if (WARN_ON_ONCE(!folio_buffers(folio))) return true; in try_to_free_buffers(). > > 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. Thanks. As a separate patch please :) Honza -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] fs: add NULL check in drop_buffers() to prevent null-ptr-deref 2025-12-10 17:23 ` Jan Kara @ 2025-12-10 17:50 ` Deepak Karn 2025-12-11 9:15 ` Jan Kara 0 siblings, 1 reply; 11+ messages in thread From: Deepak Karn @ 2025-12-10 17:50 UTC (permalink / raw) To: Jan Kara Cc: djwong, brauner, linux-fsdevel, linux-kernel, syzbot+e07658f51ca22ab65b4e, syzkaller-bugs, viro, David Howells, linux-afs On Wed, Dec 10, 2025 at 10:54 PM Jan Kara <jack@suse.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@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@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. > > 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. > > > 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. > > Thanks. As a separate patch please :) > > Honza Yes. Regards, Deepak Karn ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] fs: add NULL check in drop_buffers() to prevent null-ptr-deref 2025-12-10 17:50 ` Deepak Karn @ 2025-12-11 9:15 ` Jan Kara 2025-12-11 12:50 ` Deepak Karn 0 siblings, 1 reply; 11+ messages in thread From: Jan Kara @ 2025-12-11 9:15 UTC (permalink / raw) To: Deepak Karn Cc: Jan Kara, djwong, brauner, linux-fsdevel, linux-kernel, syzbot+e07658f51ca22ab65b4e, syzkaller-bugs, viro, David Howells, linux-afs On Wed 10-12-25 23:20:23, Deepak Karn wrote: > On Wed, Dec 10, 2025 at 10:54 PM Jan Kara <jack@suse.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@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@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. > > > > 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. Honza -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] fs: add NULL check in drop_buffers() to prevent null-ptr-deref 2025-12-11 9:15 ` Jan Kara @ 2025-12-11 12:50 ` Deepak Karn 0 siblings, 0 replies; 11+ messages in thread From: Deepak Karn @ 2025-12-11 12:50 UTC (permalink / raw) To: Jan Kara Cc: djwong, brauner, linux-fsdevel, linux-kernel, syzbot+e07658f51ca22ab65b4e, syzkaller-bugs, viro, David Howells, linux-afs On Thu, Dec 11, 2025 at 2:46 PM Jan Kara <jack@suse.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@suse.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@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@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. > > > > > > 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-12-11 12:51 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-12-08 19:03 [PATCH v2] fs: add NULL check in drop_buffers() to prevent null-ptr-deref Deepakkumar Karn 2025-12-08 19:30 ` Darrick J. Wong 2025-12-08 20:13 ` Deepakkumar Karn 2025-12-09 11:18 ` Jan Kara 2025-12-09 16:30 ` Deepak Karn 2025-12-10 9:55 ` Jan Kara 2025-12-10 15:29 ` Deepak Karn 2025-12-10 17:23 ` Jan Kara 2025-12-10 17:50 ` Deepak Karn 2025-12-11 9:15 ` Jan Kara 2025-12-11 12:50 ` Deepak Karn
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox