Re: [PATCH] ASoC: SOF: Clean up sof_ipc_flood_dfs_write()

[Péter Ujfalusi]

On 04/04/2024 10:34, Dan Carpenter wrote:
> This function doesn't support partial writes so using
> simple_write_to_buffer() doesn't really make sense.  It's better to
> just use copy_from_user().
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>

Thanks, it makes sense.

Acked-by: Peter Ujfalusi <peter.ujfalusi@xxxxxxxxxxxxxxx>

> ---
>  sound/soc/sof/sof-client-ipc-flood-test.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/sound/soc/sof/sof-client-ipc-flood-test.c b/sound/soc/sof/sof-client-ipc-flood-test.c
> index c0d6723aed59..1b2e9e25a836 100644
> --- a/sound/soc/sof/sof-client-ipc-flood-test.c
> +++ b/sound/soc/sof/sof-client-ipc-flood-test.c
> @@ -160,15 +160,20 @@ static ssize_t sof_ipc_flood_dfs_write(struct file *file, const char __user *buf
>  	unsigned long ipc_count = 0;
>  	struct dentry *dentry;
>  	int err;
> -	size_t size;
>  	char *string;
>  	int ret;
>  
> +	if (*ppos != 0)
> +		return -EINVAL;
> +
>  	string = kzalloc(count + 1, GFP_KERNEL);
>  	if (!string)
>  		return -ENOMEM;
>  
> -	size = simple_write_to_buffer(string, count, ppos, buffer, count);
> +	if (copy_from_user(string, buffer, count)) {
> +		ret = -EFAULT;
> +		goto out;
> +	}
>  
>  	/*
>  	 * write op is only supported for ipc_flood_count or
> @@ -198,7 +203,7 @@ static ssize_t sof_ipc_flood_dfs_write(struct file *file, const char __user *buf
>  	/* limit max duration/ipc count for flood test */
>  	if (flood_duration_test) {
>  		if (!ipc_duration_ms) {
> -			ret = size;
> +			ret = count;
>  			goto out;
>  		}
>  
> @@ -207,7 +212,7 @@ static ssize_t sof_ipc_flood_dfs_write(struct file *file, const char __user *buf
>  			ipc_duration_ms = MAX_IPC_FLOOD_DURATION_MS;
>  	} else {
>  		if (!ipc_count) {
> -			ret = size;
> +			ret = count;
>  			goto out;
>  		}
>  
> @@ -231,9 +236,9 @@ static ssize_t sof_ipc_flood_dfs_write(struct file *file, const char __user *buf
>  	if (err < 0)
>  		dev_err_ratelimited(dev, "debugfs write failed to idle %d\n", err);
>  
> -	/* return size if test is successful */
> +	/* return count if test is successful */
>  	if (ret >= 0)
> -		ret = size;
> +		ret = count;
>  out:
>  	kfree(string);
>  	return ret;

-- 
Péter