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