[PATCH v2 0/3] Drivers: hv: vmbus: vmbus_requestor data structure for VMBus hardening

[PATCH v2 0/3] Drivers: hv: vmbus: vmbus_requestor data structure for VMBus hardening

[Andres Beltran]

Currently, VMbus drivers use pointers into guest memory as request IDs
for interactions with Hyper-V. To be more robust in the face of errors
or malicious behavior from a compromised Hyper-V, avoid exposing
guest memory addresses to Hyper-V. Also avoid Hyper-V giving back a
bad request ID that is then treated as the address of a guest data
structure with no validation. Instead, encapsulate these memory
addresses and provide small integers as request IDs.

The first patch creates the definitions for the data structure, provides
helper methods to generate new IDs and retrieve data, and
allocates/frees the memory needed for vmbus_requestor.

The second and third patches make use of vmbus_requestor to send request
IDs to Hyper-V in storvsc and netvsc respectively.

Thanks.
Andres Beltran

Cc: linux-scsi@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: James E.J. Bottomley <jejb@linux.ibm.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>

Andres Beltran (3):
  Drivers: hv: vmbus: Add vmbus_requestor data structure for VMBus
    hardening
  scsi: storvsc: Use vmbus_requestor to generate transaction IDs for
    VMBus hardening
  hv_netvsc: Use vmbus_requestor to generate transaction IDs for VMBus
    hardening

 drivers/hv/channel.c              | 146 ++++++++++++++++++++++++++++++
 drivers/net/hyperv/hyperv_net.h   |  13 +++
 drivers/net/hyperv/netvsc.c       |  79 +++++++++++++---
 drivers/net/hyperv/rndis_filter.c |   1 +
 drivers/scsi/storvsc_drv.c        |  85 ++++++++++++++---
 include/linux/hyperv.h            |  22 +++++
 6 files changed, 321 insertions(+), 25 deletions(-)

-- 
2.25.1

[PATCH v2 2/3] scsi: storvsc: Use vmbus_requestor to generate transaction IDs for VMBus hardening

[Andres Beltran]

Currently, pointers to guest memory are passed to Hyper-V as
transaction IDs in storvsc. In the face of errors or malicious
behavior in Hyper-V, storvsc should not expose or trust the transaction
IDs returned by Hyper-V to be valid guest memory addresses. Instead,
use small integers generated by vmbus_requestor as requests
(transaction) IDs.

Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: linux-scsi@vger.kernel.org
Signed-off-by: Andres Beltran <lkmlabelt@gmail.com>
---
Changes in v2:
	- Add casts to unsigned long to fix warnings on 32bit.

 drivers/scsi/storvsc_drv.c | 85 +++++++++++++++++++++++++++++++++-----
 1 file changed, 74 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 624467e2590a..6d2df1f0fe6d 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -399,6 +399,7 @@ static int storvsc_timeout = 180;
 static struct scsi_transport_template *fc_transport_template;
 #endif
 
+static struct scsi_host_template scsi_driver;
 static void storvsc_on_channel_callback(void *context);
 
 #define STORVSC_MAX_LUNS_PER_TARGET			255
@@ -698,6 +699,12 @@ static void handle_sc_creation(struct vmbus_channel *new_sc)
 
 	memset(&props, 0, sizeof(struct vmstorage_channel_properties));
 
+	/*
+	 * The size of vmbus_requestor is an upper bound on the number of requests
+	 * that can be in-progress at any one time across all channels.
+	 */
+	new_sc->rqstor_size = scsi_driver.can_queue;
+
 	ret = vmbus_open(new_sc,
 			 storvsc_ringbuffer_size,
 			 storvsc_ringbuffer_size,
@@ -726,6 +733,7 @@ static void  handle_multichannel_storage(struct hv_device *device, int max_chns)
 	struct storvsc_cmd_request *request;
 	struct vstor_packet *vstor_packet;
 	int ret, t;
+	u64 rqst_id;
 
 	/*
 	 * If the number of CPUs is artificially restricted, such as
@@ -760,14 +768,23 @@ static void  handle_multichannel_storage(struct hv_device *device, int max_chns)
 	vstor_packet->flags = REQUEST_COMPLETION_FLAG;
 	vstor_packet->sub_channel_count = num_sc;
 
+	rqst_id = vmbus_next_request_id(&device->channel->requestor,
+					(unsigned long)request);
+	if (rqst_id == VMBUS_RQST_ERROR) {
+		dev_err(dev, "No request id available\n");
+		return;
+	}
+
 	ret = vmbus_sendpacket(device->channel, vstor_packet,
 			       (sizeof(struct vstor_packet) -
 			       vmscsi_size_delta),
-			       (unsigned long)request,
+			       rqst_id,
 			       VM_PKT_DATA_INBAND,
 			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
 
 	if (ret != 0) {
+		/* Reclaim request ID to avoid leak of IDs */
+		vmbus_request_addr(&device->channel->requestor, rqst_id);
 		dev_err(dev, "Failed to create sub-channel: err=%d\n", ret);
 		return;
 	}
