Re: [PATCH 1/2] ASoC: SOF: sof-client-probes: fix error codes in sof_probes_compr_copy()

["Péter Ujfalusi" <peter.ujfalusi@linux.intel.com>]

On 06/07/2022 12:05, Péter Ujfalusi wrote:
> 
> 
> On 06/07/2022 10:23, Dan Carpenter wrote:
>> This function tries to return the number of bytes that it was able to
>> copy to the user.  However, because there are multiple calls to
>> copy_to_user() in a row that means the bytes are not necessarily
>> consecutive so it's not useful.  Just return -EFAULT instead.
> 
> The function is copying data from a circular buffer to a use buffer.
> The single copy_to_user() is used when we don't have wrapping, the
> 'double' copy_to_user() is when we wrap, so first copy is from the end
> of the buffer then we copy the data from the start of the buffer to get
> all data.

What I wanted to say is that the original code is correct, this patch
would break the functionality.

> 
>> Fixes: 3dc0d7091778 ("ASoC: SOF: Convert the generic probe support to SOF client")
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>> ---
>>  sound/soc/sof/sof-client-probes.c | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/sound/soc/sof/sof-client-probes.c b/sound/soc/sof/sof-client-probes.c
>> index 1f1ea93a7fbf..679bc7d371fc 100644
>> --- a/sound/soc/sof/sof-client-probes.c
>> +++ b/sound/soc/sof/sof-client-probes.c
>> @@ -385,7 +385,6 @@ static int sof_probes_compr_copy(struct snd_soc_component *component,
>>  	struct snd_compr_runtime *rtd = cstream->runtime;
>>  	unsigned int offset, n;
>>  	void *ptr;
>> -	int ret;
>>  
>>  	if (count > rtd->buffer_size)
>>  		count = rtd->buffer_size;
>> @@ -395,14 +394,15 @@ static int sof_probes_compr_copy(struct snd_soc_component *component,
>>  	n = rtd->buffer_size - offset;
>>  
>>  	if (count < n) {
>> -		ret = copy_to_user(buf, ptr, count);
>> +		if (copy_to_user(buf, ptr, count))
>> +			return -EFAULT;
>>  	} else {
>> -		ret = copy_to_user(buf, ptr, n);
>> -		ret += copy_to_user(buf + n, rtd->dma_area, count - n);
>> +		if (copy_to_user(buf, ptr, n))
>> +			return -EFAULT;
>> +		if (copy_to_user(buf + n, rtd->dma_area, count - n))
>> +			return -EFAULT;
>>  	}
>>  
>> -	if (ret)
>> -		return count - ret;
>>  	return count;
>>  }
>>  
> 

-- 
Péter