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=-13.1 required=3.0 tests=BAYES_00,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,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 E9FE4C433E3 for ; Wed, 22 Jul 2020 22:39:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B251B22B43 for ; Wed, 22 Jul 2020 22:39:14 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="blOFuuB0" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1733057AbgGVWjK (ORCPT ); Wed, 22 Jul 2020 18:39:10 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43494 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1733039AbgGVWjJ (ORCPT ); Wed, 22 Jul 2020 18:39:09 -0400 Received: from mail-pf1-x443.google.com (mail-pf1-x443.google.com [IPv6:2607:f8b0:4864:20::443]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DBE95C0619E2; Wed, 22 Jul 2020 15:39:08 -0700 (PDT) Received: by mail-pf1-x443.google.com with SMTP id u185so2035419pfu.1; Wed, 22 Jul 2020 15:39:08 -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:content-transfer-encoding; bh=BHshCalt4OMpzpgm5pG7X6ddzCe31OEWzs6OlsIqweY=; b=blOFuuB0vyXTXcahUZFXq+AHeuCa2sHVekIr8WIgRPbskhj4qJIeNgStJ8JtPYCaQN nVCkfuEQeeikZmfPGI0flpgTGCbxxUEUEBRm44BsWGIK/TuVO4MwfYi2TpNQtQ2OU17d Vp9a1+kGtA+JhPTLiUAXaVm6LpGPrQGb74GJPn/uDpNinKwRT9D8lcDv3QdnYpYEJVg1 ROOh63u2lP3usRIpC6GbKjAMZ6GcpjOIfshYrvn82mWIbsTg2n2bYXE/s2X2ffTiafBA flVj1YuVchC9WNt6WMRLEIyuwBlkZ4clgyY5YZr7Zl8yS/Q/SnvzYHzH0JfwrbZhZiEK LgNQ== 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:content-transfer-encoding; bh=BHshCalt4OMpzpgm5pG7X6ddzCe31OEWzs6OlsIqweY=; b=fZhm3eGrI7Ecp6N+TRv1Ojll1gFtIM//tIHAGPxUI16nmFSjzcOrn13HMDQZjdW9o2 tZIk2EaWLfdvbcLm/gyxLHAB1FUVyFwjCP8fXGfHFWWCtKwfvP7eSvjmp7272Fo/V7mZ g7TbTrKSjEzEw8eovCC35qlBDyBOzE6yZEVMz9voSGuUGTqzciYEuEY8D5y+T1Y5r0hG rGxaLqbFfe8ftdV2GNTTNA6YVgE5MF31Nm9eODXZEJDxlzsTnLLZlPg64CSBiU9E3mL0 qjxa/0LISIBOc7BzwkU0RHR5CIbn50DWx8FWr1dojU6xwPFaqZA4e0aFTqW5NnHQPFp/ Rd/Q== X-Gm-Message-State: AOAM5304SYbHoRzt1x58Jb0oprYADMNcds9RLTia4TYgcvalYkgNHjkh o916OCi4Nx8Fv3Y6IDrgG+Q= X-Google-Smtp-Source: ABdhPJz9xiY9s9hzN2tpzoHKXa/YbwtEP3UMfMGhuGDkTSEdVrA00qvMmWE2bCl7UgYnehJwbrUENQ== X-Received: by 2002:a62:e712:: with SMTP id s18mr1603605pfh.224.1595457548420; Wed, 22 Jul 2020 15:39:08 -0700 (PDT) Received: from localhost.localdomain ([131.107.159.194]) by smtp.gmail.com with ESMTPSA id r70sm625760pfc.109.2020.07.22.15.39.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Jul 2020 15:39:08 -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 v6 3/3] hv_netvsc: Use vmbus_requestor to generate transaction IDs for VMBus hardening Date: Wed, 22 Jul 2020 18:39:04 -0400 Message-Id: <20200722223904.2801-4-lkmlabelt@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200722223904.2801-1-lkmlabelt@gmail.com> References: <20200722223904.2801-1-lkmlabelt@gmail.com> MIME-Version: 1.0 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 Reviewed-by: Haiyang Zhang Tested-by: Andrea Parri Link: https://lore.kernel.org/r/20200701001221.2540-4-lkmlabelt@gmail.com Signed-off-by: Wei Liu --- 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