[bug report] ASoC: SOF: ipc-msg-injector: Add support for IPC4 messages

[Péter Ujfalusi]

Dan,

On 16/05/2022 11:54, Dan Carpenter wrote:
> Hello Peter Ujfalusi,
> 
> The patch 066c67624d8c: "ASoC: SOF: ipc-msg-injector: Add support for
> IPC4 messages" from May 6, 2022, leads to the following Smatch static
> checker warning:
> 
> 	sound/soc/sof/sof-client-ipc-msg-injector.c:95 sof_msg_inject_ipc4_dfs_read()
> 	warn: userbuf overflow? is '8' <= 'count'
> 
> sound/soc/sof/sof-client-ipc-msg-injector.c
>     72 static ssize_t sof_msg_inject_ipc4_dfs_read(struct file *file,
>     73                                             char __user *buffer,
>     74                                             size_t count, loff_t *ppos)
>     75 {
>     76         struct sof_client_dev *cdev = file->private_data;
>     77         struct sof_msg_inject_priv *priv = cdev->data;
>     78         struct sof_ipc4_msg *ipc4_msg = priv->rx_buffer;
>     79         size_t remaining;
>     80 
>     81         if (!ipc4_msg->header_u64 || !count || *ppos)
>     82                 return 0;
>     83 
>     84         remaining = sizeof(ipc4_msg->header_u64);
>     85 
>     86         /* Only get large config have payload */
>     87         if (SOF_IPC4_MSG_IS_MODULE_MSG(ipc4_msg->primary) &&
>     88             (SOF_IPC4_MSG_TYPE_GET(ipc4_msg->primary) == SOF_IPC4_MOD_LARGE_CONFIG_GET))
>     89                 remaining += ipc4_msg->data_size;
>     90 
>     91         if (count > remaining)
>     92                 count = remaining;
>     93 
>     94         /* copy the header first */
> --> 95         if (copy_to_user(buffer, &ipc4_msg->header_u64, sizeof(ipc4_msg->header_u64)))
>     96                 return -EFAULT;
>     97 
>     98         *ppos += sizeof(ipc4_msg->header_u64);
>     99         remaining -= sizeof(ipc4_msg->header_u64);
>     100 
>     101         if (!remaining)
>     102                 return count;
>     103 
>     104         if (remaining > ipc4_msg->data_size)
>     105                 remaining = ipc4_msg->data_size;
>     106 
>     107         /* Copy the payload */
>     108         if (copy_to_user(buffer + *ppos, ipc4_msg->data_ptr, remaining))
>                                  ^^^^^^^^^^^^^^^
> Potentially writing more than count bytes resulting in corrupting the
> user space memory.

Yes, this can happen.
This is a validation, debugging module, used by developers and testers
only so we did not had the case when the provided buffer would be
smaller than the data we want to retrieve, but the issue is real and I
have sent the fix now.

> 
>     109                 return -EFAULT;
>     110 
>     111         *ppos += remaining;
>     112         return count;
>     113 }
> 
> regards,
> dan carpenter

-- 
Péter