[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