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. Cc: "David S. Miller" <davem@xxxxxxxxxxxxx> Cc: Jakub Kicinski <kuba@xxxxxxxxxx> Cc: netdev@xxxxxxxxxxxxxxx Signed-off-by: Andres Beltran <lkmlabelt@xxxxxxxxx> --- drivers/net/hyperv/hyperv_net.h | 10 +++++ drivers/net/hyperv/netvsc.c | 75 +++++++++++++++++++++++++------ drivers/net/hyperv/rndis_filter.c | 1 + include/linux/hyperv.h | 1 + 4 files changed, 73 insertions(+), 14 deletions(-) diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h index abda736e7c7d..14735c98e798 100644 --- a/drivers/net/hyperv/hyperv_net.h +++ b/drivers/net/hyperv/hyperv_net.h @@ -847,6 +847,16 @@ 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 + */ +#define NETVSC_RQSTOR_SIZE (netvsc_ring_bytes / NETVSC_MIN_OUT_MSG_SIZE + \ + netvsc_ring_bytes / NETVSC_MIN_IN_MSG_SIZE) + struct multi_send_data { struct sk_buff *skb; /* skb containing the pkt */ struct hv_netvsc_packet *pkt; /* netvsc pkt pending */ diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c index 41f5cf0bb997..c73d5aef4436 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; @@ -304,6 +304,7 @@ static int netvsc_init_buf(struct hv_device *device, unsigned int buf_size; size_t map_words; int ret = 0; + u64 rqst_id; /* Get receive buffer area. */ buf_size = device_info->recv_sections * device_info->recv_section_size; @@ -350,13 +351,21 @@ static int netvsc_init_buf(struct hv_device *device, trace_nvsp_send(ndev, init_packet); + rqst_id = vmbus_next_request_id(&device->channel->requestor, (u64)init_packet); + if (rqst_id == VMBUS_RQST_ERROR) { + netdev_err(ndev, "No request id available\n"); + goto cleanup; + } + /* Send the gpadl notification request */ ret = vmbus_sendpacket(device->channel, init_packet, sizeof(struct nvsp_message), - (unsigned long)init_packet, + 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); netdev_err(ndev, "unable to send receive buffer's gpadl to netvsp\n"); goto cleanup; @@ -432,13 +441,21 @@ static int netvsc_init_buf(struct hv_device *device, trace_nvsp_send(ndev, init_packet); + rqst_id = vmbus_next_request_id(&device->channel->requestor, (u64)init_packet); + if (rqst_id == VMBUS_RQST_ERROR) { + netdev_err(ndev, "No request id available\n"); + goto cleanup; + } + /* Send the gpadl notification request */ ret = vmbus_sendpacket(device->channel, init_packet, sizeof(struct nvsp_message), - (unsigned long)init_packet, + 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); netdev_err(ndev, "unable to send send buffer's gpadl to netvsp\n"); goto cleanup; @@ -496,6 +513,7 @@ static int negotiate_nvsp_ver(struct hv_device *device, { struct net_device *ndev = hv_get_drvdata(device); int ret; + u64 rqst_id; memset(init_packet, 0, sizeof(struct nvsp_message)); init_packet->hdr.msg_type = NVSP_MSG_TYPE_INIT; @@ -503,15 +521,24 @@ static int negotiate_nvsp_ver(struct hv_device *device, init_packet->msg.init_msg.init.max_protocol_ver = nvsp_ver; trace_nvsp_send(ndev, init_packet); + rqst_id = vmbus_next_request_id(&device->channel->requestor, (u64)init_packet); + if (rqst_id == VMBUS_RQST_ERROR) { + netdev_err(ndev, "No request id available\n"); + return -EAGAIN; + } + /* Send the init request */ ret = vmbus_sendpacket(device->channel, init_packet, sizeof(struct nvsp_message), - (unsigned long)init_packet, + 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; + } wait_for_completion(&net_device->channel_init_wait); @@ -542,7 +569,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; @@ -599,7 +626,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; @@ -680,10 +707,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 *)cmd_rqst; /* Notify the layer above us */ if (likely(skb)) { @@ -822,7 +858,7 @@ static inline int netvsc_send_pkt( struct net_device *ndev = hv_get_drvdata(device); struct net_device_context *ndev_ctx = netdev_priv(ndev); struct netdev_queue *txq = netdev_get_tx_queue(ndev, packet->q_idx); - u64 req_id; + u64 rqst_id; int ret; u32 ring_avail = hv_get_avail_to_write_percent(&out_channel->outbound); @@ -838,13 +874,18 @@ static inline int netvsc_send_pkt( else rpkt->send_buf_section_size = packet->total_data_buflen; - req_id = (ulong)skb; if (out_channel->rescind) return -ENODEV; trace_nvsp_send_pkt(ndev, out_channel, rpkt); + rqst_id = vmbus_next_request_id(&out_channel->requestor, (u64)skb); + if (rqst_id == VMBUS_RQST_ERROR) { + ret = -EAGAIN; + goto ret_check; + } + if (packet->page_buf_cnt) { if (packet->cp_partial) pb += packet->rmsg_pgcnt; @@ -852,14 +893,15 @@ static inline int netvsc_send_pkt( ret = vmbus_sendpacket_pagebuffer(out_channel, pb, packet->page_buf_cnt, &nvmsg, sizeof(nvmsg), - req_id); + rqst_id); } else { ret = vmbus_sendpacket(out_channel, &nvmsg, sizeof(nvmsg), - req_id, VM_PKT_DATA_INBAND, + rqst_id, VM_PKT_DATA_INBAND, VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED); } +ret_check: if (ret == 0) { atomic_inc_return(&nvchan->queue_sends); @@ -868,9 +910,13 @@ static inline int netvsc_send_pkt( ndev_ctx->eth_stats.stop_queue++; } } else if (ret == -EAGAIN) { + /* Reclaim request ID to avoid leak of IDs */ + vmbus_request_addr(&out_channel->requestor, rqst_id); netif_tx_stop_queue(txq); ndev_ctx->eth_stats.stop_queue++; } else { + /* Reclaim request ID to avoid leak of IDs */ + vmbus_request_addr(&out_channel->requestor, rqst_id); netdev_err(ndev, "Unable to send packet pages %u len %u, ret %d\n", packet->page_buf_cnt, packet->total_data_buflen, @@ -1422,6 +1468,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; 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 b81ceba38218..4d96e8e5ea24 100644 --- a/drivers/net/hyperv/rndis_filter.c +++ b/drivers/net/hyperv/rndis_filter.c @@ -1114,6 +1114,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; 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 c509d20ab7db..d8194924983d 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -730,6 +730,7 @@ struct vmbus_requestor { }; #define VMBUS_RQST_ERROR U64_MAX +#define VMBUS_RQST_ID_NO_RESPONSE (U64_MAX - 1) struct vmbus_device { u16 dev_type; -- 2.25.1