On 02/06/2022 12:15, Dan Carpenter wrote:
> Hello Peter Ujfalusi,
>
> The patch a9aa3381e404: "ASoC: SOF: ipc-msg-injector: Separate the
> message sending" from May 6, 2022, leads to the following Smatch
> static checker warning:
>
> sound/soc/sof/sof-client-ipc-msg-injector.c:162 sof_msg_inject_dfs_write()
> warn: kernel error codes cast to unsigned 'size'
>
> sound/soc/sof/sof-client-ipc-msg-injector.c
> 148 static ssize_t sof_msg_inject_dfs_write(struct file *file, const char __user *buffer,
> 149 size_t count, loff_t *ppos)
> 150 {
> 151 struct sof_client_dev *cdev = file->private_data;
> 152 struct sof_msg_inject_priv *priv = cdev->data;
> 153 size_t size;
> 154 int ret;
> 155
> 156 if (*ppos)
> 157 return 0;
>
> I think there needs to be an "if (count != priv->max_msg_size)" check
> or something. Or another option would be the do a memset()
The interface is used to feed in crafted IPC messages to torture the
firmware (and in some level the kernel as well). How it will handle
deliberately ill crafted messages, what it will do if a valid but
unexpected message is sent, etc.
The only check I could think of is to prevent
less than sizeof(struct sof_ipc_cmd_hdr) count writes, but one could
argue that sending a normal header (u32 size, u32 cmd) followed by only
changing the size is also a valid shortcut.
>
> memset(priv->tx_buffer, 0, priv->max_msg_size);
>
> before the simple_write_to_buffer(). Otherwise if count == 1 then we
> re-use stale data.
If count is < sizeof(struct sof_ipc_cmd_hdr) to be precise, but even if
that passes there could be stale data in the buffer for message types
where there is extended payload.
Yes, there could be more size check, but the injector must not interpret
the message, it should not block invalid messages.
Let me think a bit on the minimum count check for a moment...
>
> 158
> 159 size = simple_write_to_buffer(priv->tx_buffer, priv->max_msg_size,
> 160 ppos, buffer, count);
> 161 if (size != count)
> --> 162 return size > 0 ? -EFAULT : size;
> 163
> 164 memset(priv->rx_buffer, 0, priv->max_msg_size);
> 165
> 166 ret = sof_msg_inject_send_message(cdev);
> 167
> 168 /* return the error code if test failed */
> 169 if (ret < 0)
> 170 size = ret;
> 171
> 172 return size;
> 173 };
>
> regards,
> dan carpenter
--
Péter
