Re: [PATCH 4.19 322/323] drivers core: Use sysfs_emit and sysfs_emit_at for show(device *...) functions

[Ben Hutchings <ben@decadent.org.uk>]

[-- Attachment #1: Type: text/plain, Size: 2333 bytes --]

On Wed, 2023-08-09 at 12:42 +0200, Greg Kroah-Hartman wrote:
> From: Joe Perches <joe@perches.com>
> 
> commit aa838896d87af561a33ecefea1caa4c15a68bc47 upstream.
> 
> Convert the various sprintf fmaily calls in sysfs device show functions
> to sysfs_emit and sysfs_emit_at for PAGE_SIZE buffer safety.

[...]
> Signed-off-by: Joe Perches <joe@perches.com>
> Link: https://lore.kernel.org/r/3d033c33056d88bbe34d4ddb62afd05ee166ab9a.1600285923.git.joe@perches.com
> [ Brennan : Regenerated for 4.19 to fix CVE-2022-20166 ]

When I looked into the referenced security issue, it seemed to only be
exploitable through wakelock names, and in the upstream kernel only
after commit c8377adfa781 "PM / wakeup: Show wakeup sources stats in
sysfs" (first included in 5.4).  So I would be interested to know if
and why a fix was needed for 4.19.

More importantly, this backported version uniformly converts to
sysfs_emit(), but there are 3 places sysfs_emit_at() must be used
instead:

[...]
> --- a/drivers/base/cpu.c
> +++ b/drivers/base/cpu.c
[...]
> @@ -264,7 +264,7 @@ static ssize_t print_cpus_offline(struct
>  						      nr_cpu_ids, total_cpus-1);
>  	}
>  
> -	n += snprintf(&buf[n], len - n, "\n");
> +	n += sysfs_emit(&buf[n], "\n");
>  	return n;
>  }
>  static DEVICE_ATTR(offline, 0444, print_cpus_offline, NULL);
[...]
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
[...]
> @@ -96,7 +96,7 @@ static ssize_t node_read_meminfo(struct
>  		       nid, K(sum_zone_node_page_state(nid, NR_MLOCK)));
>  
>  #ifdef CONFIG_HIGHMEM
> -	n += sprintf(buf + n,
> +	n += sysfs_emit(buf + n,
>  		       "Node %d HighTotal:      %8lu kB\n"
>  		       "Node %d HighFree:       %8lu kB\n"
>  		       "Node %d LowTotal:       %8lu kB\n"
> @@ -106,7 +106,7 @@ static ssize_t node_read_meminfo(struct
>  		       nid, K(i.totalram - i.totalhigh),
>  		       nid, K(i.freeram - i.freehigh));
>  #endif
> -	n += sprintf(buf + n,
> +	n += sysfs_emit(buf + n,
>  		       "Node %d Dirty:          %8lu kB\n"
>  		       "Node %d Writeback:      %8lu kB\n"
>  		       "Node %d FilePages:      %8lu kB\n"
[...]

Ben.

-- 
Ben Hutchings
Kids!  Bringing about Armageddon can be dangerous.  Do not attempt it
in your own home. - Terry Pratchett and Neil Gaiman, `Good Omens'


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]