On Wed, Jul 06, 2022 at 01:21:59PM +0300, Dan Carpenter wrote: > On Wed, Jul 06, 2022 at 12:05:37PM +0300, 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. > > Ok. But the bugs in the original code are real. I will resend. Actually that's not true. The bugs in the original code are something that only affect users who deserve it? I might not resend. A fix would look something like below? regards, dan carpenter diff --git a/sound/soc/sof/sof-client-probes.c b/sound/soc/sof/sof-client-probes.c index 1f1ea93a7fbf..32fa3186c295 100644 --- a/sound/soc/sof/sof-client-probes.c +++ b/sound/soc/sof/sof-client-probes.c @@ -398,9 +398,14 @@ static int sof_probes_compr_copy(struct snd_soc_component *component, ret = copy_to_user(buf, ptr, count); } else { ret = copy_to_user(buf, ptr, n); - ret += copy_to_user(buf + n, rtd->dma_area, count - n); + if (ret) { + ret += count - n; + goto done; + } + ret = copy_to_user(buf + n, rtd->dma_area, count - n); } +done: if (ret) return count - ret; return count;