[bug report] ASoC: SOF: compress: Add copy function for capture case
Dan Carpenter
dan.carpenter at oracle.com
Thu Sep 15 13:30:19 CEST 2022
Hello Laurentiu Mihalcea,
The patch 1a01e1927802: "ASoC: SOF: compress: Add copy function for
capture case" from Aug 22, 2022, leads to the following Smatch static
checker warning:
sound/soc/sof/compress.c:315 sof_compr_copy_playback()
warn: special assignment for copy_ function '+='
sound/soc/sof/compress.c
300 static int sof_compr_copy_playback(struct snd_compr_runtime *rtd,
301 char __user *buf, size_t count)
302 {
303 void *ptr;
304 unsigned int offset, n;
305 int ret;
306
307 div_u64_rem(rtd->total_bytes_available, rtd->buffer_size, &offset);
308 ptr = rtd->dma_area + offset;
309 n = rtd->buffer_size - offset;
310
311 if (count < n) {
312 ret = copy_from_user(ptr, buf, count);
313 } else {
314 ret = copy_from_user(ptr, buf, n);
--> 315 ret += copy_from_user(rtd->dma_area, buf + n, count - n);
This API is not ideal.
The static checker warning is based on the theory that if you combine
the number of bytes copied from the first copy with the number of bytes
from the second copy, is that useful? "We copied 100 bytes. But not
necessarily consecutive bytes, so you'll have to guess which they
were!"
Generally if we're copying from the user and the destination buffer is
too small we return the number of bytes which were copied. But if the
user memory is invalid we return -EFAULT.
Here the user memory is invalid but we are returning a truncated number
of bytes, so it suggests that the kernel buffer is too small which is
kind of misleading. I guess user space kind of gets what they deserve
for passing invalid memory...
Also it looks like we can't really return -EFAULT because _soc_component_ret()
is not able to accept that...
316 }
317
318 return count - ret;
319 }
regards,
dan carpenter
More information about the Alsa-devel
mailing list