sysfs: upgrade OOB write by buggy .show hook into WARNing

sysfs: upgrade OOB write by buggy .show hook into WARNing

[Alexey Dobriyan]

Buggy .show hook will get just 1 line of dmesg:

	fill_read_buffer: ext4_attr_show+0x0/0x600 returned bad count

It may or may not oops later in some unrelated process.

But buggy .show hook most likely is corrupting random memory past sysfs
buffer therefore deserving more. WARN, make it more visible and let
QA machines panic earlier.

Also, delete useless cast -- "count" is >=0 at this point.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 fs/sysfs/file.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -70,9 +70,8 @@ static int sysfs_kf_seq_show(struct seq_file *sf, void *v)
 	 * The code works fine with PAGE_SIZE return but it's likely to
 	 * indicate truncated result or overflow in normal use cases.
 	 */
-	if (count >= (ssize_t)PAGE_SIZE) {
-		printk("fill_read_buffer: %pS returned bad count\n",
-				ops->show);
+	if (count >= PAGE_SIZE) {
+		WARN(1, "OOB write or bad count %zd at %pS\n", count, ops->show);
 		/* Try to struggle along */
 		count = PAGE_SIZE - 1;
 	}

[PATCH] sysfs: upgrade OOB write by buggy .show hook into WARNing

[Alexey Dobriyan]

Buggy .show hook will get just 1 line of dmesg:

	fill_read_buffer: ext4_attr_show+0x0/0x600 returned bad count

It may or may not oops later in some unrelated process.

But buggy .show hook most likely is corrupting random memory past sysfs
buffer therefore deserving more. WARN, make it more visible and let
QA machines panic earlier.

Also, delete useless cast -- "count" is >=0 at this point.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 fs/sysfs/file.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -70,9 +70,8 @@ static int sysfs_kf_seq_show(struct seq_file *sf, void *v)
 	 * The code works fine with PAGE_SIZE return but it's likely to
 	 * indicate truncated result or overflow in normal use cases.
 	 */
-	if (count >= (ssize_t)PAGE_SIZE) {
-		printk("fill_read_buffer: %pS returned bad count\n",
-				ops->show);
+	if (count >= PAGE_SIZE) {
+		WARN(1, "OOB write or bad count %zd at %pS\n", count, ops->show);
 		/* Try to struggle along */
 		count = PAGE_SIZE - 1;
 	}

Re: [PATCH] sysfs: upgrade OOB write by buggy .show hook into WARNing

[Greg Kroah-Hartman]

On Thu, May 07, 2026 at 12:01:43PM +0300, Alexey Dobriyan wrote:
> Buggy .show hook will get just 1 line of dmesg:
> 
> 	fill_read_buffer: ext4_attr_show+0x0/0x600 returned bad count
> 
> It may or may not oops later in some unrelated process.
> 
> But buggy .show hook most likely is corrupting random memory past sysfs
> buffer therefore deserving more. WARN, make it more visible and let
> QA machines panic earlier.
> 
> Also, delete useless cast -- "count" is >=0 at this point.
> 
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> ---
> 
>  fs/sysfs/file.c |    5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

The first was fine, no need for [PATCH] :)

> --- a/fs/sysfs/file.c
> +++ b/fs/sysfs/file.c
> @@ -70,9 +70,8 @@ static int sysfs_kf_seq_show(struct seq_file *sf, void *v)
>  	 * The code works fine with PAGE_SIZE return but it's likely to
>  	 * indicate truncated result or overflow in normal use cases.
>  	 */
> -	if (count >= (ssize_t)PAGE_SIZE) {
> -		printk("fill_read_buffer: %pS returned bad count\n",
> -				ops->show);
> +	if (count >= PAGE_SIZE) {
> +		WARN(1, "OOB write or bad count %zd at %pS\n", count, ops->show);

This is going to be interesting to see what triggers, so I'll go queue
this up soon.

And this implies you did hit this on ext4?  What sysfs file for ext4 is
doing this?  That should not be a valid sysfs file at all.

thanks,

greg k-h

Re: [PATCH] sysfs: upgrade OOB write by buggy .show hook into WARNing

[Alexey Dobriyan]

On Thu, May 07, 2026 at 11:05:16AM +0200, Greg Kroah-Hartman wrote:
> On Thu, May 07, 2026 at 12:01:43PM +0300, Alexey Dobriyan wrote:

> > --- a/fs/sysfs/file.c
> > +++ b/fs/sysfs/file.c
> > @@ -70,9 +70,8 @@ static int sysfs_kf_seq_show(struct seq_file *sf, void *v)
> >  	 * The code works fine with PAGE_SIZE return but it's likely to
> >  	 * indicate truncated result or overflow in normal use cases.
> >  	 */
> > -	if (count >= (ssize_t)PAGE_SIZE) {
> > -		printk("fill_read_buffer: %pS returned bad count\n",
> > -				ops->show);
> > +	if (count >= PAGE_SIZE) {
> > +		WARN(1, "OOB write or bad count %zd at %pS\n", count, ops->show);
> 
> This is going to be interesting to see what triggers, so I'll go queue
> this up soon.
> 
> And this implies you did hit this on ext4?

The check was triggered by out-of-tree module.
ext4 is me testing the patch with a custom written buggy hook.

	/sys/alexey

Re: [PATCH] sysfs: upgrade OOB write by buggy .show hook into WARNing

[Danilo Krummrich]

(Cc: driver-core)

On Thu May 7, 2026 at 11:01 AM CEST, Alexey Dobriyan wrote:
> Buggy .show hook will get just 1 line of dmesg:
>
> 	fill_read_buffer: ext4_attr_show+0x0/0x600 returned bad count
>
> It may or may not oops later in some unrelated process.
>
> But buggy .show hook most likely is corrupting random memory past sysfs
> buffer therefore deserving more. WARN, make it more visible and let
> QA machines panic earlier.
>
> Also, delete useless cast -- "count" is >=0 at this point.
>
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>

Reviewed-by: Danilo Krummrich <dakr@kernel.org>

NIT: This patch also cleans up the stale "fill_read_buffer:" prefix, and instead
points out the actual caller, which could be mentioned in the commit message --
no need to resend for this though.

Re: [PATCH] sysfs: upgrade OOB write by buggy .show hook into WARNing

[Alexey Dobriyan]

On Thu, May 07, 2026 at 11:28:29AM +0200, Danilo Krummrich wrote:
> (Cc: driver-core)
> 
> On Thu May 7, 2026 at 11:01 AM CEST, Alexey Dobriyan wrote:
> > Buggy .show hook will get just 1 line of dmesg:
> >
> > 	fill_read_buffer: ext4_attr_show+0x0/0x600 returned bad count
> >
> > It may or may not oops later in some unrelated process.
> >
> > But buggy .show hook most likely is corrupting random memory past sysfs
> > buffer therefore deserving more. WARN, make it more visible and let
> > QA machines panic earlier.
> >
> > Also, delete useless cast -- "count" is >=0 at this point.
> >
> > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> 
> Reviewed-by: Danilo Krummrich <dakr@kernel.org>
> 
> NIT: This patch also cleans up the stale "fill_read_buffer:" prefix, and instead
> points out the actual caller, which could be mentioned in the commit message --
> no need to resend for this though.

Yes. WARN prints enough information to locate the bug, so stale function
name just goes away.