2024-06-04 20:00:31

by Aleksandr Mishin

[permalink] [raw]
Subject: [PATCH] misc: bcm-vk: Fix NULL pointer dereference in case of buffer is not big enough in bcm_vk_read()

In case of entry is found but buffer is not big enough in bcm_vk_read()
found entry pointer remaining unset, but later dereferenced. This will lead
to NULL pointer dereference.
Fix this bug by moving pointer setting and correcting the conditions.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 88517757a829 ("misc: bcm-vk: replace usage of found with dedicated list iterator variable")
Signed-off-by: Aleksandr Mishin <[email protected]>
---
drivers/misc/bcm-vk/bcm_vk_msg.c | 38 +++++++++++++++++---------------
1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/misc/bcm-vk/bcm_vk_msg.c b/drivers/misc/bcm-vk/bcm_vk_msg.c
index 1f42d1d5a630..566bb055fcf7 100644
--- a/drivers/misc/bcm-vk/bcm_vk_msg.c
+++ b/drivers/misc/bcm-vk/bcm_vk_msg.c
@@ -1031,11 +1031,11 @@ ssize_t bcm_vk_read(struct file *p_file,
(iter->to_h_blks * VK_MSGQ_BLK_SIZE)) {
list_del(&iter->node);
atomic_dec(&ctx->pend_cnt);
- entry = iter;
} else {
/* buffer not big enough */
rc = -EMSGSIZE;
}
+ entry = iter;
goto read_loop_exit;
}
}
@@ -1044,25 +1044,27 @@ ssize_t bcm_vk_read(struct file *p_file,
spin_unlock(&chan->pendq_lock);

if (entry) {
- /* retrieve the passed down msg_id */
- set_msg_id(&entry->to_h_msg[0], entry->usr_msg_id);
- rsp_length = entry->to_h_blks * VK_MSGQ_BLK_SIZE;
- if (copy_to_user(buf, entry->to_h_msg, rsp_length) == 0)
- rc = rsp_length;
+ if (rc != -EMSGSIZE) {
+ /* retrieve the passed down msg_id */
+ set_msg_id(&entry->to_h_msg[0], entry->usr_msg_id);
+ rsp_length = entry->to_h_blks * VK_MSGQ_BLK_SIZE;
+ if (copy_to_user(buf, entry->to_h_msg, rsp_length) == 0)
+ rc = rsp_length;

- bcm_vk_free_wkent(dev, entry);
- } else if (rc == -EMSGSIZE) {
- struct vk_msg_blk tmp_msg = entry->to_h_msg[0];
+ bcm_vk_free_wkent(dev, entry);
+ } else {
+ struct vk_msg_blk tmp_msg = entry->to_h_msg[0];

- /*
- * in this case, return just the first block, so
- * that app knows what size it is looking for.
- */
- set_msg_id(&tmp_msg, entry->usr_msg_id);
- tmp_msg.size = entry->to_h_blks - 1;
- if (copy_to_user(buf, &tmp_msg, VK_MSGQ_BLK_SIZE) != 0) {
- dev_err(dev, "Error return 1st block in -EMSGSIZE\n");
- rc = -EFAULT;
+ /*
+ * in this case, return just the first block, so
+ * that app knows what size it is looking for.
+ */
+ set_msg_id(&tmp_msg, entry->usr_msg_id);
+ tmp_msg.size = entry->to_h_blks - 1;
+ if (copy_to_user(buf, &tmp_msg, VK_MSGQ_BLK_SIZE) != 0) {
+ dev_err(dev, "Error return 1st block in -EMSGSIZE\n");
+ rc = -EFAULT;
+ }
}
}
return rc;
--
2.30.2