mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] configfs: zero the write buffer on allocation
@ 2026-10-15 13:20 Breno Leitao
  2026-10-19 12:35 ` Andreas Hindborg
  0 siblings, 1 reply; 3+ messages in thread
From: Breno Leitao @ 2026-10-15 13:20 UTC (permalink / raw)
  To: Andreas Hindborg, Miguel Ojeda, Boqun Feng, Björn Roy Baron,
	Benno Lossin, Alice Ryhl, Trevor Gross, Danilo Krummrich,
	Daniel Almeida, Tamir Duberstein, Alexandre Courbot,
	Onur Özkan
  Cc: Gary Guo, Priya Bala Govindasamy, rust-for-linux, linux-kernel,
	kernel-team, Breno Leitao

fill_write_buffer() allocates the per-open buffer page with kmalloc()
and initializes only the bytes copied from userspace plus the
terminating NUL. fill_read_buffer() reuses that page if it is already
allocated.

After a write, a read on the same file descriptor therefore passes
->show() a page that still holds stale heap data. A ->show() that
returns only what it wrote never notices, but one that returns a length
covering bytes it did not write leaks that data to userspace.

The Rust bindings are exposed more directly: ->show() receives the page
as an &mut [u8; PAGE_SIZE], so a driver can read the uninitialized
bytes.

fill_read_buffer() already allocates the same page with kzalloc(). Do
the same here, so the page is initialized whichever path allocates it.
The cost is zeroing one page the first time a file is written.

Link: https://lore.kernel.org/r/cover.1790981219.git.pgovind2@uci.edu
Suggested-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 fs/configfs/file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/configfs/file.c b/fs/configfs/file.c
index 6460b000c593..a4b58616352c 100644
--- a/fs/configfs/file.c
+++ b/fs/configfs/file.c
@@ -184,7 +184,7 @@ static int fill_write_buffer(struct configfs_buffer *buffer,
 	int copied;
 
 	if (!buffer->page)
-		buffer->page = kmalloc(PAGE_SIZE, GFP_KERNEL);
+		buffer->page = kzalloc(PAGE_SIZE, GFP_KERNEL);
 	if (!buffer->page)
 		return -ENOMEM;
 

---
base-commit: 21671ce5ad38e0543331604bd96144d4d0d3b421
change-id: 20261015-b4-configfs_kzalloc-ca337878cdcb

Best regards,
--  
Breno Leitao <leitao@debian.org>


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] configfs: zero the write buffer on allocation
  2026-10-15 13:20 [PATCH] configfs: zero the write buffer on allocation Breno Leitao
@ 2026-10-19 12:35 ` Andreas Hindborg
  2026-10-26 18:26   ` Priya Bala Govindasamy
  0 siblings, 1 reply; 3+ messages in thread
From: Andreas Hindborg @ 2026-10-19 12:35 UTC (permalink / raw)
  To: Breno Leitao, Miguel Ojeda, Boqun Feng, Björn Roy Baron,
	Benno Lossin, Alice Ryhl, Trevor Gross, Danilo Krummrich,
	Daniel Almeida, Tamir Duberstein, Alexandre Courbot,
	Onur Özkan
  Cc: Gary Guo, Priya Bala Govindasamy, rust-for-linux, linux-kernel,
	kernel-team, Breno Leitao

Breno Leitao <leitao@debian.org> writes:

> fill_write_buffer() allocates the per-open buffer page with kmalloc()
> and initializes only the bytes copied from userspace plus the
> terminating NUL. fill_read_buffer() reuses that page if it is already
> allocated.
>
> After a write, a read on the same file descriptor therefore passes
> ->show() a page that still holds stale heap data. A ->show() that
> returns only what it wrote never notices, but one that returns a length
> covering bytes it did not write leaks that data to userspace.
>
> The Rust bindings are exposed more directly: ->show() receives the page
> as an &mut [u8; PAGE_SIZE], so a driver can read the uninitialized
> bytes.
>
> fill_read_buffer() already allocates the same page with kzalloc(). Do
> the same here, so the page is initialized whichever path allocates it.
> The cost is zeroing one page the first time a file is written.
>
> Link: https://lore.kernel.org/r/cover.1790981219.git.pgovind2@uci.edu
> Suggested-by: Andreas Hindborg <a.hindborg@kernel.org>
> Signed-off-by: Breno Leitao <leitao@debian.org>

Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>


Best regards,
Andreas Hindborg



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] configfs: zero the write buffer on allocation
  2026-10-19 12:35 ` Andreas Hindborg
@ 2026-10-26 18:26   ` Priya Bala Govindasamy
  0 siblings, 0 replies; 3+ messages in thread
From: Priya Bala Govindasamy @ 2026-10-26 18:26 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: Breno Leitao, Miguel Ojeda, Boqun Feng, Björn Roy Baron,
	Benno Lossin, Alice Ryhl, Trevor Gross, Danilo Krummrich,
	Daniel Almeida, Tamir Duberstein, Alexandre Courbot,
	Onur Özkan, Gary Guo, rust-for-linux, linux-kernel,
	kernel-team

On Mon, Oct 19, 2026 at 5:35 AM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Breno Leitao <leitao@debian.org> writes:
>
> > fill_write_buffer() allocates the per-open buffer page with kmalloc()
> > and initializes only the bytes copied from userspace plus the
> > terminating NUL. fill_read_buffer() reuses that page if it is already
> > allocated.
> >
> > After a write, a read on the same file descriptor therefore passes
> > ->show() a page that still holds stale heap data. A ->show() that
> > returns only what it wrote never notices, but one that returns a length
> > covering bytes it did not write leaks that data to userspace.
> >
> > The Rust bindings are exposed more directly: ->show() receives the page
> > as an &mut [u8; PAGE_SIZE], so a driver can read the uninitialized
> > bytes.
> >
> > fill_read_buffer() already allocates the same page with kzalloc(). Do
> > the same here, so the page is initialized whichever path allocates it.
> > The cost is zeroing one page the first time a file is written.
> >
> > Link: https://urldefense.com/v3/__https://lore.kernel.org/r/cover.1790981219.git.pgovind2@uci.edu__;!!CzAuKJ42GuquVTTmVmPViYEvSg!MzBh-OGC90bot_58mikAXuNn-ZOko4X3HdO1al7Y_kCFHVAaLPKpyhKVLgRpDkWqziXPzvRGlJPkvD9e-ZWTkA$
> > Suggested-by: Andreas Hindborg <a.hindborg@kernel.org>
> > Signed-off-by: Breno Leitao <leitao@debian.org>
>
> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
>
>
> Best regards,
> Andreas Hindborg
>
>

I tested this patch on v7.3-rc5 against the PoC I provided earlier [1]
and it works correctly.
Without the patch, the PoC shows that the first 8 bytes observed by
`show()` were:
  [78, 0, fb, 4f, 83, 88, ff, ff]
With the patch applied, the same bytes were:
  [78, 0, 0, 0, 0, 0, 0, 0]
so the reference is no longer created from uninitialized memory. I did
not see any new warnings in dmesg.

Tested-by: Priya Bala Govindasamy <pgovind2@uci.edu>

[1] https://lore.kernel.org/all/cover.1790981219.git.pgovind2@uci.edu/

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-26 18:26 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-15 13:20 [PATCH] configfs: zero the write buffer on allocation Breno Leitao
2026-10-19 12:35 ` Andreas Hindborg
2026-10-26 18:26   ` Priya Bala Govindasamy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®