Re: [PATCH] remoteproc: debugfs: Replace scnprintf() with sysfs_emit()

[Christophe JAILLET]

Le 19/06/2025 à 21:04, Abhinav Ananthu a écrit :
> I apologize for the mistake in the signed-off email address in the previous patch.
>
> Convert the debugfs show() functions in remoteproc_debugfs.c to use
> sysfs_emit() instead of scnprintf(). The sysfs_emit() helper is the
> preferred way to format sysfs output as it ensures the output is
> properly bounded to PAGE_SIZE and simplifies the code.
>
> This patch addresses three instances of scnprintf() usage in the file.

I think that this patch is just wrong.

The 3 functions below are not related to debugfs show function.

sysfs_emit() expect the buffer to be pages aligned (See [1]).
In the cases below, the buffers that are used are defined on the stack 
just a few lines above.

It is really unlikely that this condition will be met.


CJ

[1]: https://elixir.bootlin.com/linux/v6.15.2/source/fs/sysfs/file.c#L767


> Signed-off-by: Abhinav Ananthu <abhinav.ogl@xxxxxxxxx>
> ---
>   drivers/remoteproc/remoteproc_debugfs.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/remoteproc/remoteproc_debugfs.c b/drivers/remoteproc/remoteproc_debugfs.c
> index b86c1d09c70c..691fd523e0b5 100644
> --- a/drivers/remoteproc/remoteproc_debugfs.c
> +++ b/drivers/remoteproc/remoteproc_debugfs.c
> @@ -46,8 +46,9 @@ static ssize_t rproc_coredump_read(struct file *filp, char __user *userbuf,
>   	char buf[20];
>   	int len;
>   
> -	len = scnprintf(buf, sizeof(buf), "%s\n",
> -			rproc_coredump_str[rproc->dump_conf]);
> +	len = sysfs_emit(buf, "%s\n",
> +		rproc_coredump_str[rproc->dump_conf]);
> +
>   
>   	return simple_read_from_buffer(userbuf, count, ppos, buf, len);
>   }
> @@ -135,7 +136,7 @@ static ssize_t rproc_trace_read(struct file *filp, char __user *userbuf,
>   	va = rproc_da_to_va(data->rproc, trace->da, trace->len, NULL);
>   
>   	if (!va) {
> -		len = scnprintf(buf, sizeof(buf), "Trace %s not available\n",
> +		len = sysfs_emit(buf, "Trace %s not available\n",
>   				trace->name);
>   		va = buf;
>   	} else {
> @@ -160,7 +161,7 @@ static ssize_t rproc_name_read(struct file *filp, char __user *userbuf,
>   	char buf[100];
>   	int i;
>   
> -	i = scnprintf(buf, sizeof(buf), "%.98s\n", rproc->name);
> +	i = sysfs_emit(buf, "%.98s\n", rproc->name);
>   
>   	return simple_read_from_buffer(userbuf, count, ppos, buf, i);
>   }