linux-kernel - [PATCH] misc: bcm-vk: Fix NULL pointer dereference in case of buffer is not big enough in bcm_vk

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20240604195820.29426-1-amishin@t-argos.ru>
Date: Tue, 4 Jun 2024 22:58:20 +0300
From: Aleksandr Mishin <amishin@...rgos.ru>
To: Jakob Koschel <jakobkoschel@...il.com>
CC: Aleksandr Mishin <amishin@...rgos.ru>, Scott Branden
	<scott.branden@...adcom.com>, Broadcom internal kernel review list
	<bcm-kernel-feedback-list@...adcom.com>, Arnd Bergmann <arnd@...db.de>, Greg
 Kroah-Hartman <gregkh@...uxfoundation.org>, <linux-kernel@...r.kernel.org>,
	<lvc-project@...uxtesting.org>
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 <amishin@...rgos.ru>
---
 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