From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 89DA4C433E0 for ; Mon, 29 Jun 2020 20:03:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 61B9D20672 for ; Mon, 29 Jun 2020 20:03:17 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="OINhy7Xm" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388728AbgF2UDC (ORCPT ); Mon, 29 Jun 2020 16:03:02 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51736 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2388726AbgF2UDB (ORCPT ); Mon, 29 Jun 2020 16:03:01 -0400 Received: from mail-pg1-x544.google.com (mail-pg1-x544.google.com [IPv6:2607:f8b0:4864:20::544]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4F77FC061755; Mon, 29 Jun 2020 13:03:01 -0700 (PDT) Received: by mail-pg1-x544.google.com with SMTP id g67so7901092pgc.8; Mon, 29 Jun 2020 13:03:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:reply-to:content-transfer-encoding; bh=/gsLg2vrEv9v1DomZy+Al4WDSR7fwp7hS7EGj3MRF9E=; b=OINhy7XmDGVCP+SCSbzALvV19PfCB1+fzui8uoh5RLBtHAbSruN0EHWC9b/6uIbRm1 L+FVanMWZn1CEAUoXFVDKdrBbbE9XrBHhkFyHRfo5GNJRDFlVHC1Mvtg6VxOQgTfBJKC q8HKvfKEJNuCC/3M7yJcIME8PpytDnGvkhVy41csb0zX19TonjbGt569EEFGPJW3RkLR FGccVh73zZBCSDuHtVuQeP7hKwZkT43hb34fsyXZET/MwMrjfVd1H9XZzi6ZzSX9cs2F Cg7ZWHIj21mOZJJ01NpjDnByRY0zVlpPjpqx1lX22Vw9HORZOErV1JheyjkilTRf1P6e rtbA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:reply-to:content-transfer-encoding; bh=/gsLg2vrEv9v1DomZy+Al4WDSR7fwp7hS7EGj3MRF9E=; b=iwBPdSjfllY/70MaxHAxopCmmx1A2S5inXV+03QGzw287McKEM7yPLh/aK+ny9b+bd 13pwi3ShIEqsTCUxL4Jrs9kyrsPiyLKVrFPCZsUwrMF6VGBy01lPP0GjH1GyLN5RMcec XvXP6BbdkC49pPs7+kPI1YlIgweT+CaD9EeRdd3n6EW28oMkIKt1B+JNks5bsbP2hJgP VWZi7wZICh2A4PKsXp/uutwt/kY1jvm3tP00MydWceCjqxaDRPFGtqW1DfhsQ3nodOdm pnIFgoUjWolN/QwzNcoTVwcKWgR7/RGb/q25kOjz8XZYh1+SVHBYEpo61ZEvst9Pi7BF 7mTA== X-Gm-Message-State: AOAM532P6ohkQuVJ0szkESq2835k5l2xc+2RiN4I4qQws+butbD1w9gD c7YkaUHIpAzMp9mbp1/Frhc= X-Google-Smtp-Source: ABdhPJw10/Z46CoL3/bysljxRW4wxA98a7qm+xk5KD8WGDK3bO0ynrD6ip2gqGuMQzxANBrCH+9h6g== X-Received: by 2002:a63:c34e:: with SMTP id e14mr11367248pgd.55.1593460980873; Mon, 29 Jun 2020 13:03:00 -0700 (PDT) Received: from localhost.localdomain ([131.107.160.194]) by smtp.gmail.com with ESMTPSA id j10sm531558pgh.28.2020.06.29.13.03.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 29 Jun 2020 13:03:00 -0700 (PDT) From: Andres Beltran To: kys@microsoft.com, haiyangz@microsoft.com, sthemmin@microsoft.com, wei.liu@kernel.org Cc: linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org, mikelley@microsoft.com, parri.andrea@gmail.com, Andres Beltran , "David S . Miller" , Jakub Kicinski , netdev@vger.kernel.org Subject: [PATCH v2 3/3] hv_netvsc: Use vmbus_requestor to generate transaction IDs for VMBus hardening Date: Mon, 29 Jun 2020 16:02:27 -0400 Message-Id: <20200629200227.1518784-4-lkmlabelt@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200629200227.1518784-1-lkmlabelt@gmail.com> References: <20200629200227.1518784-1-lkmlabelt@gmail.com> MIME-Version: 1.0 Reply-To: t-mabelt@microsoft.com Content-Transfer-Encoding: 8bit Sender: linux-hyperv-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-hyperv@vger.kernel.org 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 Cc: Jakub Kicinski Cc: netdev@vger.kernel.org Signed-off-by: Andres Beltran --- Changes in v2: - Add casts to unsigned long to fix warnings on 32bit. - Use an inline function to get the requestor size. drivers/net/hyperv/hyperv_net.h | 13 +++++ drivers/net/hyperv/netvsc.c | 79 +++++++++++++++++++++++++------ drivers/net/hyperv/rndis_filter.c | 1 + include/linux/hyperv.h | 1 + 4 files changed, 80 insertions(+), 14 deletions(-) diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h index abda736e7c7d..f43b614f2345 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; +} + 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..79b907a29433 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,22 @@ static int netvsc_init_buf(struct hv_device *device, trace_nvsp_send(ndev, init_packet); + rqst_id = vmbus_next_request_id(&device->channel->requestor, + (unsigned long)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 +442,22 @@ static int netvsc_init_buf(struct hv_device *device, trace_nvsp_send(ndev, init_packet); + rqst_id = vmbus_next_request_id(&device->channel->requestor, + (unsigned long)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 +515,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 +523,25 @@ 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, + (unsigned long)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 +572,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 +629,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 +710,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)) { @@ -822,7 +861,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 +877,19 @@ 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, + (unsigned long)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 +897,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 +914,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 +1472,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 b81ceba38218..10489ba44a09 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(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 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