* [PATCH] fs/buffer: add alert in try_to_free_buffers() for folios without buffers
@ 2025-12-10 18:02 Deepakkumar Karn
2025-12-11 9:23 ` Jan Kara
0 siblings, 1 reply; 7+ messages in thread
From: Deepakkumar Karn @ 2025-12-10 18:02 UTC (permalink / raw)
To: Alexander Viro
Cc: Christian Brauner, Jan Kara, linux-kernel, linux-fsdevel,
Deepakkumar Karn
try_to_free_buffers() can be called on folios with no buffers attached
when filemap_release_folio() is invoked on a folio belonging to a mapping
with AS_RELEASE_ALWAYS set but no release_folio operation defined.
In such cases, folio_needs_release() returns true because of the
AS_RELEASE_ALWAYS flag, but the folio has no private buffer data. This
causes try_to_free_buffers() to call drop_buffers() on a folio with no
buffers, leading to a null pointer dereference.
Adding a check in try_to_free_buffers() to return early if the folio has no
buffers attached, with WARN_ON_ONCE() to alert about the misconfiguration.
This provides defensive hardening.
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..b229baa77055 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -2948,6 +2948,10 @@ bool try_to_free_buffers(struct folio *folio)
if (folio_test_writeback(folio))
return false;
+ /* Misconfigured folio check */
+ if (WARN_ON_ONCE(!folio_buffers(folio)))
+ return false;
+
if (mapping == NULL) { /* can this still happen? */
ret = drop_buffers(folio, &buffers_to_free);
goto out;
--
2.52.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] fs/buffer: add alert in try_to_free_buffers() for folios without buffers
2025-12-10 18:02 [PATCH] fs/buffer: add alert in try_to_free_buffers() for folios without buffers Deepakkumar Karn
@ 2025-12-11 9:23 ` Jan Kara
2025-12-11 13:12 ` [PATCH v2] " Deepakkumar Karn
0 siblings, 1 reply; 7+ messages in thread
From: Jan Kara @ 2025-12-11 9:23 UTC (permalink / raw)
To: Deepakkumar Karn
Cc: Alexander Viro, Christian Brauner, Jan Kara, linux-kernel,
linux-fsdevel
On Wed 10-12-25 23:32:28, Deepakkumar Karn wrote:
> try_to_free_buffers() can be called on folios with no buffers attached
> when filemap_release_folio() is invoked on a folio belonging to a mapping
> with AS_RELEASE_ALWAYS set but no release_folio operation defined.
>
> In such cases, folio_needs_release() returns true because of the
> AS_RELEASE_ALWAYS flag, but the folio has no private buffer data. This
> causes try_to_free_buffers() to call drop_buffers() on a folio with no
> buffers, leading to a null pointer dereference.
>
> Adding a check in try_to_free_buffers() to return early if the folio has no
> buffers attached, with WARN_ON_ONCE() to alert about the misconfiguration.
> This provides defensive hardening.
>
> 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..b229baa77055 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -2948,6 +2948,10 @@ bool try_to_free_buffers(struct folio *folio)
> if (folio_test_writeback(folio))
> return false;
>
> + /* Misconfigured folio check */
> + if (WARN_ON_ONCE(!folio_buffers(folio)))
> + return false;
This should really be returning true. Otherwise the folio will never get
released.
Honza
> +
> if (mapping == NULL) { /* can this still happen? */
> ret = drop_buffers(folio, &buffers_to_free);
> goto out;
> --
> 2.52.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2] fs/buffer: add alert in try_to_free_buffers() for folios without buffers
2025-12-11 9:23 ` Jan Kara
@ 2025-12-11 13:12 ` Deepakkumar Karn
2025-12-11 13:27 ` Jan Kara
2025-12-15 14:07 ` Christian Brauner
0 siblings, 2 replies; 7+ messages in thread
From: Deepakkumar Karn @ 2025-12-11 13:12 UTC (permalink / raw)
To: jack; +Cc: brauner, dkarn, linux-fsdevel, linux-kernel, viro
try_to_free_buffers() can be called on folios with no buffers attached
when filemap_release_folio() is invoked on a folio belonging to a mapping
with AS_RELEASE_ALWAYS set but no release_folio operation defined.
In such cases, folio_needs_release() returns true because of the
AS_RELEASE_ALWAYS flag, but the folio has no private buffer data. This
causes try_to_free_buffers() to call drop_buffers() on a folio with no
buffers, leading to a null pointer dereference.
Adding a check in try_to_free_buffers() to return early if the folio has no
buffers attached, with WARN_ON_ONCE() to alert about the misconfiguration.
This provides defensive hardening.
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..28e4d53f1717 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -2948,6 +2948,10 @@ bool try_to_free_buffers(struct folio *folio)
if (folio_test_writeback(folio))
return false;
+ /* Misconfigured folio check */
+ if (WARN_ON_ONCE(!folio_buffers(folio)))
+ return true;
+
if (mapping == NULL) { /* can this still happen? */
ret = drop_buffers(folio, &buffers_to_free);
goto out;
--
2.52.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2] fs/buffer: add alert in try_to_free_buffers() for folios without buffers
2025-12-11 13:12 ` [PATCH v2] " Deepakkumar Karn
@ 2025-12-11 13:27 ` Jan Kara
2025-12-15 14:07 ` Christian Brauner
1 sibling, 0 replies; 7+ messages in thread
From: Jan Kara @ 2025-12-11 13:27 UTC (permalink / raw)
To: Deepakkumar Karn; +Cc: jack, brauner, linux-fsdevel, linux-kernel, viro
On Thu 11-12-25 18:42:11, Deepakkumar Karn wrote:
> try_to_free_buffers() can be called on folios with no buffers attached
> when filemap_release_folio() is invoked on a folio belonging to a mapping
> with AS_RELEASE_ALWAYS set but no release_folio operation defined.
>
> In such cases, folio_needs_release() returns true because of the
> AS_RELEASE_ALWAYS flag, but the folio has no private buffer data. This
> causes try_to_free_buffers() to call drop_buffers() on a folio with no
> buffers, leading to a null pointer dereference.
>
> Adding a check in try_to_free_buffers() to return early if the folio has no
> buffers attached, with WARN_ON_ONCE() to alert about the misconfiguration.
> This provides defensive hardening.
>
> Signed-off-by: Deepakkumar Karn <dkarn@redhat.com>
Looks good to me now. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/buffer.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/fs/buffer.c b/fs/buffer.c
> index 838c0c571022..28e4d53f1717 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -2948,6 +2948,10 @@ bool try_to_free_buffers(struct folio *folio)
> if (folio_test_writeback(folio))
> return false;
>
> + /* Misconfigured folio check */
> + if (WARN_ON_ONCE(!folio_buffers(folio)))
> + return true;
> +
> if (mapping == NULL) { /* can this still happen? */
> ret = drop_buffers(folio, &buffers_to_free);
> goto out;
> --
> 2.52.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] fs/buffer: add alert in try_to_free_buffers() for folios without buffers
2025-12-11 13:12 ` [PATCH v2] " Deepakkumar Karn
2025-12-11 13:27 ` Jan Kara
@ 2025-12-15 14:07 ` Christian Brauner
2025-12-15 14:10 ` Matthew Wilcox
1 sibling, 1 reply; 7+ messages in thread
From: Christian Brauner @ 2025-12-15 14:07 UTC (permalink / raw)
To: jack, Deepakkumar Karn
Cc: Christian Brauner, linux-fsdevel, linux-kernel, viro
On Thu, 11 Dec 2025 18:42:11 +0530, Deepakkumar Karn wrote:
> try_to_free_buffers() can be called on folios with no buffers attached
> when filemap_release_folio() is invoked on a folio belonging to a mapping
> with AS_RELEASE_ALWAYS set but no release_folio operation defined.
>
> In such cases, folio_needs_release() returns true because of the
> AS_RELEASE_ALWAYS flag, but the folio has no private buffer data. This
> causes try_to_free_buffers() to call drop_buffers() on a folio with no
> buffers, leading to a null pointer dereference.
>
> [...]
Applied to the vfs-7.0.misc branch of the vfs/vfs.git tree.
Patches in the vfs-7.0.misc branch should appear in linux-next soon.
Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.
It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.
Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-7.0.misc
[1/1] fs/buffer: add alert in try_to_free_buffers() for folios without buffers
https://git.kernel.org/vfs/vfs/c/b68f91ef3b3f
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] fs/buffer: add alert in try_to_free_buffers() for folios without buffers
2025-12-15 14:07 ` Christian Brauner
@ 2025-12-15 14:10 ` Matthew Wilcox
2025-12-15 18:00 ` Jan Kara
0 siblings, 1 reply; 7+ messages in thread
From: Matthew Wilcox @ 2025-12-15 14:10 UTC (permalink / raw)
To: Christian Brauner
Cc: jack, Deepakkumar Karn, linux-fsdevel, linux-kernel, viro
On Mon, Dec 15, 2025 at 03:07:35PM +0100, Christian Brauner wrote:
> On Thu, 11 Dec 2025 18:42:11 +0530, Deepakkumar Karn wrote:
> > try_to_free_buffers() can be called on folios with no buffers attached
> > when filemap_release_folio() is invoked on a folio belonging to a mapping
> > with AS_RELEASE_ALWAYS set but no release_folio operation defined.
> >
> > In such cases, folio_needs_release() returns true because of the
> > AS_RELEASE_ALWAYS flag, but the folio has no private buffer data. This
> > causes try_to_free_buffers() to call drop_buffers() on a folio with no
> > buffers, leading to a null pointer dereference.
> >
> > [...]
>
> Applied to the vfs-7.0.misc branch of the vfs/vfs.git tree.
> Patches in the vfs-7.0.misc branch should appear in linux-next soon.
No, this is the wrong fix. Please drop.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] fs/buffer: add alert in try_to_free_buffers() for folios without buffers
2025-12-15 14:10 ` Matthew Wilcox
@ 2025-12-15 18:00 ` Jan Kara
0 siblings, 0 replies; 7+ messages in thread
From: Jan Kara @ 2025-12-15 18:00 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Christian Brauner, jack, Deepakkumar Karn, linux-fsdevel,
linux-kernel, viro
On Mon 15-12-25 14:10:08, Matthew Wilcox wrote:
> On Mon, Dec 15, 2025 at 03:07:35PM +0100, Christian Brauner wrote:
> > On Thu, 11 Dec 2025 18:42:11 +0530, Deepakkumar Karn wrote:
> > > try_to_free_buffers() can be called on folios with no buffers attached
> > > when filemap_release_folio() is invoked on a folio belonging to a mapping
> > > with AS_RELEASE_ALWAYS set but no release_folio operation defined.
> > >
> > > In such cases, folio_needs_release() returns true because of the
> > > AS_RELEASE_ALWAYS flag, but the folio has no private buffer data. This
> > > causes try_to_free_buffers() to call drop_buffers() on a folio with no
> > > buffers, leading to a null pointer dereference.
> > >
> > > [...]
> >
> > Applied to the vfs-7.0.misc branch of the vfs/vfs.git tree.
> > Patches in the vfs-7.0.misc branch should appear in linux-next soon.
>
> No, this is the wrong fix. Please drop.
Nobody says this is a fix (it has WARN_ON() in it after all). But it's a
sensible hardening regardless of other changes we do in this area. After our
discussion I agree that the change to mapping_set_release_always() is not
needed if we instead rework how .release_folio (and try_to_free_buffers())
is called but this is a separate change...
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-12-15 18:00 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-10 18:02 [PATCH] fs/buffer: add alert in try_to_free_buffers() for folios without buffers Deepakkumar Karn
2025-12-11 9:23 ` Jan Kara
2025-12-11 13:12 ` [PATCH v2] " Deepakkumar Karn
2025-12-11 13:27 ` Jan Kara
2025-12-15 14:07 ` Christian Brauner
2025-12-15 14:10 ` Matthew Wilcox
2025-12-15 18:00 ` Jan Kara
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox