From 03b30cc38dd3c3521dafb1cb2ac4ecd8470bbf0a Mon Sep 17 00:00:00 2001 From: Jiapeng Chong Date: Thu, 29 Apr 2021 18:21:21 +0800 Subject: hv_balloon: Remove redundant assignment to region_start Variable region_start is set to pg_start but this value is never read as it is overwritten later on, hence it is a redundant assignment and can be removed. Cleans up the following clang-analyzer warning: drivers/hv/hv_balloon.c:1013:3: warning: Value stored to 'region_start' is never read [clang-analyzer-deadcode.DeadStores]. Reported-by: Abaci Robot Signed-off-by: Jiapeng Chong Link: https://lore.kernel.org/r/1619691681-86256-1-git-send-email-jiapeng.chong@linux.alibaba.com Signed-off-by: Wei Liu --- drivers/hv/hv_balloon.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/hv') diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c index 58af84e30144..7f11ea07d698 100644 --- a/drivers/hv/hv_balloon.c +++ b/drivers/hv/hv_balloon.c @@ -1010,7 +1010,6 @@ static void hot_add_req(struct work_struct *dummy) * that need to be hot-added while ensuring the alignment * and size requirements of Linux as it relates to hot-add. */ - region_start = pg_start; region_size = (pfn_cnt / HA_CHUNK) * HA_CHUNK; if (pfn_cnt % HA_CHUNK) region_size += HA_CHUNK; -- cgit v1.2.3 From adae1e931acd8b430d31141a283ea06d4b705417 Mon Sep 17 00:00:00 2001 From: Andres Beltran Date: Thu, 8 Apr 2021 18:14:39 +0200 Subject: Drivers: hv: vmbus: Copy packets sent by Hyper-V out of the ring buffer Pointers to ring-buffer packets sent by Hyper-V are used within the guest VM. Hyper-V can send packets with erroneous values or modify packet fields after they are processed by the guest. To defend against these scenarios, return a copy of the incoming VMBus packet after validating its length and offset fields in hv_pkt_iter_first(). In this way, the packet can no longer be modified by the host. Signed-off-by: Andres Beltran Co-developed-by: Andrea Parri (Microsoft) Signed-off-by: Andrea Parri (Microsoft) Reviewed-by: Michael Kelley Link: https://lore.kernel.org/r/20210408161439.341988-1-parri.andrea@gmail.com Signed-off-by: Wei Liu --- drivers/hv/channel.c | 9 +++-- drivers/hv/hv_fcopy.c | 1 + drivers/hv/hv_kvp.c | 1 + drivers/hv/hyperv_vmbus.h | 2 +- drivers/hv/ring_buffer.c | 82 +++++++++++++++++++++++++++++++++------ drivers/net/hyperv/hyperv_net.h | 7 ++++ drivers/net/hyperv/netvsc.c | 2 + drivers/net/hyperv/rndis_filter.c | 2 + drivers/scsi/storvsc_drv.c | 10 +++++ include/linux/hyperv.h | 48 +++++++++++++++++++---- net/vmw_vsock/hyperv_transport.c | 4 +- 11 files changed, 143 insertions(+), 25 deletions(-) (limited to 'drivers/hv') diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index c2635e913a92..bfbca4eeb773 100644 --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c @@ -662,12 +662,15 @@ static int __vmbus_open(struct vmbus_channel *newchannel, newchannel->onchannel_callback = onchannelcallback; newchannel->channel_callback_context = context; - err = hv_ringbuffer_init(&newchannel->outbound, page, send_pages); + if (!newchannel->max_pkt_size) + newchannel->max_pkt_size = VMBUS_DEFAULT_MAX_PKT_SIZE; + + err = hv_ringbuffer_init(&newchannel->outbound, page, send_pages, 0); if (err) goto error_clean_ring; - err = hv_ringbuffer_init(&newchannel->inbound, - &page[send_pages], recv_pages); + err = hv_ringbuffer_init(&newchannel->inbound, &page[send_pages], + recv_pages, newchannel->max_pkt_size); if (err) goto error_clean_ring; diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c index 59ce85e00a02..660036da7449 100644 --- a/drivers/hv/hv_fcopy.c +++ b/drivers/hv/hv_fcopy.c @@ -349,6 +349,7 @@ int hv_fcopy_init(struct hv_util_service *srv) { recv_buffer = srv->recv_buffer; fcopy_transaction.recv_channel = srv->channel; + fcopy_transaction.recv_channel->max_pkt_size = HV_HYP_PAGE_SIZE * 2; /* * When this driver loads, the user level daemon that diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c index b49962d312ce..c698592b83e4 100644 --- a/drivers/hv/hv_kvp.c +++ b/drivers/hv/hv_kvp.c @@ -757,6 +757,7 @@ hv_kvp_init(struct hv_util_service *srv) { recv_buffer = srv->recv_buffer; kvp_transaction.recv_channel = srv->channel; + kvp_transaction.recv_channel->max_pkt_size = HV_HYP_PAGE_SIZE * 4; /* * When this driver loads, the user level daemon that diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h index 9416e09ebd58..42f3d9d123a1 100644 --- a/drivers/hv/hyperv_vmbus.h +++ b/drivers/hv/hyperv_vmbus.h @@ -174,7 +174,7 @@ extern int hv_synic_cleanup(unsigned int cpu); void hv_ringbuffer_pre_init(struct vmbus_channel *channel); int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info, - struct page *pages, u32 pagecnt); + struct page *pages, u32 pagecnt, u32 max_pkt_size); void hv_ringbuffer_cleanup(struct hv_ring_buffer_info *ring_info); diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c index 374f8afbf8a5..e621f8d9b436 100644 --- a/drivers/hv/ring_buffer.c +++ b/drivers/hv/ring_buffer.c @@ -181,7 +181,7 @@ void hv_ringbuffer_pre_init(struct vmbus_channel *channel) /* Initialize the ring buffer. */ int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info, - struct page *pages, u32 page_cnt) + struct page *pages, u32 page_cnt, u32 max_pkt_size) { int i; struct page **pages_wraparound; @@ -223,6 +223,14 @@ int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info, sizeof(struct hv_ring_buffer); ring_info->priv_read_index = 0; + /* Initialize buffer that holds copies of incoming packets */ + if (max_pkt_size) { + ring_info->pkt_buffer = kzalloc(max_pkt_size, GFP_KERNEL); + if (!ring_info->pkt_buffer) + return -ENOMEM; + ring_info->pkt_buffer_size = max_pkt_size; + } + spin_lock_init(&ring_info->ring_lock); return 0; @@ -235,6 +243,9 @@ void hv_ringbuffer_cleanup(struct hv_ring_buffer_info *ring_info) vunmap(ring_info->ring_buffer); ring_info->ring_buffer = NULL; mutex_unlock(&ring_info->ring_buffer_mutex); + + kfree(ring_info->pkt_buffer); + ring_info->pkt_buffer_size = 0; } /* Write to the ring buffer. */ @@ -375,7 +386,7 @@ int hv_ringbuffer_read(struct vmbus_channel *channel, memcpy(buffer, (const char *)desc + offset, packetlen); /* Advance ring index to next packet descriptor */ - __hv_pkt_iter_next(channel, desc); + __hv_pkt_iter_next(channel, desc, true); /* Notify host of update */ hv_pkt_iter_close(channel); @@ -401,6 +412,22 @@ static u32 hv_pkt_iter_avail(const struct hv_ring_buffer_info *rbi) return (rbi->ring_datasize - priv_read_loc) + write_loc; } +/* + * Get first vmbus packet without copying it out of the ring buffer + */ +struct vmpacket_descriptor *hv_pkt_iter_first_raw(struct vmbus_channel *channel) +{ + struct hv_ring_buffer_info *rbi = &channel->inbound; + + hv_debug_delay_test(channel, MESSAGE_DELAY); + + if (hv_pkt_iter_avail(rbi) < sizeof(struct vmpacket_descriptor)) + return NULL; + + return (struct vmpacket_descriptor *)(hv_get_ring_buffer(rbi) + rbi->priv_read_index); +} +EXPORT_SYMBOL_GPL(hv_pkt_iter_first_raw); + /* * Get first vmbus packet from ring buffer after read_index * @@ -409,17 +436,49 @@ static u32 hv_pkt_iter_avail(const struct hv_ring_buffer_info *rbi) struct vmpacket_descriptor *hv_pkt_iter_first(struct vmbus_channel *channel) { struct hv_ring_buffer_info *rbi = &channel->inbound; - struct vmpacket_descriptor *desc; + struct vmpacket_descriptor *desc, *desc_copy; + u32 bytes_avail, pkt_len, pkt_offset; - hv_debug_delay_test(channel, MESSAGE_DELAY); - if (hv_pkt_iter_avail(rbi) < sizeof(struct vmpacket_descriptor)) + desc = hv_pkt_iter_first_raw(channel); + if (!desc) return NULL; - desc = hv_get_ring_buffer(rbi) + rbi->priv_read_index; - if (desc) - prefetch((char *)desc + (desc->len8 << 3)); + bytes_avail = min(rbi->pkt_buffer_size, hv_pkt_iter_avail(rbi)); + + /* + * Ensure the compiler does not use references to incoming Hyper-V values (which + * could change at any moment) when reading local variables later in the code + */ + pkt_len = READ_ONCE(desc->len8) << 3; + pkt_offset = READ_ONCE(desc->offset8) << 3; + + /* + * If pkt_len is invalid, set it to the smaller of hv_pkt_iter_avail() and + * rbi->pkt_buffer_size + */ + if (pkt_len < sizeof(struct vmpacket_descriptor) || pkt_len > bytes_avail) + pkt_len = bytes_avail; + + /* + * If pkt_offset is invalid, arbitrarily set it to + * the size of vmpacket_descriptor + */ + if (pkt_offset < sizeof(struct vmpacket_descriptor) || pkt_offset > pkt_len) + pkt_offset = sizeof(struct vmpacket_descriptor); + + /* Copy the Hyper-V packet out of the ring buffer */ + desc_copy = (struct vmpacket_descriptor *)rbi->pkt_buffer; + memcpy(desc_copy, desc, pkt_len); + + /* + * Hyper-V could still change len8 and offset8 after the earlier read. + * Ensure that desc_copy has legal values for len8 and offset8 that + * are consistent with the copy we just made + */ + desc_copy->len8 = pkt_len >> 3; + desc_copy->offset8 = pkt_offset >> 3; - return desc; + return desc_copy; } EXPORT_SYMBOL_GPL(hv_pkt_iter_first); @@ -431,7 +490,8 @@ EXPORT_SYMBOL_GPL(hv_pkt_iter_first); */ struct vmpacket_descriptor * __hv_pkt_iter_next(struct vmbus_channel *channel, - const struct vmpacket_descriptor *desc) + const struct vmpacket_descriptor *desc, + bool copy) { struct hv_ring_buffer_info *rbi = &channel->inbound; u32 packetlen = desc->len8 << 3; @@ -444,7 +504,7 @@ __hv_pkt_iter_next(struct vmbus_channel *channel, rbi->priv_read_index -= dsize; /* more data? */ - return hv_pkt_iter_first(channel); + return copy ? hv_pkt_iter_first(channel) : hv_pkt_iter_first_raw(channel); } EXPORT_SYMBOL_GPL(__hv_pkt_iter_next); diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h index 442c520ab8f3..b11aa68b44ec 100644 --- a/drivers/net/hyperv/hyperv_net.h +++ b/drivers/net/hyperv/hyperv_net.h @@ -895,9 +895,16 @@ static inline u32 netvsc_rqstor_size(unsigned long ringbytes) ringbytes / NETVSC_MIN_IN_MSG_SIZE; } +/* XFER PAGE packets can specify a maximum of 375 ranges for NDIS >= 6.0 + * and a maximum of 64 ranges for NDIS < 6.0 with no RSC; with RSC, this + * limit is raised to 562 (= NVSP_RSC_MAX). + */ +#define NETVSC_MAX_XFER_PAGE_RANGES NVSP_RSC_MAX #define NETVSC_XFER_HEADER_SIZE(rng_cnt) \ (offsetof(struct vmtransfer_page_packet_header, ranges) + \ (rng_cnt) * sizeof(struct vmtransfer_page_range)) +#define NETVSC_MAX_PKT_SIZE (NETVSC_XFER_HEADER_SIZE(NETVSC_MAX_XFER_PAGE_RANGES) + \ + sizeof(struct nvsp_message) + (sizeof(u32) * VRSS_SEND_TAB_SIZE)) struct multi_send_data { struct sk_buff *skb; /* skb containing the pkt */ diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c index 9d07c9ce4be2..067077138e52 100644 --- a/drivers/net/hyperv/netvsc.c +++ b/drivers/net/hyperv/netvsc.c @@ -1650,6 +1650,8 @@ struct netvsc_device *netvsc_device_add(struct hv_device *device, /* Open the channel */ device->channel->rqstor_size = netvsc_rqstor_size(netvsc_ring_bytes); + device->channel->max_pkt_size = NETVSC_MAX_PKT_SIZE; + 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 c0e89e107d57..d7ff9ddcbae2 100644 --- a/drivers/net/hyperv/rndis_filter.c +++ b/drivers/net/hyperv/rndis_filter.c @@ -1260,6 +1260,8 @@ static void netvsc_sc_open(struct vmbus_channel *new_sc) nvchan->channel = new_sc; new_sc->rqstor_size = netvsc_rqstor_size(netvsc_ring_bytes); + new_sc->max_pkt_size = NETVSC_MAX_PKT_SIZE; + ret = vmbus_open(new_sc, netvsc_ring_bytes, netvsc_ring_bytes, NULL, 0, netvsc_channel_cb, nvchan); diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index e6718a74e5da..07149fa72b68 100644 --- a/drivers/scsi/storvsc_drv.c +++ b/drivers/scsi/storvsc_drv.c @@ -406,6 +406,14 @@ static void storvsc_on_channel_callback(void *context); #define STORVSC_IDE_MAX_TARGETS 1 #define STORVSC_IDE_MAX_CHANNELS 1 +/* + * Upper bound on the size of a storvsc packet. vmscsi_size_delta is not + * included in the calculation because it is set after STORVSC_MAX_PKT_SIZE + * is used in storvsc_connect_to_vsp + */ +#define STORVSC_MAX_PKT_SIZE (sizeof(struct vmpacket_descriptor) +\ + sizeof(struct vstor_packet)) + struct storvsc_cmd_request { struct scsi_cmnd *cmd; @@ -701,6 +709,7 @@ static void handle_sc_creation(struct vmbus_channel *new_sc) return; memset(&props, 0, sizeof(struct vmstorage_channel_properties)); + new_sc->max_pkt_size = STORVSC_MAX_PKT_SIZE; /* * The size of vmbus_requestor is an upper bound on the number of requests @@ -1294,6 +1303,7 @@ static int storvsc_connect_to_vsp(struct hv_device *device, u32 ring_size, memset(&props, 0, sizeof(struct vmstorage_channel_properties)); + device->channel->max_pkt_size = STORVSC_MAX_PKT_SIZE; /* * 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. diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index d1e59dbef1dd..3932446f215f 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -181,6 +181,10 @@ struct hv_ring_buffer_info { * being freed while the ring buffer is being accessed. */ struct mutex ring_buffer_mutex; + + /* Buffer that holds a copy of an incoming host packet */ + void *pkt_buffer; + u32 pkt_buffer_size; }; @@ -799,6 +803,8 @@ struct vmbus_device { bool allowed_in_isolated; }; +#define VMBUS_DEFAULT_MAX_PKT_SIZE 4096 + struct vmbus_channel { struct list_head listentry; @@ -1021,6 +1027,9 @@ struct vmbus_channel { /* request/transaction ids for VMBus */ struct vmbus_requestor requestor; u32 rqstor_size; + + /* The max size of a packet on this channel */ + u32 max_pkt_size; }; u64 vmbus_next_request_id(struct vmbus_requestor *rqstor, u64 rqst_addr); @@ -1662,32 +1671,55 @@ static inline u32 hv_pkt_datalen(const struct vmpacket_descriptor *desc) } +struct vmpacket_descriptor * +hv_pkt_iter_first_raw(struct vmbus_channel *channel); + struct vmpacket_descriptor * hv_pkt_iter_first(struct vmbus_channel *channel); struct vmpacket_descriptor * __hv_pkt_iter_next(struct vmbus_channel *channel, - const struct vmpacket_descriptor *pkt); + const struct vmpacket_descriptor *pkt, + bool copy); void hv_pkt_iter_close(struct vmbus_channel *channel); -/* - * Get next packet descriptor from iterator - * If at end of list, return NULL and update host. - */ static inline struct vmpacket_descriptor * -hv_pkt_iter_next(struct vmbus_channel *channel, - const struct vmpacket_descriptor *pkt) +hv_pkt_iter_next_pkt(struct vmbus_channel *channel, + const struct vmpacket_descriptor *pkt, + bool copy) { struct vmpacket_descriptor *nxt; - nxt = __hv_pkt_iter_next(channel, pkt); + nxt = __hv_pkt_iter_next(channel, pkt, copy); if (!nxt) hv_pkt_iter_close(channel); return nxt; } +/* + * Get next packet descriptor without copying it out of the ring buffer + * If at end of list, return NULL and update host. + */ +static inline struct vmpacket_descriptor * +hv_pkt_iter_next_raw(struct vmbus_channel *channel, + const struct vmpacket_descriptor *pkt) +{ + return hv_pkt_iter_next_pkt(channel, pkt, false); +} + +/* + * Get next packet descriptor from iterator + * If at end of list, return NULL and update host. + */ +static inline struct vmpacket_descriptor * +hv_pkt_iter_next(struct vmbus_channel *channel, + const struct vmpacket_descriptor *pkt) +{ + return hv_pkt_iter_next_pkt(channel, pkt, true); +} + #define foreach_vmbus_pkt(pkt, channel) \ for (pkt = hv_pkt_iter_first(channel); pkt; \ pkt = hv_pkt_iter_next(channel, pkt)) diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c index cc3bae2659e7..19189cf30a72 100644 --- a/net/vmw_vsock/hyperv_transport.c +++ b/net/vmw_vsock/hyperv_transport.c @@ -596,7 +596,7 @@ static ssize_t hvs_stream_dequeue(struct vsock_sock *vsk, struct msghdr *msg, return -EOPNOTSUPP; if (need_refill) { - hvs->recv_desc = hv_pkt_iter_first(hvs->chan); + hvs->recv_desc = hv_pkt_iter_first_raw(hvs->chan); ret = hvs_update_recv_data(hvs); if (ret) return ret; @@ -610,7 +610,7 @@ static ssize_t hvs_stream_dequeue(struct vsock_sock *vsk, struct msghdr *msg, hvs->recv_data_len -= to_read; if (hvs->recv_data_len == 0) { - hvs->recv_desc = hv_pkt_iter_next(hvs->chan, hvs->recv_desc); + hvs->recv_desc = hv_pkt_iter_next_raw(hvs->chan, hvs->recv_desc); if (hvs->recv_desc) { ret = hvs_update_recv_data(hvs); if (ret) -- cgit v1.2.3 From bf5fd8cae3c8f0d1e6f71a076e0ce2bd17645d0b Mon Sep 17 00:00:00 2001 From: "Andrea Parri (Microsoft)" Date: Mon, 10 May 2021 23:08:41 +0200 Subject: scsi: storvsc: Use blk_mq_unique_tag() to generate requestIDs Use blk_mq_unique_tag() to generate requestIDs for StorVSC, avoiding all issues with allocating enough entries in the VMbus requestor. Suggested-by: Michael Kelley Signed-off-by: Andrea Parri (Microsoft) Reviewed-by: Michael Kelley Acked-by: Martin K. Petersen Link: https://lore.kernel.org/r/20210510210841.370472-1-parri.andrea@gmail.com Signed-off-by: Wei Liu --- drivers/hv/channel.c | 14 +++--- drivers/hv/ring_buffer.c | 13 +++--- drivers/net/hyperv/netvsc.c | 8 ++-- drivers/net/hyperv/rndis_filter.c | 2 + drivers/scsi/storvsc_drv.c | 94 ++++++++++++++++++++++++++------------- include/linux/hyperv.h | 13 +++++- 6 files changed, 95 insertions(+), 49 deletions(-) (limited to 'drivers/hv') diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index bfbca4eeb773..f3761c73b074 100644 --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c @@ -1189,15 +1189,14 @@ 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 + * @channel: Pointer to the VMbus channel struct * @rqst_add: Guest memory address to be stored in the array */ -u64 vmbus_next_request_id(struct vmbus_requestor *rqstor, u64 rqst_addr) +u64 vmbus_next_request_id(struct vmbus_channel *channel, u64 rqst_addr) { + struct vmbus_requestor *rqstor = &channel->requestor; 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) @@ -1231,16 +1230,15 @@ 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 + * @channel: Pointer to the VMbus channel 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) +u64 vmbus_request_addr(struct vmbus_channel *channel, u64 trans_id) { + struct vmbus_requestor *rqstor = &channel->requestor; 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) diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c index e621f8d9b436..2aee356840a2 100644 --- a/drivers/hv/ring_buffer.c +++ b/drivers/hv/ring_buffer.c @@ -312,10 +312,12 @@ int hv_ringbuffer_write(struct vmbus_channel *channel, */ if (desc->flags == VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED) { - rqst_id = vmbus_next_request_id(&channel->requestor, requestid); - if (rqst_id == VMBUS_RQST_ERROR) { - spin_unlock_irqrestore(&outring_info->ring_lock, flags); - return -EAGAIN; + if (channel->next_request_id_callback != NULL) { + rqst_id = channel->next_request_id_callback(channel, requestid); + if (rqst_id == VMBUS_RQST_ERROR) { + spin_unlock_irqrestore(&outring_info->ring_lock, flags); + return -EAGAIN; + } } } desc = hv_get_ring_buffer(outring_info) + old_write; @@ -343,7 +345,8 @@ int hv_ringbuffer_write(struct vmbus_channel *channel, if (channel->rescind) { if (rqst_id != VMBUS_NO_RQSTOR) { /* Reclaim request ID to avoid leak of IDs */ - vmbus_request_addr(&channel->requestor, rqst_id); + if (channel->request_addr_callback != NULL) + channel->request_addr_callback(channel, rqst_id); } return -ENODEV; } diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c index 067077138e52..7bd935412853 100644 --- a/drivers/net/hyperv/netvsc.c +++ b/drivers/net/hyperv/netvsc.c @@ -757,7 +757,7 @@ static void netvsc_send_tx_complete(struct net_device *ndev, int queue_sends; u64 cmd_rqst; - cmd_rqst = vmbus_request_addr(&channel->requestor, (u64)desc->trans_id); + cmd_rqst = channel->request_addr_callback(channel, (u64)desc->trans_id); if (cmd_rqst == VMBUS_RQST_ERROR) { netdev_err(ndev, "Incorrect transaction id\n"); return; @@ -817,8 +817,8 @@ static void netvsc_send_completion(struct net_device *ndev, /* First check if this is a VMBUS completion without data payload */ if (!msglen) { - cmd_rqst = vmbus_request_addr(&incoming_channel->requestor, - (u64)desc->trans_id); + cmd_rqst = incoming_channel->request_addr_callback(incoming_channel, + (u64)desc->trans_id); if (cmd_rqst == VMBUS_RQST_ERROR) { netdev_err(ndev, "Invalid transaction id\n"); return; @@ -1649,6 +1649,8 @@ struct netvsc_device *netvsc_device_add(struct hv_device *device, netvsc_poll, NAPI_POLL_WEIGHT); /* Open the channel */ + device->channel->next_request_id_callback = vmbus_next_request_id; + device->channel->request_addr_callback = vmbus_request_addr; device->channel->rqstor_size = netvsc_rqstor_size(netvsc_ring_bytes); device->channel->max_pkt_size = NETVSC_MAX_PKT_SIZE; diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c index d7ff9ddcbae2..983bf362466a 100644 --- a/drivers/net/hyperv/rndis_filter.c +++ b/drivers/net/hyperv/rndis_filter.c @@ -1259,6 +1259,8 @@ static void netvsc_sc_open(struct vmbus_channel *new_sc) /* Set the channel before opening.*/ nvchan->channel = new_sc; + new_sc->next_request_id_callback = vmbus_next_request_id; + new_sc->request_addr_callback = vmbus_request_addr; new_sc->rqstor_size = netvsc_rqstor_size(netvsc_ring_bytes); new_sc->max_pkt_size = NETVSC_MAX_PKT_SIZE; diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index 07149fa72b68..403753929320 100644 --- a/drivers/scsi/storvsc_drv.c +++ b/drivers/scsi/storvsc_drv.c @@ -696,6 +696,23 @@ old_is_alloced: spin_unlock_irqrestore(&stor_device->lock, flags); } +static u64 storvsc_next_request_id(struct vmbus_channel *channel, u64 rqst_addr) +{ + struct storvsc_cmd_request *request = + (struct storvsc_cmd_request *)(unsigned long)rqst_addr; + + if (rqst_addr == VMBUS_RQST_INIT) + return VMBUS_RQST_INIT; + if (rqst_addr == VMBUS_RQST_RESET) + return VMBUS_RQST_RESET; + + /* + * Cannot return an ID of 0, which is reserved for an unsolicited + * message from Hyper-V. + */ + return (u64)blk_mq_unique_tag(request->cmd->request) + 1; +} + static void handle_sc_creation(struct vmbus_channel *new_sc) { struct hv_device *device = new_sc->primary_channel->device_obj; @@ -711,11 +728,7 @@ static void handle_sc_creation(struct vmbus_channel *new_sc) memset(&props, 0, sizeof(struct vmstorage_channel_properties)); new_sc->max_pkt_size = STORVSC_MAX_PKT_SIZE; - /* - * 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; + new_sc->next_request_id_callback = storvsc_next_request_id; ret = vmbus_open(new_sc, storvsc_ringbuffer_size, @@ -782,7 +795,7 @@ static void handle_multichannel_storage(struct hv_device *device, int max_chns) ret = vmbus_sendpacket(device->channel, vstor_packet, (sizeof(struct vstor_packet) - stor_device->vmscsi_size_delta), - (unsigned long)request, + VMBUS_RQST_INIT, VM_PKT_DATA_INBAND, VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED); @@ -851,7 +864,7 @@ static int storvsc_execute_vstor_op(struct hv_device *device, ret = vmbus_sendpacket(device->channel, vstor_packet, (sizeof(struct vstor_packet) - stor_device->vmscsi_size_delta), - (unsigned long)request, + VMBUS_RQST_INIT, VM_PKT_DATA_INBAND, VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED); if (ret != 0) @@ -1253,6 +1266,7 @@ static void storvsc_on_channel_callback(void *context) const struct vmpacket_descriptor *desc; struct hv_device *device; struct storvsc_device *stor_device; + struct Scsi_Host *shost; if (channel->primary_channel != NULL) device = channel->primary_channel->device_obj; @@ -1263,20 +1277,12 @@ static void storvsc_on_channel_callback(void *context) if (!stor_device) return; - 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; - } + shost = stor_device->host; - request = (struct storvsc_cmd_request *)(unsigned long)cmd_rqst; + foreach_vmbus_pkt(desc, channel) { + struct vstor_packet *packet = hv_pkt_data(desc); + struct storvsc_cmd_request *request = NULL; + u64 rqst_id = desc->trans_id; if (hv_pkt_datalen(desc) < sizeof(struct vstor_packet) - stor_device->vmscsi_size_delta) { @@ -1284,14 +1290,44 @@ static void storvsc_on_channel_callback(void *context) continue; } - if (request == &stor_device->init_request || - request == &stor_device->reset_request) { - memcpy(&request->vstor_packet, packet, - (sizeof(struct vstor_packet) - stor_device->vmscsi_size_delta)); - complete(&request->wait_event); + if (rqst_id == VMBUS_RQST_INIT) { + request = &stor_device->init_request; + } else if (rqst_id == VMBUS_RQST_RESET) { + request = &stor_device->reset_request; } else { + /* Hyper-V can send an unsolicited message with ID of 0 */ + if (rqst_id == 0) { + /* + * storvsc_on_receive() looks at the vstor_packet in the message + * from the ring buffer. If the operation in the vstor_packet is + * COMPLETE_IO, then we call storvsc_on_io_completion(), and + * dereference the guest memory address. Make sure we don't call + * storvsc_on_io_completion() with a guest memory address that is + * zero if Hyper-V were to construct and send such a bogus packet. + */ + if (packet->operation == VSTOR_OPERATION_COMPLETE_IO) { + dev_err(&device->device, "Invalid packet with ID of 0\n"); + continue; + } + } else { + struct scsi_cmnd *scmnd; + + /* Transaction 'rqst_id' corresponds to tag 'rqst_id - 1' */ + scmnd = scsi_host_find_tag(shost, rqst_id - 1); + if (scmnd == NULL) { + dev_err(&device->device, "Incorrect transaction ID\n"); + continue; + } + request = (struct storvsc_cmd_request *)scsi_cmd_priv(scmnd); + } + storvsc_on_receive(stor_device, packet, request); + continue; } + + memcpy(&request->vstor_packet, packet, + (sizeof(struct vstor_packet) - stor_device->vmscsi_size_delta)); + complete(&request->wait_event); } } @@ -1304,11 +1340,7 @@ static int storvsc_connect_to_vsp(struct hv_device *device, u32 ring_size, memset(&props, 0, sizeof(struct vmstorage_channel_properties)); device->channel->max_pkt_size = STORVSC_MAX_PKT_SIZE; - /* - * 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; + device->channel->next_request_id_callback = storvsc_next_request_id; ret = vmbus_open(device->channel, ring_size, @@ -1634,7 +1666,7 @@ static int storvsc_host_reset_handler(struct scsi_cmnd *scmnd) ret = vmbus_sendpacket(device->channel, vstor_packet, (sizeof(struct vstor_packet) - stor_device->vmscsi_size_delta), - (unsigned long)&stor_device->reset_request, + VMBUS_RQST_RESET, VM_PKT_DATA_INBAND, VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED); if (ret != 0) diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index 3932446f215f..2e859d2f9609 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -794,7 +794,11 @@ struct vmbus_requestor { #define VMBUS_NO_RQSTOR U64_MAX #define VMBUS_RQST_ERROR (U64_MAX - 1) +/* NetVSC-specific */ #define VMBUS_RQST_ID_NO_RESPONSE (U64_MAX - 2) +/* StorVSC-specific */ +#define VMBUS_RQST_INIT (U64_MAX - 2) +#define VMBUS_RQST_RESET (U64_MAX - 3) struct vmbus_device { u16 dev_type; @@ -1024,6 +1028,11 @@ struct vmbus_channel { u32 fuzz_testing_interrupt_delay; u32 fuzz_testing_message_delay; + /* callback to generate a request ID from a request address */ + u64 (*next_request_id_callback)(struct vmbus_channel *channel, u64 rqst_addr); + /* callback to retrieve a request address from a request ID */ + u64 (*request_addr_callback)(struct vmbus_channel *channel, u64 rqst_id); + /* request/transaction ids for VMBus */ struct vmbus_requestor requestor; u32 rqstor_size; @@ -1032,8 +1041,8 @@ struct vmbus_channel { u32 max_pkt_size; }; -u64 vmbus_next_request_id(struct vmbus_requestor *rqstor, u64 rqst_addr); -u64 vmbus_request_addr(struct vmbus_requestor *rqstor, u64 trans_id); +u64 vmbus_next_request_id(struct vmbus_channel *channel, u64 rqst_addr); +u64 vmbus_request_addr(struct vmbus_channel *channel, u64 trans_id); static inline bool is_hvsock_channel(const struct vmbus_channel *c) { -- cgit v1.2.3 From c6a8625fa4c6b0a97860d053271660ccedc3d1b3 Mon Sep 17 00:00:00 2001 From: YueHaibing Date: Fri, 14 May 2021 15:01:16 +0800 Subject: hv_utils: Fix passing zero to 'PTR_ERR' warning Sparse warn this: drivers/hv/hv_util.c:753 hv_timesync_init() warn: passing zero to 'PTR_ERR' Use PTR_ERR_OR_ZERO instead of PTR_ERR to fix this. Signed-off-by: YueHaibing Link: https://lore.kernel.org/r/20210514070116.16800-1-yuehaibing@huawei.com [ wei: change %ld to %d ] Signed-off-by: Wei Liu --- drivers/hv/hv_util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/hv') diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c index e4aefeb330da..136576cba26f 100644 --- a/drivers/hv/hv_util.c +++ b/drivers/hv/hv_util.c @@ -750,8 +750,8 @@ static int hv_timesync_init(struct hv_util_service *srv) */ hv_ptp_clock = ptp_clock_register(&ptp_hyperv_info, NULL); if (IS_ERR_OR_NULL(hv_ptp_clock)) { - pr_err("cannot register PTP clock: %ld\n", - PTR_ERR(hv_ptp_clock)); + pr_err("cannot register PTP clock: %d\n", + PTR_ERR_OR_ZERO(hv_ptp_clock)); hv_ptp_clock = NULL; } -- cgit v1.2.3 From 9de6655cc5a6a1febc514465c87c24a0e96d8dba Mon Sep 17 00:00:00 2001 From: Jiapeng Chong Date: Tue, 25 May 2021 18:58:41 +0800 Subject: drivers: hv: Fix missing error code in vmbus_connect() Eliminate the follow smatch warning: drivers/hv/connection.c:236 vmbus_connect() warn: missing error code 'ret'. Reported-by: Abaci Robot Signed-off-by: Jiapeng Chong Reviewed-by: Michael Kelley Link: https://lore.kernel.org/r/1621940321-72353-1-git-send-email-jiapeng.chong@linux.alibaba.com Signed-off-by: Wei Liu --- drivers/hv/connection.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/hv') diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c index 311cd005b3be..5e479d54918c 100644 --- a/drivers/hv/connection.c +++ b/drivers/hv/connection.c @@ -232,8 +232,10 @@ int vmbus_connect(void) */ for (i = 0; ; i++) { - if (i == ARRAY_SIZE(vmbus_versions)) + if (i == ARRAY_SIZE(vmbus_versions)) { + ret = -EDOM; goto cleanup; + } version = vmbus_versions[i]; if (version > max_version) -- cgit v1.2.3 From a4d7e8ae4a541557d7a2c815835b786c18c3613c Mon Sep 17 00:00:00 2001 From: Michael Kelley Date: Wed, 2 Jun 2021 14:36:44 -0700 Subject: Drivers: hv: Move Hyper-V extended capability check to arch neutral code The extended capability query code is currently under arch/x86, but it is architecture neutral, and is used by arch neutral code in the Hyper-V balloon driver. Hence the balloon driver fails to build on other architectures. Fix by moving the ext cap code out from arch/x86. Because it is also called from built-in architecture specific code, it can't be in a module, so the Makefile treats as built-in even when CONFIG_HYPERV is "m". Also drivers/Makefile is tweaked because this is the first occurrence of a Hyper-V file that is built-in even when CONFIG_HYPERV is "m". While here, update the hypercall status check to use the new helper function instead of open coding. No functional change. Signed-off-by: Michael Kelley Reviewed-by: Sunil Muthuswamy Link: https://lore.kernel.org/r/1622669804-2016-1-git-send-email-mikelley@microsoft.com Signed-off-by: Wei Liu --- arch/x86/hyperv/hv_init.c | 47 --------------------------------- drivers/Makefile | 2 +- drivers/hv/Makefile | 3 +++ drivers/hv/hv_common.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 70 insertions(+), 48 deletions(-) create mode 100644 drivers/hv/hv_common.c (limited to 'drivers/hv') diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c index bb0ae4b5c00f..6952e219cba3 100644 --- a/arch/x86/hyperv/hv_init.c +++ b/arch/x86/hyperv/hv_init.c @@ -614,50 +614,3 @@ bool hv_is_isolation_supported(void) return hv_get_isolation_type() != HV_ISOLATION_TYPE_NONE; } EXPORT_SYMBOL_GPL(hv_is_isolation_supported); - -/* Bit mask of the extended capability to query: see HV_EXT_CAPABILITY_xxx */ -bool hv_query_ext_cap(u64 cap_query) -{ - /* - * The address of the 'hv_extended_cap' variable will be used as an - * output parameter to the hypercall below and so it should be - * compatible with 'virt_to_phys'. Which means, it's address should be - * directly mapped. Use 'static' to keep it compatible; stack variables - * can be virtually mapped, making them imcompatible with - * 'virt_to_phys'. - * Hypercall input/output addresses should also be 8-byte aligned. - */ - static u64 hv_extended_cap __aligned(8); - static bool hv_extended_cap_queried; - u64 status; - - /* - * Querying extended capabilities is an extended hypercall. Check if the - * partition supports extended hypercall, first. - */ - if (!(ms_hyperv.priv_high & HV_ENABLE_EXTENDED_HYPERCALLS)) - return false; - - /* Extended capabilities do not change at runtime. */ - if (hv_extended_cap_queried) - return hv_extended_cap & cap_query; - - status = hv_do_hypercall(HV_EXT_CALL_QUERY_CAPABILITIES, NULL, - &hv_extended_cap); - - /* - * The query extended capabilities hypercall should not fail under - * any normal circumstances. Avoid repeatedly making the hypercall, on - * error. - */ - hv_extended_cap_queried = true; - status &= HV_HYPERCALL_RESULT_MASK; - if (status != HV_STATUS_SUCCESS) { - pr_err("Hyper-V: Extended query capabilities hypercall failed 0x%llx\n", - status); - return false; - } - - return hv_extended_cap & cap_query; -} -EXPORT_SYMBOL_GPL(hv_query_ext_cap); diff --git a/drivers/Makefile b/drivers/Makefile index 5a6d613e868d..1c2e1acbd098 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -161,7 +161,7 @@ obj-$(CONFIG_SOUNDWIRE) += soundwire/ # Virtualization drivers obj-$(CONFIG_VIRT_DRIVERS) += virt/ -obj-$(CONFIG_HYPERV) += hv/ +obj-$(subst m,y,$(CONFIG_HYPERV)) += hv/ obj-$(CONFIG_PM_DEVFREQ) += devfreq/ obj-$(CONFIG_EXTCON) += extcon/ diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile index 94daf8240c95..d76df5c8c2a9 100644 --- a/drivers/hv/Makefile +++ b/drivers/hv/Makefile @@ -11,3 +11,6 @@ hv_vmbus-y := vmbus_drv.o \ channel_mgmt.o ring_buffer.o hv_trace.o hv_vmbus-$(CONFIG_HYPERV_TESTING) += hv_debugfs.o hv_utils-y := hv_util.o hv_kvp.o hv_snapshot.o hv_fcopy.o hv_utils_transport.o + +# Code that must be built-in +obj-$(subst m,y,$(CONFIG_HYPERV)) += hv_common.o diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c new file mode 100644 index 000000000000..f0053c786891 --- /dev/null +++ b/drivers/hv/hv_common.c @@ -0,0 +1,66 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* + * Architecture neutral utility routines for interacting with + * Hyper-V. This file is specifically for code that must be + * built-in to the kernel image when CONFIG_HYPERV is set + * (vs. being in a module) because it is called from architecture + * specific code under arch/. + * + * Copyright (C) 2021, Microsoft, Inc. + * + * Author : Michael Kelley + */ + +#include +#include +#include +#include +#include + + +/* Bit mask of the extended capability to query: see HV_EXT_CAPABILITY_xxx */ +bool hv_query_ext_cap(u64 cap_query) +{ + /* + * The address of the 'hv_extended_cap' variable will be used as an + * output parameter to the hypercall below and so it should be + * compatible with 'virt_to_phys'. Which means, it's address should be + * directly mapped. Use 'static' to keep it compatible; stack variables + * can be virtually mapped, making them imcompatible with + * 'virt_to_phys'. + * Hypercall input/output addresses should also be 8-byte aligned. + */ + static u64 hv_extended_cap __aligned(8); + static bool hv_extended_cap_queried; + u64 status; + + /* + * Querying extended capabilities is an extended hypercall. Check if the + * partition supports extended hypercall, first. + */ + if (!(ms_hyperv.priv_high & HV_ENABLE_EXTENDED_HYPERCALLS)) + return false; + + /* Extended capabilities do not change at runtime. */ + if (hv_extended_cap_queried) + return hv_extended_cap & cap_query; + + status = hv_do_hypercall(HV_EXT_CALL_QUERY_CAPABILITIES, NULL, + &hv_extended_cap); + + /* + * The query extended capabilities hypercall should not fail under + * any normal circumstances. Avoid repeatedly making the hypercall, on + * error. + */ + hv_extended_cap_queried = true; + if (!hv_result_success(status)) { + pr_err("Hyper-V: Extended query capabilities hypercall failed 0x%llx\n", + status); + return false; + } + + return hv_extended_cap & cap_query; +} +EXPORT_SYMBOL_GPL(hv_query_ext_cap); -- cgit v1.2.3