History

misc: bcm_vk: Fix possible null-pointer dereferences in bcm_vk_read()

In the function bcm_vk_read(), the pointer entry is checked, indicating that it can be NULL. If entry is NULL and rc is set to -EMSGSIZE, the following code may cause null-pointer dereferences: struct vk_msg_blk tmp_msg = entry->to_h_msg[0]; set_msg_id(&tmp_msg, entry->usr_msg_id); tmp_msg.size = entry->to_h_blks - 1; To prevent these possible null-pointer dereferences, copy to_h_msg, usr_msg_id, and to_h_blks from iter into temporary variables, and return these temporary variables to the application instead of accessing them through a potentially NULL entry.

Tuo Li committed 11 months ago

Commit ba75ecb1 parent b54c82d1 file · +8 −4Browse files

Unified · Split

drivers/misc/bcm-vk/bcm_vk_msg.c+8−4
@@ -1010,6 +1010,9 @@ ssize_t bcm_vk_read(struct file *p_file, · 1009 unmodified lines
1010 struct device *dev = &vk->pdev->dev;1010 struct device *dev = &vk->pdev->dev;
1011 struct bcm_vk_msg_chan *chan = &vk->to_h_msg_chan;1011 struct bcm_vk_msg_chan *chan = &vk->to_h_msg_chan;
1012 struct bcm_vk_wkent *entry = NULL, *iter;1012 struct bcm_vk_wkent *entry = NULL, *iter;
1013 struct vk_msg_blk tmp_msg;
1014 u32 tmp_usr_msg_id;
1015 u32 tmp_blks;
1013 u32 q_num;1016 u32 q_num;
1014 u32 rsp_length;1017 u32 rsp_length;
1015 1018
@@ -1034,6 +1037,9 @@ ssize_t bcm_vk_read(struct file *p_file, · 18 unmodified lines
1034 entry = iter;1037 entry = iter;
1035 } else {1038 } else {
1036 /* buffer not big enough */1039 /* buffer not big enough */
1040 tmp_msg = iter->to_h_msg[0];
1041 tmp_usr_msg_id = iter->usr_msg_id;
1042 tmp_blks = iter->to_h_blks;
1037 rc = -EMSGSIZE;1043 rc = -EMSGSIZE;
1038 }1044 }
1039 goto read_loop_exit;1045 goto read_loop_exit;
@@ -1052,14 +1058,12 @@ read_loop_exit: · 12 unmodified lines
1052 1058
1053 bcm_vk_free_wkent(dev, entry);1059 bcm_vk_free_wkent(dev, entry);
1054 } else if (rc == -EMSGSIZE) {1060 } else if (rc == -EMSGSIZE) {
1055 struct vk_msg_blk tmp_msg = entry->to_h_msg[0];
1056
1057 /*1061 /*
1058 * in this case, return just the first block, so1062 * in this case, return just the first block, so
1059 * that app knows what size it is looking for.1063 * that app knows what size it is looking for.
1060 */1064 */
1061 set_msg_id(&tmp_msg, entry->usr_msg_id);1065 set_msg_id(&tmp_msg, tmp_usr_msg_id);
1062 tmp_msg.size = entry->to_h_blks - 1;1066 tmp_msg.size = tmp_blks - 1;
1063 if (copy_to_user(buf, &tmp_msg, VK_MSGQ_BLK_SIZE) != 0) {1067 if (copy_to_user(buf, &tmp_msg, VK_MSGQ_BLK_SIZE) != 0) {
1064 dev_err(dev, "Error return 1st block in -EMSGSIZE\n");1068 dev_err(dev, "Error return 1st block in -EMSGSIZE\n");
1065 rc = -EFAULT;1069 rc = -EFAULT;