Thread (4 messages) 4 messages, 4 authors, 2020-08-14

RE: [PATCH] hv_netvsc: Add validation for untrusted Hyper-V values

From: Haiyang Zhang <haiyangz@microsoft.com>
Date: 2020-08-02 22:26:17
Also in: lkml, netdev

quoted hunk ↗ jump to hunk
-----Original Message-----
From: Andres Beltran <redacted>
Sent: Tuesday, July 28, 2020 6:53 PM
To: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
[off-list ref]; Stephen Hemminger [off-list ref];
wei.liu@kernel.org
Cc: linux-hyperv@vger.kernel.org; linux-kernel@vger.kernel.org; Michael
Kelley [off-list ref]; parri.andrea@gmail.com; Saruhan
Karademir [off-list ref]; Andres Beltran [off-list ref];
David S . Miller [off-list ref]; Jakub Kicinski [off-list ref];
netdev@vger.kernel.org
Subject: [PATCH] hv_netvsc: Add validation for untrusted Hyper-V values

For additional robustness in the face of Hyper-V errors or malicious
behavior, validate all values that originate from packets that Hyper-V
has sent to the guest in the host-to-guest ring buffer. Ensure that
invalid values cannot cause indexing off the end of an array, or
subvert an existing validation via integer overflow. Ensure that
outgoing packets do not have any leftover guest memory that has not
been zeroed out.

Cc: David S. Miller <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org
Signed-off-by: Andres Beltran <redacted>
---
 drivers/net/hyperv/hyperv_net.h   |  4 ++
 drivers/net/hyperv/netvsc.c       | 99 +++++++++++++++++++++++++++----
 drivers/net/hyperv/netvsc_drv.c   |  7 +++
 drivers/net/hyperv/rndis_filter.c | 73 ++++++++++++++++++++---
 4 files changed, 163 insertions(+), 20 deletions(-)
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index f43b614f2345..7df5943fa46f 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -860,6 +860,10 @@ static inline u32 netvsc_rqstor_size(unsigned long
ringbytes)
 	       ringbytes / NETVSC_MIN_IN_MSG_SIZE;
 }

+#define NETVSC_XFER_HEADER_SIZE(rng_cnt) \
+		(offsetof(struct vmtransfer_page_packet_header, ranges) + \
+		(rng_cnt) * sizeof(struct vmtransfer_page_range))
+
 struct multi_send_data {
 	struct sk_buff *skb; /* skb containing the pkt */
 	struct hv_netvsc_packet *pkt; /* netvsc pkt pending */
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 79b907a29433..7aa5276a1f36 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -398,6 +398,15 @@ static int netvsc_init_buf(struct hv_device *device,
 	net_device->recv_section_size = resp->sections[0].sub_alloc_size;
 	net_device->recv_section_cnt = resp->sections[0].num_sub_allocs;

+	/* Ensure buffer will not overflow */
+	if (net_device->recv_section_size < NETVSC_MTU_MIN ||
(u64)net_device->recv_section_size *
+	    (u64)net_device->recv_section_cnt > (u64)buf_size) {
+		netdev_err(ndev, "invalid recv_section_size %u\n",
+			   net_device->recv_section_size);
+		ret = -EINVAL;
+		goto cleanup;
+	}
+
 	/* Setup receive completion ring.
 	 * Add 1 to the recv_section_cnt because at least one entry in a
 	 * ring buffer has to be empty.
@@ -479,6 +488,12 @@ static int netvsc_init_buf(struct hv_device *device,
 	/* Parse the response */
 	net_device->send_section_size = init_packet->msg.
 				v1_msg.send_send_buf_complete.section_size;
+	if (net_device->send_section_size < NETVSC_MTU_MIN) {
+		netdev_err(ndev, "invalid send_section_size %u\n",
+			   net_device->send_section_size);
+		ret = -EINVAL;
+		goto cleanup;
+	}

 	/* Section count is simply the size divided by the section size. */
 	net_device->send_section_cnt = buf_size / net_device-
quoted
send_section_size;
@@ -770,12 +785,24 @@ static void netvsc_send_completion(struct
net_device *ndev,
 				   int budget)
 {
 	const struct nvsp_message *nvsp_packet = hv_pkt_data(desc);
+	u32 msglen = hv_pkt_datalen(desc);
+
+	/* Ensure packet is big enough to read header fields */
+	if (msglen < sizeof(struct nvsp_message_header)) {
+		netdev_err(ndev, "nvsp_message length too small: %u\n",
msglen);
+		return;
+	}

 	switch (nvsp_packet->hdr.msg_type) {
 	case NVSP_MSG_TYPE_INIT_COMPLETE:
 	case NVSP_MSG1_TYPE_SEND_RECV_BUF_COMPLETE:
 	case NVSP_MSG1_TYPE_SEND_SEND_BUF_COMPLETE:
 	case NVSP_MSG5_TYPE_SUBCHANNEL:
+		if (msglen < sizeof(struct nvsp_message)) {
+			netdev_err(ndev, "nvsp_msg5 length too small: %u\n",
+				   msglen);
+			return;
+		}
struct nvsp_message includes all message types, so its length is the longest type,
The messages from older host version are not necessarily reaching the 
sizeof(struct nvsp_message).

Testing on both new and older hosts are recommended, in case I didn't find out all issues
like this one.

Thanks,
- Haiyang
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help