* [PATCH v8 1/3] Drivers: hv: vmbus: Add vmbus_requestor data structure for VMBus hardening
2020-11-04 15:40 [PATCH v8 0/3] Drivers: hv: vmbus: vmbus_requestor data structure for VMBus hardening Andrea Parri (Microsoft)
@ 2020-11-04 15:40 ` Andrea Parri (Microsoft)
2020-11-05 12:24 ` Dan Carpenter
2020-11-05 12:51 ` Andrea Parri
2020-11-04 15:40 ` [PATCH v8 2/3] scsi: storvsc: Use vmbus_requestor to generate transaction IDs " Andrea Parri (Microsoft)
2020-11-04 15:40 ` [PATCH v8 3/3] hv_netvsc: " Andrea Parri (Microsoft)
2 siblings, 2 replies; 11+ messages in thread
From: Andrea Parri (Microsoft) @ 2020-11-04 15:40 UTC (permalink / raw)
To: linux-kernel
Cc: K . Y . Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
linux-hyperv, Andres Beltran, Michael Kelley, Saruhan Karademir,
Juan Vazquez, Andrea Parri
From: Andres Beltran <lkmlabelt@gmail.com>
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.
Signed-off-by: Andres Beltran <lkmlabelt@gmail.com>
Co-developed-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
---
drivers/hv/channel.c | 174 ++++++++++++++++++++++++++++++++++++--
drivers/hv/hyperv_vmbus.h | 3 +-
drivers/hv/ring_buffer.c | 28 +++++-
include/linux/hyperv.h | 22 +++++
4 files changed, 218 insertions(+), 9 deletions(-)
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index fbdda9938039a..6fb0c76bfbf81 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -503,6 +503,70 @@ int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
}
EXPORT_SYMBOL_GPL(vmbus_establish_gpadl);
+/**
+ * request_arr_init - Allocates memory for the requestor array. Each slot
+ * keeps track of the next available slot in the array. Initially, each
+ * slot points to the next one (as in a Linked List). The last slot
+ * does not point to anything, so its value is U64_MAX by default.
+ * @size The size of the array
+ */
+static u64 *request_arr_init(u32 size)
+{
+ int i;
+ u64 *req_arr;
+
+ req_arr = kcalloc(size, sizeof(u64), GFP_KERNEL);
+ if (!req_arr)
+ return NULL;
+
+ for (i = 0; i < size - 1; i++)
+ req_arr[i] = i + 1;
+
+ /* Last slot (no more available slots) */
+ req_arr[i] = U64_MAX;
+
+ return req_arr;
+}
+
+/*
+ * vmbus_alloc_requestor - Initializes @rqstor's fields.
+ * Index 0 is the first free slot
+ * @size: Size of the requestor array
+ */
+static int vmbus_alloc_requestor(struct vmbus_requestor *rqstor, u32 size)
+{
+ u64 *rqst_arr;
+ unsigned long *bitmap;
+
+ rqst_arr = request_arr_init(size);
+ if (!rqst_arr)
+ return -ENOMEM;
+
+ bitmap = bitmap_zalloc(size, GFP_KERNEL);
+ if (!bitmap) {
+ kfree(rqst_arr);
+ return -ENOMEM;
+ }
+
+ rqstor->req_arr = rqst_arr;
+ rqstor->req_bitmap = bitmap;
+ rqstor->size = size;
+ rqstor->next_request_id = 0;
+ spin_lock_init(&rqstor->req_lock);
+
+ return 0;
+}
+
+/*
+ * vmbus_free_requestor - Frees memory allocated for @rqstor
+ * @rqstor: Pointer to the requestor struct
+ */
+static void vmbus_free_requestor(struct vmbus_requestor *rqstor)
+{
+ kfree(rqstor->req_arr);
+ bitmap_free(rqstor->req_bitmap);
+}
+
static int __vmbus_open(struct vmbus_channel *newchannel,
void *userdata, u32 userdatalen,
void (*onchannelcallback)(void *context), void *context)
@@ -523,6 +587,12 @@ static int __vmbus_open(struct vmbus_channel *newchannel,
if (newchannel->state != CHANNEL_OPEN_STATE)
return -EINVAL;
+ /* Create and init requestor */
+ if (newchannel->rqstor_size) {
+ if (vmbus_alloc_requestor(&newchannel->requestor, newchannel->rqstor_size))
+ return -ENOMEM;
+ }
+
newchannel->state = CHANNEL_OPENING_STATE;
newchannel->onchannel_callback = onchannelcallback;
newchannel->channel_callback_context = context;
@@ -626,6 +696,7 @@ static int __vmbus_open(struct vmbus_channel *newchannel,
error_clean_ring:
hv_ringbuffer_cleanup(&newchannel->outbound);
hv_ringbuffer_cleanup(&newchannel->inbound);
+ vmbus_free_requestor(&newchannel->requestor);
newchannel->state = CHANNEL_OPEN_STATE;
return err;
}
@@ -808,6 +879,9 @@ static int vmbus_close_internal(struct vmbus_channel *channel)
channel->ringbuffer_gpadlhandle = 0;
}
+ if (!ret)
+ vmbus_free_requestor(&channel->requestor);
+
return ret;
}
@@ -888,7 +962,7 @@ int vmbus_sendpacket(struct vmbus_channel *channel, void *buffer,
/* in 8-bytes granularity */
desc.offset8 = sizeof(struct vmpacket_descriptor) >> 3;
desc.len8 = (u16)(packetlen_aligned >> 3);
- desc.trans_id = requestid;
+ desc.trans_id = VMBUS_RQST_ERROR; /* will be updated in hv_ringbuffer_write() */
bufferlist[0].iov_base = &desc;
bufferlist[0].iov_len = sizeof(struct vmpacket_descriptor);
@@ -897,7 +971,7 @@ int vmbus_sendpacket(struct vmbus_channel *channel, void *buffer,
bufferlist[2].iov_base = &aligned_data;
bufferlist[2].iov_len = (packetlen_aligned - packetlen);
- return hv_ringbuffer_write(channel, bufferlist, num_vecs);
+ return hv_ringbuffer_write(channel, bufferlist, num_vecs, requestid);
}
EXPORT_SYMBOL(vmbus_sendpacket);
@@ -939,7 +1013,7 @@ int vmbus_sendpacket_pagebuffer(struct vmbus_channel *channel,
desc.flags = VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED;
desc.dataoffset8 = descsize >> 3; /* in 8-bytes granularity */
desc.length8 = (u16)(packetlen_aligned >> 3);
- desc.transactionid = requestid;
+ desc.transactionid = VMBUS_RQST_ERROR; /* will be updated in hv_ringbuffer_write() */
desc.reserved = 0;
desc.rangecount = pagecount;
@@ -956,7 +1030,7 @@ int vmbus_sendpacket_pagebuffer(struct vmbus_channel *channel,
bufferlist[2].iov_base = &aligned_data;
bufferlist[2].iov_len = (packetlen_aligned - packetlen);
- return hv_ringbuffer_write(channel, bufferlist, 3);
+ return hv_ringbuffer_write(channel, bufferlist, 3, requestid);
}
EXPORT_SYMBOL_GPL(vmbus_sendpacket_pagebuffer);
@@ -983,7 +1057,7 @@ int vmbus_sendpacket_mpb_desc(struct vmbus_channel *channel,
desc->flags = VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED;
desc->dataoffset8 = desc_size >> 3; /* in 8-bytes granularity */
desc->length8 = (u16)(packetlen_aligned >> 3);
- desc->transactionid = requestid;
+ desc->transactionid = VMBUS_RQST_ERROR; /* will be updated in hv_ringbuffer_write() */
desc->reserved = 0;
desc->rangecount = 1;
@@ -994,7 +1068,7 @@ int vmbus_sendpacket_mpb_desc(struct vmbus_channel *channel,
bufferlist[2].iov_base = &aligned_data;
bufferlist[2].iov_len = (packetlen_aligned - packetlen);
- return hv_ringbuffer_write(channel, bufferlist, 3);
+ return hv_ringbuffer_write(channel, bufferlist, 3, requestid);
}
EXPORT_SYMBOL_GPL(vmbus_sendpacket_mpb_desc);
@@ -1042,3 +1116,91 @@ int vmbus_recvpacket_raw(struct vmbus_channel *channel, void *buffer,
buffer_actual_len, requestid, true);
}
EXPORT_SYMBOL_GPL(vmbus_recvpacket_raw);
+
+/*
+ * vmbus_next_request_id - Returns a new request id. It is also
+ * the index at which the guest memory address is stored.
+ * Uses a spin lock to avoid race conditions.
+ * @rqstor: Pointer to the requestor struct
+ * @rqst_add: Guest memory address to be stored in the array
+ */
+u64 vmbus_next_request_id(struct vmbus_requestor *rqstor, u64 rqst_addr)
+{
+ unsigned long flags;
+ u64 current_id;
+ const struct vmbus_channel *channel =
+ container_of(rqstor, const struct vmbus_channel, requestor);
+
+ /* Check rqstor has been initialized */
+ if (!channel->rqstor_size)
+ return VMBUS_NO_RQSTOR;
+
+ spin_lock_irqsave(&rqstor->req_lock, flags);
+ current_id = rqstor->next_request_id;
+
+ /* Requestor array is full */
+ if (current_id >= rqstor->size) {
+ spin_unlock_irqrestore(&rqstor->req_lock, flags);
+ return VMBUS_RQST_ERROR;
+ }
+
+ rqstor->next_request_id = rqstor->req_arr[current_id];
+ rqstor->req_arr[current_id] = rqst_addr;
+
+ /* The already held spin lock provides atomicity */
+ bitmap_set(rqstor->req_bitmap, current_id, 1);
+
+ spin_unlock_irqrestore(&rqstor->req_lock, flags);
+
+ /*
+ * Cannot return an ID of 0, which is reserved for an unsolicited
+ * message from Hyper-V.
+ */
+ return current_id + 1;
+}
+EXPORT_SYMBOL_GPL(vmbus_next_request_id);
+
+/*
+ * vmbus_request_addr - Returns the memory address stored at @trans_id
+ * in @rqstor. Uses a spin lock to avoid race conditions.
+ * @rqstor: Pointer to the requestor struct
+ * @trans_id: Request id sent back from Hyper-V. Becomes the requestor's
+ * next request id.
+ */
+u64 vmbus_request_addr(struct vmbus_requestor *rqstor, u64 trans_id)
+{
+ unsigned long flags;
+ u64 req_addr;
+ const struct vmbus_channel *channel =
+ container_of(rqstor, const struct vmbus_channel, requestor);
+
+ /* Check rqstor has been initialized */
+ if (!channel->rqstor_size)
+ return VMBUS_NO_RQSTOR;
+
+ /* Hyper-V can send an unsolicited message with ID of 0 */
+ if (!trans_id)
+ return trans_id;
+
+ spin_lock_irqsave(&rqstor->req_lock, flags);
+
+ /* Data corresponding to trans_id is stored at trans_id - 1 */
+ trans_id--;
+
+ /* Invalid trans_id */
+ if (trans_id >= rqstor->size || !test_bit(trans_id, rqstor->req_bitmap)) {
+ spin_unlock_irqrestore(&rqstor->req_lock, flags);
+ return VMBUS_RQST_ERROR;
+ }
+
+ req_addr = rqstor->req_arr[trans_id];
+ rqstor->req_arr[trans_id] = rqstor->next_request_id;
+ rqstor->next_request_id = trans_id;
+
+ /* The already held spin lock provides atomicity */
+ bitmap_clear(rqstor->req_bitmap, trans_id, 1);
+
+ spin_unlock_irqrestore(&rqstor->req_lock, flags);
+ return req_addr;
+}
+EXPORT_SYMBOL_GPL(vmbus_request_addr);
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 40e2b9f91163c..02f3e89888366 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -179,7 +179,8 @@ int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info,
void hv_ringbuffer_cleanup(struct hv_ring_buffer_info *ring_info);
int hv_ringbuffer_write(struct vmbus_channel *channel,
- const struct kvec *kv_list, u32 kv_count);
+ const struct kvec *kv_list, u32 kv_count,
+ u64 requestid);
int hv_ringbuffer_read(struct vmbus_channel *channel,
void *buffer, u32 buflen, u32 *buffer_actual_len,
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 356e22159e834..e55d3c75e148c 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -248,7 +248,8 @@ void hv_ringbuffer_cleanup(struct hv_ring_buffer_info *ring_info)
/* Write to the ring buffer. */
int hv_ringbuffer_write(struct vmbus_channel *channel,
- const struct kvec *kv_list, u32 kv_count)
+ const struct kvec *kv_list, u32 kv_count,
+ u64 requestid)
{
int i;
u32 bytes_avail_towrite;
@@ -258,6 +259,8 @@ int hv_ringbuffer_write(struct vmbus_channel *channel,
u64 prev_indices;
unsigned long flags;
struct hv_ring_buffer_info *outring_info = &channel->outbound;
+ struct vmpacket_descriptor *desc = kv_list[0].iov_base;
+ u64 rqst_id = VMBUS_NO_RQSTOR;
if (channel->rescind)
return -ENODEV;
@@ -300,6 +303,22 @@ int hv_ringbuffer_write(struct vmbus_channel *channel,
kv_list[i].iov_len);
}
+ /*
+ * Allocate the request ID after the data has been copied into the
+ * ring buffer. Once this request ID is allocated, the completion
+ * path could find the data and free it.
+ */
+
+ if (desc->flags == VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED) {
+ rqst_id = vmbus_next_request_id(&channel->requestor, requestid);
+ if (rqst_id == VMBUS_RQST_ERROR) {
+ pr_err("No request id available\n");
+ return -EAGAIN;
+ }
+ }
+ desc = hv_get_ring_buffer(outring_info) + old_write;
+ desc->trans_id = (rqst_id == VMBUS_NO_RQSTOR) ? requestid : rqst_id;
+
/* Set previous packet start */
prev_indices = hv_get_ring_bufferindices(outring_info);
@@ -319,8 +338,13 @@ int hv_ringbuffer_write(struct vmbus_channel *channel,
hv_signal_on_write(old_write, channel);
- if (channel->rescind)
+ if (channel->rescind) {
+ if (rqst_id != VMBUS_NO_RQSTOR) {
+ /* Reclaim request ID to avoid leak of IDs */
+ vmbus_request_addr(&channel->requestor, rqst_id);
+ }
return -ENODEV;
+ }
return 0;
}
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 1ce131f29f3b4..5b6d5c4e37110 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -764,6 +764,22 @@ enum vmbus_device_type {
HV_UNKNOWN,
};
+/*
+ * Provides request ids for VMBus. Encapsulates guest memory
+ * addresses and stores the next available slot in req_arr
+ * to generate new ids in constant time.
+ */
+struct vmbus_requestor {
+ u64 *req_arr;
+ unsigned long *req_bitmap; /* is a given slot available? */
+ u32 size;
+ u64 next_request_id;
+ spinlock_t req_lock; /* provides atomicity */
+};
+
+#define VMBUS_NO_RQSTOR U64_MAX
+#define VMBUS_RQST_ERROR (U64_MAX - 1)
+
struct vmbus_device {
u16 dev_type;
guid_t guid;
@@ -988,8 +1004,14 @@ struct vmbus_channel {
u32 fuzz_testing_interrupt_delay;
u32 fuzz_testing_message_delay;
+ /* request/transaction ids for VMBus */
+ struct vmbus_requestor requestor;
+ u32 rqstor_size;
};
+u64 vmbus_next_request_id(struct vmbus_requestor *rqstor, u64 rqst_addr);
+u64 vmbus_request_addr(struct vmbus_requestor *rqstor, u64 trans_id);
+
static inline bool is_hvsock_channel(const struct vmbus_channel *c)
{
return !!(c->offermsg.offer.chn_flags &
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v8 1/3] Drivers: hv: vmbus: Add vmbus_requestor data structure for VMBus hardening
2020-11-04 15:40 ` [PATCH v8 1/3] Drivers: hv: vmbus: Add " Andrea Parri (Microsoft)
@ 2020-11-05 12:24 ` Dan Carpenter
2020-11-05 12:51 ` Andrea Parri
1 sibling, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2020-11-05 12:24 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 12409 bytes --]
Hi Andrea,
url: https://github.com/0day-ci/linux/commits/Andrea-Parri-Microsoft/Drivers-hv-vmbus-vmbus_requestor-data-structure-for-VMBus-hardening/20201104-234406
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: i386-randconfig-m031-20201104 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/hv/ring_buffer.c:349 hv_ringbuffer_write() warn: inconsistent returns 'outring_info->ring_lock'.
drivers/hv/ring_buffer.c:349 hv_ringbuffer_write() warn: inconsistent returns 'flags'.
vim +349 drivers/hv/ring_buffer.c
1f6ee4e7d83586c drivers/hv/ring_buffer.c K. Y. Srinivasan 2016-11-06 250 int hv_ringbuffer_write(struct vmbus_channel *channel,
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 251 const struct kvec *kv_list, u32 kv_count,
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 252 u64 requestid)
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 253 {
2c616a8b6bd3b51 drivers/hv/ring_buffer.c Stephen Hemminger 2017-03-04 254 int i;
fc8c72ebfa7547c drivers/staging/hv/ring_buffer.c Haiyang Zhang 2010-11-08 255 u32 bytes_avail_towrite;
2c616a8b6bd3b51 drivers/hv/ring_buffer.c Stephen Hemminger 2017-03-04 256 u32 totalbytes_towrite = sizeof(u64);
66a605438146923 drivers/staging/hv/ring_buffer.c K. Y. Srinivasan 2011-05-10 257 u32 next_write_location;
98fa8cf4bcc79cb drivers/hv/ring_buffer.c K. Y. Srinivasan 2012-12-01 258 u32 old_write;
2c616a8b6bd3b51 drivers/hv/ring_buffer.c Stephen Hemminger 2017-03-04 259 u64 prev_indices;
2c616a8b6bd3b51 drivers/hv/ring_buffer.c Stephen Hemminger 2017-03-04 260 unsigned long flags;
1f6ee4e7d83586c drivers/hv/ring_buffer.c K. Y. Srinivasan 2016-11-06 261 struct hv_ring_buffer_info *outring_info = &channel->outbound;
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 262 struct vmpacket_descriptor *desc = kv_list[0].iov_base;
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 263 u64 rqst_id = VMBUS_NO_RQSTOR;
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 264
e7e97dd8b77ee73 drivers/hv/ring_buffer.c K. Y. Srinivasan 2016-12-07 265 if (channel->rescind)
e7e97dd8b77ee73 drivers/hv/ring_buffer.c K. Y. Srinivasan 2016-12-07 266 return -ENODEV;
e7e97dd8b77ee73 drivers/hv/ring_buffer.c K. Y. Srinivasan 2016-12-07 267
011a7c3cc3aa60c drivers/hv/ring_buffer.c K. Y. Srinivasan 2014-02-01 268 for (i = 0; i < kv_count; i++)
011a7c3cc3aa60c drivers/hv/ring_buffer.c K. Y. Srinivasan 2014-02-01 269 totalbytes_towrite += kv_list[i].iov_len;
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 270
fc8c72ebfa7547c drivers/staging/hv/ring_buffer.c Haiyang Zhang 2010-11-08 271 spin_lock_irqsave(&outring_info->ring_lock, flags);
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 272
a6341f000024cdf drivers/hv/ring_buffer.c K. Y. Srinivasan 2016-04-02 273 bytes_avail_towrite = hv_get_bytes_to_write(outring_info);
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 274
822f18d4d3e9d4e drivers/hv/ring_buffer.c Vitaly Kuznetsov 2015-12-14 275 /*
822f18d4d3e9d4e drivers/hv/ring_buffer.c Vitaly Kuznetsov 2015-12-14 276 * If there is only room for the packet, assume it is full.
822f18d4d3e9d4e drivers/hv/ring_buffer.c Vitaly Kuznetsov 2015-12-14 277 * Otherwise, the next time around, we think the ring buffer
822f18d4d3e9d4e drivers/hv/ring_buffer.c Vitaly Kuznetsov 2015-12-14 278 * is empty since the read index == write index.
822f18d4d3e9d4e drivers/hv/ring_buffer.c Vitaly Kuznetsov 2015-12-14 279 */
fc8c72ebfa7547c drivers/staging/hv/ring_buffer.c Haiyang Zhang 2010-11-08 280 if (bytes_avail_towrite <= totalbytes_towrite) {
396ae57ef1ef978 drivers/hv/ring_buffer.c Kimberly Brown 2019-02-04 281 ++channel->out_full_total;
396ae57ef1ef978 drivers/hv/ring_buffer.c Kimberly Brown 2019-02-04 282
396ae57ef1ef978 drivers/hv/ring_buffer.c Kimberly Brown 2019-02-04 283 if (!channel->out_full_flag) {
396ae57ef1ef978 drivers/hv/ring_buffer.c Kimberly Brown 2019-02-04 284 ++channel->out_full_first;
396ae57ef1ef978 drivers/hv/ring_buffer.c Kimberly Brown 2019-02-04 285 channel->out_full_flag = true;
396ae57ef1ef978 drivers/hv/ring_buffer.c Kimberly Brown 2019-02-04 286 }
396ae57ef1ef978 drivers/hv/ring_buffer.c Kimberly Brown 2019-02-04 287
fc8c72ebfa7547c drivers/staging/hv/ring_buffer.c Haiyang Zhang 2010-11-08 288 spin_unlock_irqrestore(&outring_info->ring_lock, flags);
d2598f017f02bbc drivers/staging/hv/ring_buffer.c K. Y. Srinivasan 2011-08-25 289 return -EAGAIN;
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 290 }
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 291
396ae57ef1ef978 drivers/hv/ring_buffer.c Kimberly Brown 2019-02-04 292 channel->out_full_flag = false;
396ae57ef1ef978 drivers/hv/ring_buffer.c Kimberly Brown 2019-02-04 293
454f18a963cf651 drivers/staging/hv/RingBuffer.c Bill Pemberton 2009-07-27 294 /* Write to the ring buffer */
2b8a912ef2713a7 drivers/staging/hv/ring_buffer.c K. Y. Srinivasan 2011-05-10 295 next_write_location = hv_get_next_write_location(outring_info);
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 296
98fa8cf4bcc79cb drivers/hv/ring_buffer.c K. Y. Srinivasan 2012-12-01 297 old_write = next_write_location;
98fa8cf4bcc79cb drivers/hv/ring_buffer.c K. Y. Srinivasan 2012-12-01 298
011a7c3cc3aa60c drivers/hv/ring_buffer.c K. Y. Srinivasan 2014-02-01 299 for (i = 0; i < kv_count; i++) {
2b8a912ef2713a7 drivers/staging/hv/ring_buffer.c K. Y. Srinivasan 2011-05-10 300 next_write_location = hv_copyto_ringbuffer(outring_info,
fc8c72ebfa7547c drivers/staging/hv/ring_buffer.c Haiyang Zhang 2010-11-08 301 next_write_location,
011a7c3cc3aa60c drivers/hv/ring_buffer.c K. Y. Srinivasan 2014-02-01 302 kv_list[i].iov_base,
011a7c3cc3aa60c drivers/hv/ring_buffer.c K. Y. Srinivasan 2014-02-01 303 kv_list[i].iov_len);
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 304 }
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 305
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 306 /*
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 307 * Allocate the request ID after the data has been copied into the
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 308 * ring buffer. Once this request ID is allocated, the completion
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 309 * path could find the data and free it.
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 310 */
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 311
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 312 if (desc->flags == VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED) {
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 313 rqst_id = vmbus_next_request_id(&channel->requestor, requestid);
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 314 if (rqst_id == VMBUS_RQST_ERROR) {
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 315 pr_err("No request id available\n");
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 316 return -EAGAIN;
Need to call spin_unlock_irqrestore(&outring_info->ring_lock, flags);
before returning.
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 317 }
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 318 }
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 319 desc = hv_get_ring_buffer(outring_info) + old_write;
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 320 desc->trans_id = (rqst_id == VMBUS_NO_RQSTOR) ? requestid : rqst_id;
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 321
454f18a963cf651 drivers/staging/hv/RingBuffer.c Bill Pemberton 2009-07-27 322 /* Set previous packet start */
2b8a912ef2713a7 drivers/staging/hv/ring_buffer.c K. Y. Srinivasan 2011-05-10 323 prev_indices = hv_get_ring_bufferindices(outring_info);
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 324
2b8a912ef2713a7 drivers/staging/hv/ring_buffer.c K. Y. Srinivasan 2011-05-10 325 next_write_location = hv_copyto_ringbuffer(outring_info,
fc8c72ebfa7547c drivers/staging/hv/ring_buffer.c Haiyang Zhang 2010-11-08 326 next_write_location,
fc8c72ebfa7547c drivers/staging/hv/ring_buffer.c Haiyang Zhang 2010-11-08 327 &prev_indices,
59471438ccbebaf drivers/staging/hv/RingBuffer.c Greg Kroah-Hartman 2009-07-14 328 sizeof(u64));
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 329
98fa8cf4bcc79cb drivers/hv/ring_buffer.c K. Y. Srinivasan 2012-12-01 330 /* Issue a full memory barrier before updating the write index */
dcd0eeca4454d5c drivers/hv/ring_buffer.c K. Y. Srinivasan 2016-04-02 331 virt_mb();
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 332
454f18a963cf651 drivers/staging/hv/RingBuffer.c Bill Pemberton 2009-07-27 333 /* Now, update the write location */
2b8a912ef2713a7 drivers/staging/hv/ring_buffer.c K. Y. Srinivasan 2011-05-10 334 hv_set_next_write_location(outring_info, next_write_location);
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 335
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 336
fc8c72ebfa7547c drivers/staging/hv/ring_buffer.c Haiyang Zhang 2010-11-08 337 spin_unlock_irqrestore(&outring_info->ring_lock, flags);
98fa8cf4bcc79cb drivers/hv/ring_buffer.c K. Y. Srinivasan 2012-12-01 338
b103a56f80878b4 drivers/hv/ring_buffer.c Stephen Hemminger 2017-02-05 339 hv_signal_on_write(old_write, channel);
e7e97dd8b77ee73 drivers/hv/ring_buffer.c K. Y. Srinivasan 2016-12-07 340
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 341 if (channel->rescind) {
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 342 if (rqst_id != VMBUS_NO_RQSTOR) {
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 343 /* Reclaim request ID to avoid leak of IDs */
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 344 vmbus_request_addr(&channel->requestor, rqst_id);
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 345 }
e7e97dd8b77ee73 drivers/hv/ring_buffer.c K. Y. Srinivasan 2016-12-07 346 return -ENODEV;
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 347 }
e7e97dd8b77ee73 drivers/hv/ring_buffer.c K. Y. Srinivasan 2016-12-07 348
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 @349 return 0;
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 350 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 37597 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v8 1/3] Drivers: hv: vmbus: Add vmbus_requestor data structure for VMBus hardening
@ 2020-11-05 12:24 ` Dan Carpenter
0 siblings, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2020-11-05 12:24 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 12409 bytes --]
Hi Andrea,
url: https://github.com/0day-ci/linux/commits/Andrea-Parri-Microsoft/Drivers-hv-vmbus-vmbus_requestor-data-structure-for-VMBus-hardening/20201104-234406
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: i386-randconfig-m031-20201104 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/hv/ring_buffer.c:349 hv_ringbuffer_write() warn: inconsistent returns 'outring_info->ring_lock'.
drivers/hv/ring_buffer.c:349 hv_ringbuffer_write() warn: inconsistent returns 'flags'.
vim +349 drivers/hv/ring_buffer.c
1f6ee4e7d83586c drivers/hv/ring_buffer.c K. Y. Srinivasan 2016-11-06 250 int hv_ringbuffer_write(struct vmbus_channel *channel,
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 251 const struct kvec *kv_list, u32 kv_count,
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 252 u64 requestid)
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 253 {
2c616a8b6bd3b51 drivers/hv/ring_buffer.c Stephen Hemminger 2017-03-04 254 int i;
fc8c72ebfa7547c drivers/staging/hv/ring_buffer.c Haiyang Zhang 2010-11-08 255 u32 bytes_avail_towrite;
2c616a8b6bd3b51 drivers/hv/ring_buffer.c Stephen Hemminger 2017-03-04 256 u32 totalbytes_towrite = sizeof(u64);
66a605438146923 drivers/staging/hv/ring_buffer.c K. Y. Srinivasan 2011-05-10 257 u32 next_write_location;
98fa8cf4bcc79cb drivers/hv/ring_buffer.c K. Y. Srinivasan 2012-12-01 258 u32 old_write;
2c616a8b6bd3b51 drivers/hv/ring_buffer.c Stephen Hemminger 2017-03-04 259 u64 prev_indices;
2c616a8b6bd3b51 drivers/hv/ring_buffer.c Stephen Hemminger 2017-03-04 260 unsigned long flags;
1f6ee4e7d83586c drivers/hv/ring_buffer.c K. Y. Srinivasan 2016-11-06 261 struct hv_ring_buffer_info *outring_info = &channel->outbound;
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 262 struct vmpacket_descriptor *desc = kv_list[0].iov_base;
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 263 u64 rqst_id = VMBUS_NO_RQSTOR;
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 264
e7e97dd8b77ee73 drivers/hv/ring_buffer.c K. Y. Srinivasan 2016-12-07 265 if (channel->rescind)
e7e97dd8b77ee73 drivers/hv/ring_buffer.c K. Y. Srinivasan 2016-12-07 266 return -ENODEV;
e7e97dd8b77ee73 drivers/hv/ring_buffer.c K. Y. Srinivasan 2016-12-07 267
011a7c3cc3aa60c drivers/hv/ring_buffer.c K. Y. Srinivasan 2014-02-01 268 for (i = 0; i < kv_count; i++)
011a7c3cc3aa60c drivers/hv/ring_buffer.c K. Y. Srinivasan 2014-02-01 269 totalbytes_towrite += kv_list[i].iov_len;
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 270
fc8c72ebfa7547c drivers/staging/hv/ring_buffer.c Haiyang Zhang 2010-11-08 271 spin_lock_irqsave(&outring_info->ring_lock, flags);
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 272
a6341f000024cdf drivers/hv/ring_buffer.c K. Y. Srinivasan 2016-04-02 273 bytes_avail_towrite = hv_get_bytes_to_write(outring_info);
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 274
822f18d4d3e9d4e drivers/hv/ring_buffer.c Vitaly Kuznetsov 2015-12-14 275 /*
822f18d4d3e9d4e drivers/hv/ring_buffer.c Vitaly Kuznetsov 2015-12-14 276 * If there is only room for the packet, assume it is full.
822f18d4d3e9d4e drivers/hv/ring_buffer.c Vitaly Kuznetsov 2015-12-14 277 * Otherwise, the next time around, we think the ring buffer
822f18d4d3e9d4e drivers/hv/ring_buffer.c Vitaly Kuznetsov 2015-12-14 278 * is empty since the read index == write index.
822f18d4d3e9d4e drivers/hv/ring_buffer.c Vitaly Kuznetsov 2015-12-14 279 */
fc8c72ebfa7547c drivers/staging/hv/ring_buffer.c Haiyang Zhang 2010-11-08 280 if (bytes_avail_towrite <= totalbytes_towrite) {
396ae57ef1ef978 drivers/hv/ring_buffer.c Kimberly Brown 2019-02-04 281 ++channel->out_full_total;
396ae57ef1ef978 drivers/hv/ring_buffer.c Kimberly Brown 2019-02-04 282
396ae57ef1ef978 drivers/hv/ring_buffer.c Kimberly Brown 2019-02-04 283 if (!channel->out_full_flag) {
396ae57ef1ef978 drivers/hv/ring_buffer.c Kimberly Brown 2019-02-04 284 ++channel->out_full_first;
396ae57ef1ef978 drivers/hv/ring_buffer.c Kimberly Brown 2019-02-04 285 channel->out_full_flag = true;
396ae57ef1ef978 drivers/hv/ring_buffer.c Kimberly Brown 2019-02-04 286 }
396ae57ef1ef978 drivers/hv/ring_buffer.c Kimberly Brown 2019-02-04 287
fc8c72ebfa7547c drivers/staging/hv/ring_buffer.c Haiyang Zhang 2010-11-08 288 spin_unlock_irqrestore(&outring_info->ring_lock, flags);
d2598f017f02bbc drivers/staging/hv/ring_buffer.c K. Y. Srinivasan 2011-08-25 289 return -EAGAIN;
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 290 }
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 291
396ae57ef1ef978 drivers/hv/ring_buffer.c Kimberly Brown 2019-02-04 292 channel->out_full_flag = false;
396ae57ef1ef978 drivers/hv/ring_buffer.c Kimberly Brown 2019-02-04 293
454f18a963cf651 drivers/staging/hv/RingBuffer.c Bill Pemberton 2009-07-27 294 /* Write to the ring buffer */
2b8a912ef2713a7 drivers/staging/hv/ring_buffer.c K. Y. Srinivasan 2011-05-10 295 next_write_location = hv_get_next_write_location(outring_info);
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 296
98fa8cf4bcc79cb drivers/hv/ring_buffer.c K. Y. Srinivasan 2012-12-01 297 old_write = next_write_location;
98fa8cf4bcc79cb drivers/hv/ring_buffer.c K. Y. Srinivasan 2012-12-01 298
011a7c3cc3aa60c drivers/hv/ring_buffer.c K. Y. Srinivasan 2014-02-01 299 for (i = 0; i < kv_count; i++) {
2b8a912ef2713a7 drivers/staging/hv/ring_buffer.c K. Y. Srinivasan 2011-05-10 300 next_write_location = hv_copyto_ringbuffer(outring_info,
fc8c72ebfa7547c drivers/staging/hv/ring_buffer.c Haiyang Zhang 2010-11-08 301 next_write_location,
011a7c3cc3aa60c drivers/hv/ring_buffer.c K. Y. Srinivasan 2014-02-01 302 kv_list[i].iov_base,
011a7c3cc3aa60c drivers/hv/ring_buffer.c K. Y. Srinivasan 2014-02-01 303 kv_list[i].iov_len);
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 304 }
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 305
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 306 /*
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 307 * Allocate the request ID after the data has been copied into the
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 308 * ring buffer. Once this request ID is allocated, the completion
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 309 * path could find the data and free it.
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 310 */
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 311
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 312 if (desc->flags == VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED) {
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 313 rqst_id = vmbus_next_request_id(&channel->requestor, requestid);
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 314 if (rqst_id == VMBUS_RQST_ERROR) {
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 315 pr_err("No request id available\n");
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 316 return -EAGAIN;
Need to call spin_unlock_irqrestore(&outring_info->ring_lock, flags);
before returning.
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 317 }
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 318 }
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 319 desc = hv_get_ring_buffer(outring_info) + old_write;
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 320 desc->trans_id = (rqst_id == VMBUS_NO_RQSTOR) ? requestid : rqst_id;
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 321
454f18a963cf651 drivers/staging/hv/RingBuffer.c Bill Pemberton 2009-07-27 322 /* Set previous packet start */
2b8a912ef2713a7 drivers/staging/hv/ring_buffer.c K. Y. Srinivasan 2011-05-10 323 prev_indices = hv_get_ring_bufferindices(outring_info);
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 324
2b8a912ef2713a7 drivers/staging/hv/ring_buffer.c K. Y. Srinivasan 2011-05-10 325 next_write_location = hv_copyto_ringbuffer(outring_info,
fc8c72ebfa7547c drivers/staging/hv/ring_buffer.c Haiyang Zhang 2010-11-08 326 next_write_location,
fc8c72ebfa7547c drivers/staging/hv/ring_buffer.c Haiyang Zhang 2010-11-08 327 &prev_indices,
59471438ccbebaf drivers/staging/hv/RingBuffer.c Greg Kroah-Hartman 2009-07-14 328 sizeof(u64));
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 329
98fa8cf4bcc79cb drivers/hv/ring_buffer.c K. Y. Srinivasan 2012-12-01 330 /* Issue a full memory barrier before updating the write index */
dcd0eeca4454d5c drivers/hv/ring_buffer.c K. Y. Srinivasan 2016-04-02 331 virt_mb();
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 332
454f18a963cf651 drivers/staging/hv/RingBuffer.c Bill Pemberton 2009-07-27 333 /* Now, update the write location */
2b8a912ef2713a7 drivers/staging/hv/ring_buffer.c K. Y. Srinivasan 2011-05-10 334 hv_set_next_write_location(outring_info, next_write_location);
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 335
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 336
fc8c72ebfa7547c drivers/staging/hv/ring_buffer.c Haiyang Zhang 2010-11-08 337 spin_unlock_irqrestore(&outring_info->ring_lock, flags);
98fa8cf4bcc79cb drivers/hv/ring_buffer.c K. Y. Srinivasan 2012-12-01 338
b103a56f80878b4 drivers/hv/ring_buffer.c Stephen Hemminger 2017-02-05 339 hv_signal_on_write(old_write, channel);
e7e97dd8b77ee73 drivers/hv/ring_buffer.c K. Y. Srinivasan 2016-12-07 340
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 341 if (channel->rescind) {
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 342 if (rqst_id != VMBUS_NO_RQSTOR) {
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 343 /* Reclaim request ID to avoid leak of IDs */
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 344 vmbus_request_addr(&channel->requestor, rqst_id);
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 345 }
e7e97dd8b77ee73 drivers/hv/ring_buffer.c K. Y. Srinivasan 2016-12-07 346 return -ENODEV;
4d6a4de24f39114 drivers/hv/ring_buffer.c Andres Beltran 2020-11-04 347 }
e7e97dd8b77ee73 drivers/hv/ring_buffer.c K. Y. Srinivasan 2016-12-07 348
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 @349 return 0;
3e7ee4902fe6996 drivers/staging/hv/RingBuffer.c Hank Janssen 2009-07-13 350 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 37597 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v8 1/3] Drivers: hv: vmbus: Add vmbus_requestor data structure for VMBus hardening
2020-11-04 15:40 ` [PATCH v8 1/3] Drivers: hv: vmbus: Add " Andrea Parri (Microsoft)
2020-11-05 12:24 ` Dan Carpenter
@ 2020-11-05 12:51 ` Andrea Parri
1 sibling, 0 replies; 11+ messages in thread
From: Andrea Parri @ 2020-11-05 12:51 UTC (permalink / raw)
To: linux-kernel
Cc: K . Y . Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
linux-hyperv, Andres Beltran, Michael Kelley, Saruhan Karademir,
Juan Vazquez, Dan Carpenter, lkp
> @@ -300,6 +303,22 @@ int hv_ringbuffer_write(struct vmbus_channel *channel,
> kv_list[i].iov_len);
> }
>
> + /*
> + * Allocate the request ID after the data has been copied into the
> + * ring buffer. Once this request ID is allocated, the completion
> + * path could find the data and free it.
> + */
> +
> + if (desc->flags == VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED) {
> + rqst_id = vmbus_next_request_id(&channel->requestor, requestid);
> + if (rqst_id == VMBUS_RQST_ERROR) {
> + pr_err("No request id available\n");
> + return -EAGAIN;
FYI, the lkp kernel test robot reported a missing call to
spin_unlock_irqrestore(&outring_info->ring_lock, flags) before the
above 'return': I'll address this in the next submission.
Andrea
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v8 2/3] scsi: storvsc: Use vmbus_requestor to generate transaction IDs for VMBus hardening
2020-11-04 15:40 [PATCH v8 0/3] Drivers: hv: vmbus: vmbus_requestor data structure for VMBus hardening Andrea Parri (Microsoft)
2020-11-04 15:40 ` [PATCH v8 1/3] Drivers: hv: vmbus: Add " Andrea Parri (Microsoft)
@ 2020-11-04 15:40 ` Andrea Parri (Microsoft)
2020-11-04 15:40 ` [PATCH v8 3/3] hv_netvsc: " Andrea Parri (Microsoft)
2 siblings, 0 replies; 11+ messages in thread
From: Andrea Parri (Microsoft) @ 2020-11-04 15:40 UTC (permalink / raw)
To: linux-kernel
Cc: K . Y . Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
linux-hyperv, Andres Beltran, Michael Kelley, Saruhan Karademir,
Juan Vazquez, Andrea Parri, James E.J. Bottomley,
Martin K. Petersen, linux-scsi
From: Andres Beltran <lkmlabelt@gmail.com>
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.
Signed-off-by: Andres Beltran <lkmlabelt@gmail.com>
Co-developed-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: linux-scsi@vger.kernel.org
---
drivers/scsi/storvsc_drv.c | 26 +++++++++++++++++++++++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 0c65fbd41035e..369a6c6266729 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,
@@ -1242,9 +1249,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;
+
+ 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)desc->trans_id);
+ request = (struct storvsc_cmd_request *)(unsigned long)cmd_rqst;
if (request == &stor_device->init_request ||
request == &stor_device->reset_request) {
@@ -1265,6 +1280,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,
@@ -1572,7 +1593,6 @@ static int storvsc_host_reset_handler(struct scsi_cmnd *scmnd)
struct vstor_packet *vstor_packet;
int ret, t;
-
stor_device = get_out_stor_device(device);
if (!stor_device)
return FAILED;
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v8 3/3] hv_netvsc: Use vmbus_requestor to generate transaction IDs for VMBus hardening
2020-11-04 15:40 [PATCH v8 0/3] Drivers: hv: vmbus: vmbus_requestor data structure for VMBus hardening Andrea Parri (Microsoft)
2020-11-04 15:40 ` [PATCH v8 1/3] Drivers: hv: vmbus: Add " Andrea Parri (Microsoft)
2020-11-04 15:40 ` [PATCH v8 2/3] scsi: storvsc: Use vmbus_requestor to generate transaction IDs " Andrea Parri (Microsoft)
@ 2020-11-04 15:40 ` Andrea Parri (Microsoft)
2020-11-04 21:43 ` Jakub Kicinski
2 siblings, 1 reply; 11+ messages in thread
From: Andrea Parri (Microsoft) @ 2020-11-04 15:40 UTC (permalink / raw)
To: linux-kernel
Cc: K . Y . Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
linux-hyperv, Andres Beltran, Michael Kelley, Saruhan Karademir,
Juan Vazquez, Andrea Parri, David S. Miller, Jakub Kicinski,
netdev
From: Andres Beltran <lkmlabelt@gmail.com>
Currently, pointers to guest memory are passed to Hyper-V as
transaction IDs in netvsc. In the face of errors or malicious
behavior in Hyper-V, netvsc 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.
Signed-off-by: Andres Beltran <lkmlabelt@gmail.com>
Co-developed-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org
---
drivers/net/hyperv/hyperv_net.h | 13 +++++++++++++
drivers/net/hyperv/netvsc.c | 22 ++++++++++++++++------
drivers/net/hyperv/rndis_filter.c | 1 +
include/linux/hyperv.h | 1 +
4 files changed, 31 insertions(+), 6 deletions(-)
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index a0f338cf14247..2a87cfa27ac02 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -847,6 +847,19 @@ struct nvsp_message {
#define NETVSC_XDP_HDRM 256
+#define NETVSC_MIN_OUT_MSG_SIZE (sizeof(struct vmpacket_descriptor) + \
+ sizeof(struct nvsp_message))
+#define NETVSC_MIN_IN_MSG_SIZE sizeof(struct vmpacket_descriptor)
+
+/* Estimated requestor size:
+ * out_ring_size/min_out_msg_size + in_ring_size/min_in_msg_size
+ */
+static inline u32 netvsc_rqstor_size(unsigned long ringbytes)
+{
+ return ringbytes / NETVSC_MIN_OUT_MSG_SIZE +
+ 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))
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 0c3de94b51787..f74ee8b90b84d 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -50,7 +50,7 @@ void netvsc_switch_datapath(struct net_device *ndev, bool vf)
vmbus_sendpacket(dev->channel, init_pkt,
sizeof(struct nvsp_message),
- (unsigned long)init_pkt,
+ VMBUS_RQST_ID_NO_RESPONSE,
VM_PKT_DATA_INBAND, 0);
}
@@ -163,7 +163,7 @@ static void netvsc_revoke_recv_buf(struct hv_device *device,
ret = vmbus_sendpacket(device->channel,
revoke_packet,
sizeof(struct nvsp_message),
- (unsigned long)revoke_packet,
+ VMBUS_RQST_ID_NO_RESPONSE,
VM_PKT_DATA_INBAND, 0);
/* If the failure is because the channel is rescinded;
* ignore the failure since we cannot send on a rescinded
@@ -213,7 +213,7 @@ static void netvsc_revoke_send_buf(struct hv_device *device,
ret = vmbus_sendpacket(device->channel,
revoke_packet,
sizeof(struct nvsp_message),
- (unsigned long)revoke_packet,
+ VMBUS_RQST_ID_NO_RESPONSE,
VM_PKT_DATA_INBAND, 0);
/* If the failure is because the channel is rescinded;
@@ -557,7 +557,7 @@ static int negotiate_nvsp_ver(struct hv_device *device,
ret = vmbus_sendpacket(device->channel, init_packet,
sizeof(struct nvsp_message),
- (unsigned long)init_packet,
+ VMBUS_RQST_ID_NO_RESPONSE,
VM_PKT_DATA_INBAND, 0);
return ret;
@@ -614,7 +614,7 @@ static int netvsc_connect_vsp(struct hv_device *device,
/* Send the init request */
ret = vmbus_sendpacket(device->channel, init_packet,
sizeof(struct nvsp_message),
- (unsigned long)init_packet,
+ VMBUS_RQST_ID_NO_RESPONSE,
VM_PKT_DATA_INBAND, 0);
if (ret != 0)
goto cleanup;
@@ -695,10 +695,19 @@ static void netvsc_send_tx_complete(struct net_device *ndev,
const struct vmpacket_descriptor *desc,
int budget)
{
- struct sk_buff *skb = (struct sk_buff *)(unsigned long)desc->trans_id;
+ struct sk_buff *skb;
struct net_device_context *ndev_ctx = netdev_priv(ndev);
u16 q_idx = 0;
int queue_sends;
+ u64 cmd_rqst;
+
+ cmd_rqst = vmbus_request_addr(&channel->requestor, (u64)desc->trans_id);
+ if (cmd_rqst == VMBUS_RQST_ERROR) {
+ netdev_err(ndev, "Incorrect transaction id\n");
+ return;
+ }
+
+ skb = (struct sk_buff *)(unsigned long)cmd_rqst;
/* Notify the layer above us */
if (likely(skb)) {
@@ -1520,6 +1529,7 @@ struct netvsc_device *netvsc_device_add(struct hv_device *device,
netvsc_poll, NAPI_POLL_WEIGHT);
/* Open the channel */
+ device->channel->rqstor_size = netvsc_rqstor_size(netvsc_ring_bytes);
ret = vmbus_open(device->channel, netvsc_ring_bytes,
netvsc_ring_bytes, NULL, 0,
netvsc_channel_cb, net_device->chan_table);
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index b22e47bcfeca1..6ae43319ece68 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -1172,6 +1172,7 @@ static void netvsc_sc_open(struct vmbus_channel *new_sc)
/* Set the channel before opening.*/
nvchan->channel = new_sc;
+ new_sc->rqstor_size = netvsc_rqstor_size(netvsc_ring_bytes);
ret = vmbus_open(new_sc, netvsc_ring_bytes,
netvsc_ring_bytes, NULL, 0,
netvsc_channel_cb, nvchan);
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 5b6d5c4e37110..5ddb479c4d4cb 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -779,6 +779,7 @@ struct vmbus_requestor {
#define VMBUS_NO_RQSTOR U64_MAX
#define VMBUS_RQST_ERROR (U64_MAX - 1)
+#define VMBUS_RQST_ID_NO_RESPONSE (U64_MAX - 2)
struct vmbus_device {
u16 dev_type;
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v8 3/3] hv_netvsc: Use vmbus_requestor to generate transaction IDs for VMBus hardening
2020-11-04 15:40 ` [PATCH v8 3/3] hv_netvsc: " Andrea Parri (Microsoft)
@ 2020-11-04 21:43 ` Jakub Kicinski
2020-11-04 21:45 ` Jakub Kicinski
0 siblings, 1 reply; 11+ messages in thread
From: Jakub Kicinski @ 2020-11-04 21:43 UTC (permalink / raw)
To: Andrea Parri (Microsoft)
Cc: linux-kernel, K . Y . Srinivasan, Haiyang Zhang,
Stephen Hemminger, Wei Liu, linux-hyperv, Andres Beltran,
Michael Kelley, Saruhan Karademir, Juan Vazquez, David S. Miller,
netdev
On Wed, 4 Nov 2020 16:40:27 +0100 Andrea Parri (Microsoft) wrote:
> From: Andres Beltran <lkmlabelt@gmail.com>
>
> Currently, pointers to guest memory are passed to Hyper-V as
> transaction IDs in netvsc. In the face of errors or malicious
> behavior in Hyper-V, netvsc 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.
>
> Signed-off-by: Andres Beltran <lkmlabelt@gmail.com>
> Co-developed-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> Reviewed-by: Michael Kelley <mikelley@microsoft.com>
I'm assuming this is targeting net-next? If so could you please tag it
as [PATCH net-next vN]?
> @@ -695,10 +695,19 @@ static void netvsc_send_tx_complete(struct net_device *ndev,
> const struct vmpacket_descriptor *desc,
> int budget)
> {
> - struct sk_buff *skb = (struct sk_buff *)(unsigned long)desc->trans_id;
> + struct sk_buff *skb;
> struct net_device_context *ndev_ctx = netdev_priv(ndev);
Swap these two lines please to keep the variables declaration lines
longest to shortest.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v8 3/3] hv_netvsc: Use vmbus_requestor to generate transaction IDs for VMBus hardening
2020-11-04 21:43 ` Jakub Kicinski
@ 2020-11-04 21:45 ` Jakub Kicinski
2020-11-05 3:32 ` Andrea Parri
0 siblings, 1 reply; 11+ messages in thread
From: Jakub Kicinski @ 2020-11-04 21:45 UTC (permalink / raw)
To: Andrea Parri (Microsoft)
Cc: linux-kernel, K . Y . Srinivasan, Haiyang Zhang,
Stephen Hemminger, Wei Liu, linux-hyperv, Andres Beltran,
Michael Kelley, Saruhan Karademir, Juan Vazquez, David S. Miller,
netdev
On Wed, 4 Nov 2020 13:43:48 -0800 Jakub Kicinski wrote:
> On Wed, 4 Nov 2020 16:40:27 +0100 Andrea Parri (Microsoft) wrote:
> > From: Andres Beltran <lkmlabelt@gmail.com>
> >
> > Currently, pointers to guest memory are passed to Hyper-V as
> > transaction IDs in netvsc. In the face of errors or malicious
> > behavior in Hyper-V, netvsc 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.
> >
> > Signed-off-by: Andres Beltran <lkmlabelt@gmail.com>
> > Co-developed-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> > Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> > Reviewed-by: Michael Kelley <mikelley@microsoft.com>
>
> I'm assuming this is targeting net-next? If so could you please tag it
> as [PATCH net-next vN]?
Ah, you don't 'cause you only sent us the third patch. In that case with
the nit below addressed:
Acked-by: Jakub Kicinski <kuba@kernel.org>
> > @@ -695,10 +695,19 @@ static void netvsc_send_tx_complete(struct net_device *ndev,
> > const struct vmpacket_descriptor *desc,
> > int budget)
> > {
> > - struct sk_buff *skb = (struct sk_buff *)(unsigned long)desc->trans_id;
> > + struct sk_buff *skb;
> > struct net_device_context *ndev_ctx = netdev_priv(ndev);
>
> Swap these two lines please to keep the variables declaration lines
> longest to shortest.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v8 3/3] hv_netvsc: Use vmbus_requestor to generate transaction IDs for VMBus hardening
2020-11-04 21:45 ` Jakub Kicinski
@ 2020-11-05 3:32 ` Andrea Parri
0 siblings, 0 replies; 11+ messages in thread
From: Andrea Parri @ 2020-11-05 3:32 UTC (permalink / raw)
To: Jakub Kicinski
Cc: linux-kernel, K . Y . Srinivasan, Haiyang Zhang,
Stephen Hemminger, Wei Liu, linux-hyperv, Andres Beltran,
Michael Kelley, Saruhan Karademir, Juan Vazquez, David S. Miller,
netdev
On Wed, Nov 04, 2020 at 01:45:05PM -0800, Jakub Kicinski wrote:
> On Wed, 4 Nov 2020 13:43:48 -0800 Jakub Kicinski wrote:
> > On Wed, 4 Nov 2020 16:40:27 +0100 Andrea Parri (Microsoft) wrote:
> > > From: Andres Beltran <lkmlabelt@gmail.com>
> > >
> > > Currently, pointers to guest memory are passed to Hyper-V as
> > > transaction IDs in netvsc. In the face of errors or malicious
> > > behavior in Hyper-V, netvsc 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.
> > >
> > > Signed-off-by: Andres Beltran <lkmlabelt@gmail.com>
> > > Co-developed-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> > > Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> > > Reviewed-by: Michael Kelley <mikelley@microsoft.com>
> >
> > I'm assuming this is targeting net-next? If so could you please tag it
> > as [PATCH net-next vN]?
>
> Ah, you don't 'cause you only sent us the third patch. In that case with
> the nit below addressed:
>
> Acked-by: Jakub Kicinski <kuba@kernel.org>
I fixed the declarations locally. Thank you for the review, Jakub.
(Yes, FWIW, I was imaging the series to go via hyperv-next...)
Thanks,
Andrea
>
> > > @@ -695,10 +695,19 @@ static void netvsc_send_tx_complete(struct net_device *ndev,
> > > const struct vmpacket_descriptor *desc,
> > > int budget)
> > > {
> > > - struct sk_buff *skb = (struct sk_buff *)(unsigned long)desc->trans_id;
> > > + struct sk_buff *skb;
> > > struct net_device_context *ndev_ctx = netdev_priv(ndev);
> >
> > Swap these two lines please to keep the variables declaration lines
> > longest to shortest.
>
^ permalink raw reply [flat|nested] 11+ messages in thread