All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Andrea Parri (Microsoft)" <parri.andrea@gmail.com>
To: linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org
Cc: "K . Y . Srinivasan" <kys@microsoft.com>,
	Haiyang Zhang <haiyangz@microsoft.com>,
	Stephen Hemminger <sthemmin@microsoft.com>,
	Wei Liu <wei.liu@kernel.org>,
	Michael Kelley <mikelley@microsoft.com>,
	Juan Vazquez <juvazq@microsoft.com>,
	Saruhan Karademir <skarade@microsoft.com>,
	"Andrea Parri (Microsoft)" <parri.andrea@gmail.com>
Subject: [PATCH v2 3/7] Drivers: hv: vmbus: Avoid double fetch of payload_size in vmbus_on_msg_dpc()
Date: Wed,  2 Dec 2020 10:22:10 +0100	[thread overview]
Message-ID: <20201202092214.13520-4-parri.andrea@gmail.com> (raw)
In-Reply-To: <20201202092214.13520-1-parri.andrea@gmail.com>

vmbus_on_msg_dpc() double fetches from payload_size.  The double fetch
can lead to a buffer overflow when (mem)copying the hv_message object.
Avoid the double fetch by saving the value of payload_size into a local
variable.

Reported-by: Juan Vazquez <juvazq@microsoft.com>
Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
---
 drivers/hv/vmbus_drv.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 82b23baa446d7..0e39f1d6182e9 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1056,6 +1056,7 @@ void vmbus_on_msg_dpc(unsigned long data)
 	void *page_addr = hv_cpu->synic_message_page;
 	struct hv_message *msg = (struct hv_message *)page_addr +
 				  VMBUS_MESSAGE_SINT;
+	__u8 payload_size = msg->header.payload_size;
 	struct vmbus_channel_message_header *hdr;
 	enum vmbus_channel_message_type msgtype;
 	const struct vmbus_channel_message_table_entry *entry;
@@ -1089,9 +1090,8 @@ void vmbus_on_msg_dpc(unsigned long data)
 		goto msg_handled;
 	}
 
-	if (msg->header.payload_size > HV_MESSAGE_PAYLOAD_BYTE_COUNT) {
-		WARN_ONCE(1, "payload size is too large (%d)\n",
-			  msg->header.payload_size);
+	if (payload_size > HV_MESSAGE_PAYLOAD_BYTE_COUNT) {
+		WARN_ONCE(1, "payload size is too large (%d)\n", payload_size);
 		goto msg_handled;
 	}
 
@@ -1100,21 +1100,18 @@ void vmbus_on_msg_dpc(unsigned long data)
 	if (!entry->message_handler)
 		goto msg_handled;
 
-	if (msg->header.payload_size < entry->min_payload_len) {
-		WARN_ONCE(1, "message too short: msgtype=%d len=%d\n",
-			  msgtype, msg->header.payload_size);
+	if (payload_size < entry->min_payload_len) {
+		WARN_ONCE(1, "message too short: msgtype=%d len=%d\n", msgtype, payload_size);
 		goto msg_handled;
 	}
 
 	if (entry->handler_type	== VMHT_BLOCKING) {
-		ctx = kmalloc(sizeof(*ctx) + msg->header.payload_size,
-			      GFP_ATOMIC);
+		ctx = kmalloc(sizeof(*ctx) + payload_size, GFP_ATOMIC);
 		if (ctx == NULL)
 			return;
 
 		INIT_WORK(&ctx->work, vmbus_onmessage_work);
-		memcpy(&ctx->msg, msg, sizeof(msg->header) +
-		       msg->header.payload_size);
+		memcpy(&ctx->msg, msg, sizeof(msg->header) + payload_size);
 
 		/*
 		 * The host can generate a rescind message while we
-- 
2.25.1


  parent reply	other threads:[~2020-12-02  9:23 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-02  9:22 [PATCH v2 0/7] Drivers: hv: vmbus: More VMBus-hardening changes Andrea Parri (Microsoft)
2020-12-02  9:22 ` [PATCH v2 1/7] Drivers: hv: vmbus: Initialize memory to be sent to the host Andrea Parri (Microsoft)
2020-12-02  9:22 ` [PATCH v2 2/7] Drivers: hv: vmbus: Avoid double fetch of msgtype in vmbus_on_msg_dpc() Andrea Parri (Microsoft)
2020-12-02 12:22   ` Wei Liu
2020-12-02 13:37     ` Andrea Parri
2020-12-02 13:40       ` Wei Liu
2020-12-02 14:14         ` Andrea Parri
2020-12-02  9:22 ` Andrea Parri (Microsoft) [this message]
2020-12-02  9:22 ` [PATCH v2 4/7] Drivers: hv: vmbus: Copy the hv_message object " Andrea Parri (Microsoft)
2020-12-06 18:39   ` Michael Kelley
2020-12-06 21:33     ` Andrea Parri
2020-12-02  9:22 ` [PATCH v2 5/7] Drivers: hv: vmbus: Avoid use-after-free in vmbus_onoffer_rescind() Andrea Parri (Microsoft)
2020-12-02  9:22 ` [PATCH v2 6/7] Drivers: hv: vmbus: Resolve race condition " Andrea Parri (Microsoft)
2020-12-02  9:22 ` [PATCH v2 7/7] Drivers: hv: vmbus: Do not allow overwriting vmbus_connection.channels[] Andrea Parri (Microsoft)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201202092214.13520-4-parri.andrea@gmail.com \
    --to=parri.andrea@gmail.com \
    --cc=haiyangz@microsoft.com \
    --cc=juvazq@microsoft.com \
    --cc=kys@microsoft.com \
    --cc=linux-hyperv@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mikelley@microsoft.com \
    --cc=skarade@microsoft.com \
    --cc=sthemmin@microsoft.com \
    --cc=wei.liu@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.