@@ -818,20 +835,31 @@ static int storvsc_execute_vstor_op(struct hv_device *device,
 {
 	struct vstor_packet *vstor_packet;
 	int ret, t;
+	u64 rqst_id;
 
 	vstor_packet = &request->vstor_packet;
 
 	init_completion(&request->wait_event);
 	vstor_packet->flags = REQUEST_COMPLETION_FLAG;
 
+	rqst_id = vmbus_next_request_id(&device->channel->requestor,
+					(unsigned long)request);
+	if (rqst_id == VMBUS_RQST_ERROR) {
+		dev_err(&device->device, "No request id available\n");
+		return -EAGAIN;
+	}
+
 	ret = vmbus_sendpacket(device->channel, vstor_packet,
 			       (sizeof(struct vstor_packet) -
 			       vmscsi_size_delta),
-			       (unsigned long)request,
+			       rqst_id,
 			       VM_PKT_DATA_INBAND,
 			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
-	if (ret != 0)
+	if (ret != 0) {
+		/* Reclaim request ID to avoid leak of IDs */
+		vmbus_request_addr(&device->channel->requestor, rqst_id);
 		return ret;
+	}
 
 	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
 	if (t == 0)
@@ -1233,9 +1261,17 @@ static void storvsc_on_channel_callback(void *context)
 	foreach_vmbus_pkt(desc, channel) {
 		void *packet = hv_pkt_data(desc);
 		struct storvsc_cmd_request *request;
+		u64 cmd_rqst;
 
-		request = (struct storvsc_cmd_request *)
-			((unsigned long)desc->trans_id);
+		cmd_rqst = vmbus_request_addr(&channel->requestor,
+					      desc->trans_id);
+		if (cmd_rqst == VMBUS_RQST_ERROR) {
+			dev_err(&device->device,
+				"Incorrect transaction id\n");
+			continue;
+		}
+
+		request = (struct storvsc_cmd_request *)(unsigned long)cmd_rqst;
 
 		if (request == &stor_device->init_request ||
 		    request == &stor_device->reset_request) {
@@ -1256,6 +1292,12 @@ static int storvsc_connect_to_vsp(struct hv_device *device, u32 ring_size,
 
 	memset(&props, 0, sizeof(struct vmstorage_channel_properties));
 
+	/*
+	 * The size of vmbus_requestor is an upper bound on the number of requests
+	 * that can be in-progress at any one time across all channels.
+	 */
+	device->channel->rqstor_size = scsi_driver.can_queue;
+
 	ret = vmbus_open(device->channel,
 			 ring_size,
 			 ring_size,
@@ -1369,6 +1411,7 @@ static int storvsc_do_io(struct hv_device *device,
 	int ret = 0;
 	const struct cpumask *node_mask;
 	int tgt_cpu;
+	u64 rqst_id;
 
 	vstor_packet = &request->vstor_packet;
 	stor_device = get_out_stor_device(device);
@@ -1463,6 +1506,13 @@ static int storvsc_do_io(struct hv_device *device,
 
 	vstor_packet->operation = VSTOR_OPERATION_EXECUTE_SRB;
 
+	rqst_id = vmbus_next_request_id(&outgoing_channel->requestor,
+					(unsigned long)request);
+	if (rqst_id == VMBUS_RQST_ERROR) {
+		dev_err(&device->device, "No request id available\n");
+		return -EAGAIN;
+	}
+
 	if (request->payload->range.len) {
 
 		ret = vmbus_sendpacket_mpb_desc(outgoing_channel,
@@ -1470,18 +1520,21 @@ static int storvsc_do_io(struct hv_device *device,
 				vstor_packet,
 				(sizeof(struct vstor_packet) -
 				vmscsi_size_delta),
-				(unsigned long)request);
+				rqst_id);
 	} else {
 		ret = vmbus_sendpacket(outgoing_channel, vstor_packet,
 			       (sizeof(struct vstor_packet) -
 				vmscsi_size_delta),
-			       (unsigned long)request,
+			       rqst_id,
 			       VM_PKT_DATA_INBAND,
 			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
 	}
 
-	if (ret != 0)
+	if (ret != 0) {
+		/* Reclaim request ID to avoid leak of IDs */
+		vmbus_request_addr(&outgoing_channel->requestor, rqst_id);
 		return ret;
+	}
 
 	atomic_inc(&stor_device->num_outstanding_req);
 
@@ -1562,7 +1615,7 @@ static int storvsc_host_reset_handler(struct scsi_cmnd *scmnd)
 	struct storvsc_cmd_request *request;
 	struct vstor_packet *vstor_packet;
 	int ret, t;
-
+	u64 rqst_id;
 
 	stor_device = get_out_stor_device(device);
 	if (!stor_device)
@@ -1577,14 +1630,24 @@ static int storvsc_host_reset_handler(struct scsi_cmnd *scmnd)
 	vstor_packet->flags = REQUEST_COMPLETION_FLAG;
 	vstor_packet->vm_srb.path_id = stor_device->path_id;
 
+	rqst_id = vmbus_next_request_id(&device->channel->requestor,
+					(unsigned long)&stor_device->reset_request);
+	if (rqst_id == VMBUS_RQST_ERROR) {
+		dev_err(&device->device, "No request id available\n");
+		return FAILED;
+	}
+
 	ret = vmbus_sendpacket(device->channel, vstor_packet,
 			       (sizeof(struct vstor_packet) -
 				vmscsi_size_delta),
-			       (unsigned long)&stor_device->reset_request,
+			       rqst_id,
 			       VM_PKT_DATA_INBAND,
 			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
-	if (ret != 0)
+	if (ret != 0) {
+		/* Reclaim request ID to avoid leak of IDs */
+		vmbus_request_addr(&device->channel->requestor, rqst_id);
 		return FAILED;
+	}
 
 	t = wait_for_completion_timeout(&request->wait_event, 5*HZ);
 	if (t == 0)
-- 
2.25.1