netdev - [RFC 2/2] Revert "hv_netvsc: netvsc_teardown

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20180127001814.11203-3-sthemmin@microsoft.com>
Date:   Fri, 26 Jan 2018 16:18:14 -0800
From:   Stephen Hemminger <stephen@...workplumber.org>
To:     kys@...rosoft.com, haiyangz@...rosoft.com, vkuznets@...hat.com,
        mgamal@...hat.com
Cc:     netdev@...r.kernel.org, Stephen Hemminger <sthemmin@...rosoft.com>,
        Stephen Hemminger <stephen@...workplumber.org>
Subject: [RFC 2/2] Revert "hv_netvsc: netvsc_teardown_gpadl() split"

This reverts commit 0cf737808ae7cb25e952be619db46b9147a92f46.

The problem that the previous commit was trying to solve was that
undoing the mapping of the receive buffer after revoke was
problematic.  This was because the shutdown logic was not ensuring
that there were no receive and sends in flight. The changes in
commit 30de1885e897 ("hv_netvsc: make sure device is idle before changes")
ensure that device is completely idle.

Windows Server 2012 does not allow the receive buffer mapping to be
undone after the channel is closed. This because it assumes when
channel is closed, guest won't use it.

Signed-off-by: Stephen Hemminger <stephen@...workplumber.org>
---
 drivers/net/hyperv/netvsc.c | 69 ++++++++++++++++++++++-----------------------
 1 file changed, 33 insertions(+), 36 deletions(-)

diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 619a04f98321..6db9bfb5c595 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -101,11 +101,12 @@ static void free_netvsc_device_rcu(struct netvsc_device *nvdev)
 	call_rcu(&nvdev->rcu, free_netvsc_device);
 }
 
-static void netvsc_revoke_buf(struct hv_device *device,
-			      struct netvsc_device *net_device)
+static void netvsc_destroy_buf(struct hv_device *device)
 {
 	struct nvsp_message *revoke_packet;
 	struct net_device *ndev = hv_get_drvdata(device);
+	struct net_device_context *ndc = netdev_priv(ndev);
+	struct netvsc_device *net_device = rtnl_dereference(ndc->nvdev);
 	int ret;
 
 	/*
@@ -148,6 +149,28 @@ static void netvsc_revoke_buf(struct hv_device *device,
 		net_device->recv_section_cnt = 0;
 	}
 
+	/* Teardown the gpadl on the vsp end */
+	if (net_device->recv_buf_gpadl_handle) {
+		ret = vmbus_teardown_gpadl(device->channel,
+					   net_device->recv_buf_gpadl_handle);
+
+		/* If we failed here, we might as well return and have a leak
+		 * rather than continue and a bugchk
+		 */
+		if (ret != 0) {
+			netdev_err(ndev,
+				   "unable to teardown receive buffer's gpadl\n");
+			return;
+		}
+		net_device->recv_buf_gpadl_handle = 0;
+	}
+
+	if (net_device->recv_buf) {
+		/* Free up the receive buffer */
+		vfree(net_device->recv_buf);
+		net_device->recv_buf = NULL;
+	}
+
 	/* Deal with the send buffer we may have setup.
 	 * If we got a  send section size, it means we received a
 	 * NVSP_MSG1_TYPE_SEND_SEND_BUF_COMPLETE msg (ie sent
@@ -188,35 +211,7 @@ static void netvsc_revoke_buf(struct hv_device *device,
 		}
 		net_device->send_section_cnt = 0;
 	}
-}
-
-static void netvsc_teardown_gpadl(struct hv_device *device,
-				  struct netvsc_device *net_device)
-{
-	struct net_device *ndev = hv_get_drvdata(device);
-	int ret;
-
-	if (net_device->recv_buf_gpadl_handle) {
-		ret = vmbus_teardown_gpadl(device->channel,
-					   net_device->recv_buf_gpadl_handle);
-
-		/* If we failed here, we might as well return and have a leak
-		 * rather than continue and a bugchk
-		 */
-		if (ret != 0) {
-			netdev_err(ndev,
-				   "unable to teardown receive buffer's gpadl\n");
-			return;
-		}
-		net_device->recv_buf_gpadl_handle = 0;
-	}
-
-	if (net_device->recv_buf) {
-		/* Free up the receive buffer */
-		vfree(net_device->recv_buf);
-		net_device->recv_buf = NULL;
-	}
-
+	/* Teardown the gpadl on the vsp end */
 	if (net_device->send_buf_gpadl_handle) {
 		ret = vmbus_teardown_gpadl(device->channel,
 					   net_device->send_buf_gpadl_handle);
@@ -431,8 +426,7 @@ static int netvsc_init_buf(struct hv_device *device,
 	goto exit;
 
 cleanup:
-	netvsc_revoke_buf(device, net_device);
-	netvsc_teardown_gpadl(device, net_device);
+	netvsc_destroy_buf(device);
 
 exit:
 	return ret;
@@ -551,6 +545,11 @@ static int netvsc_connect_vsp(struct hv_device *device,
 	return ret;
 }
 
+static void netvsc_disconnect_vsp(struct hv_device *device)
+{
+	netvsc_destroy_buf(device);
+}
+
 /*
  * netvsc_device_remove - Callback when the root bus device is removed
  */
@@ -564,7 +563,7 @@ void netvsc_device_remove(struct hv_device *device)
 
 	cancel_work_sync(&net_device->subchan_work);
 
-	netvsc_revoke_buf(device, net_device);
+	netvsc_disconnect_vsp(device);
 
 	RCU_INIT_POINTER(net_device_ctx->nvdev, NULL);
 
@@ -577,8 +576,6 @@ void netvsc_device_remove(struct hv_device *device)
 	/* Now, we can close the channel safely */
 	vmbus_close(device->channel);
 
-	netvsc_teardown_gpadl(device, net_device);
-
 	/* And dissassociate NAPI context from device */
 	for (i = 0; i < net_device->num_chn; i++)
 		netif_napi_del(&net_device->chan_table[i].napi);
-- 
2.15.1