[bug report] ASoC: SOF: compress: Add copy function for capture case
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
participants (1)
-
Dan Carpenter