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]
Date:   Fri, 26 Jan 2018 16:18:13 -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 1/2] hv_netvsc: make sure device is idle before changes

Make sure that device is in detached state before doing ring
and mtu changes. When doing these changes, wait for all outstanding
send completions and ring buffer events.

Signed-off-by: Stephen Hemminger <stephen@...workplumber.org>
---
 drivers/net/hyperv/hyperv_net.h   |  1 -
 drivers/net/hyperv/netvsc.c       |  6 +----
 drivers/net/hyperv/netvsc_drv.c   | 29 +++++++++++++----------
 drivers/net/hyperv/rndis_filter.c | 48 ++++++++++++++++++---------------------
 4 files changed, 40 insertions(+), 44 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 0db3bd1ea06f..a846a9c50ddb 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -211,7 +211,6 @@ void netvsc_channel_cb(void *context);
 int netvsc_poll(struct napi_struct *napi, int budget);
 
 void rndis_set_subchannel(struct work_struct *w);
-bool rndis_filter_opened(const struct netvsc_device *nvdev);
 int rndis_filter_open(struct netvsc_device *nvdev);
 int rndis_filter_close(struct netvsc_device *nvdev);
 struct netvsc_device *rndis_filter_device_add(struct hv_device *dev,
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 17e529af79dc..619a04f98321 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -849,7 +849,7 @@ int netvsc_send(struct net_device *ndev,
 	bool try_batch, xmit_more;
 
 	/* If device is rescinded, return error and packet will get dropped. */
-	if (unlikely(!net_device || net_device->destroy))
+	if (unlikely(!net_device))
 		return -ENODEV;
 
 	/* We may race with netvsc_connect_vsp()/netvsc_init_buf() and get
@@ -996,10 +996,6 @@ static int send_recv_completions(struct net_device *ndev,
 			mrc->first = 0;
 	}
 
-	/* receive completion ring has been emptied */
-	if (unlikely(nvdev->destroy))
-		wake_up(&nvdev->wait_drain);
-
 	return 0;
 }
 
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index c5584c2d440e..ef395e379a83 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -820,7 +820,7 @@ static int netvsc_set_channels(struct net_device *net,
 	    channels->rx_count || channels->tx_count || channels->other_count)
 		return -EINVAL;
 
-	if (!nvdev || nvdev->destroy)
+	if (!nvdev)
 		return -ENODEV;
 
 	if (nvdev->nvsp_version < NVSP_PROTOCOL_VERSION_5)
@@ -830,9 +830,6 @@ static int netvsc_set_channels(struct net_device *net,
 		return -EINVAL;
 
 	orig = nvdev->num_chn;
-	was_opened = rndis_filter_opened(nvdev);
-	if (was_opened)
-		rndis_filter_close(nvdev);
 
 	memset(&device_info, 0, sizeof(device_info));
 	device_info.num_chn = count;
@@ -841,6 +838,11 @@ static int netvsc_set_channels(struct net_device *net,
 	device_info.recv_sections = nvdev->recv_section_cnt;
 	device_info.recv_section_size = nvdev->recv_section_size;
 
+	was_opened = netif_running(net);
+	netif_device_detach(net);
+	if (was_opened)
+		rndis_filter_close(nvdev);
+
 	rndis_filter_device_remove(dev, nvdev);
 
 	nvdev = rndis_filter_device_add(dev, &device_info);
@@ -859,6 +861,8 @@ static int netvsc_set_channels(struct net_device *net,
 	if (was_opened)
 		rndis_filter_open(nvdev);
 
+	netif_device_attach(net);
+
 	/* We may have missed link change notifications */
 	net_device_ctx->last_reconfig = 0;
 	schedule_delayed_work(&net_device_ctx->dwork, 0);
@@ -934,7 +938,7 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
 	bool was_opened;
 	int ret = 0;
 
-	if (!nvdev || nvdev->destroy)
+	if (!nvdev)
 		return -ENODEV;
 
 	/* Change MTU of underlying VF netdev first. */
@@ -944,11 +948,6 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
 			return ret;
 	}
 
-	netif_device_detach(ndev);
-	was_opened = rndis_filter_opened(nvdev);
-	if (was_opened)
-		rndis_filter_close(nvdev);
-
 	memset(&device_info, 0, sizeof(device_info));
 	device_info.num_chn = nvdev->num_chn;
 	device_info.send_sections = nvdev->send_section_cnt;
@@ -956,6 +955,11 @@ static int netvsc_change_mtu(struct net_device *ndev, int mtu)
 	device_info.recv_sections = nvdev->recv_section_cnt;
 	device_info.recv_section_size = nvdev->recv_section_size;
 
+	was_opened = netif_running(ndev);
+	netif_device_detach(ndev);
+	if (was_opened)
+		rndis_filter_close(nvdev);
+
 	rndis_filter_device_remove(hdev, nvdev);
 
 	ndev->mtu = mtu;
@@ -1497,7 +1501,7 @@ static int netvsc_set_ringparam(struct net_device *ndev,
 	bool was_opened;
 	int ret = 0;
 
-	if (!nvdev || nvdev->destroy)
+	if (!nvdev)
 		return -ENODEV;
 
 	memset(&orig, 0, sizeof(orig));
@@ -1519,8 +1523,8 @@ static int netvsc_set_ringparam(struct net_device *ndev,
 	device_info.recv_sections = new_rx;
 	device_info.recv_section_size = nvdev->recv_section_size;
 
+	was_opened = netif_running(ndev);
 	netif_device_detach(ndev);
-	was_opened = rndis_filter_opened(nvdev);
 	if (was_opened)
 		rndis_filter_close(nvdev);
 
@@ -1542,6 +1546,7 @@ static int netvsc_set_ringparam(struct net_device *ndev,
 
 	if (was_opened)
 		rndis_filter_open(nvdev);
+
 	netif_device_attach(ndev);
 
 	/* We may have missed link change notifications */
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index c3ca191fea7f..a6c9c9a2973d 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -923,6 +923,7 @@ static int rndis_filter_init_device(struct rndis_device *dev,
 	return ret;
 }
 
+/* Are there any outstanding sends */
 static bool netvsc_device_idle(const struct netvsc_device *nvdev)
 {
 	int i;
@@ -930,8 +931,7 @@ static bool netvsc_device_idle(const struct netvsc_device *nvdev)
 	for (i = 0; i < nvdev->num_chn; i++) {
 		const struct netvsc_channel *nvchan = &nvdev->chan_table[i];
 
-		if (nvchan->mrc.first != nvchan->mrc.next)
-			return false;
+		napi_synchronize(&nvchan->napi);
 
 		if (atomic_read(&nvchan->queue_sends) > 0)
 			return false;
@@ -944,14 +944,12 @@ static void rndis_filter_halt_device(struct rndis_device *dev)
 {
 	struct rndis_request *request;
 	struct rndis_halt_request *halt;
-	struct net_device_context *net_device_ctx = netdev_priv(dev->ndev);
-	struct netvsc_device *nvdev = rtnl_dereference(net_device_ctx->nvdev);
 
 	/* Attempt to do a rndis device halt */
 	request = get_rndis_request(dev, RNDIS_MSG_HALT,
 				RNDIS_MESSAGE_SIZE(struct rndis_halt_request));
 	if (!request)
-		goto cleanup;
+		return;
 
 	/* Setup the rndis set */
 	halt = &request->request_msg.msg.halt_req;
@@ -962,17 +960,7 @@ static void rndis_filter_halt_device(struct rndis_device *dev)
 
 	dev->state = RNDIS_DEV_UNINITIALIZED;
 
-cleanup:
-	nvdev->destroy = true;
-
-	/* Force flag to be ordered before waiting */
-	wmb();
-
-	/* Wait for all send completions */
-	wait_event(nvdev->wait_drain, netvsc_device_idle(nvdev));
-
-	if (request)
-		put_rndis_request(dev, request);
+	put_rndis_request(dev, request);
 }
 
 static int rndis_filter_open_device(struct rndis_device *dev)
@@ -994,9 +982,11 @@ static int rndis_filter_open_device(struct rndis_device *dev)
 
 static int rndis_filter_close_device(struct rndis_device *dev)
 {
+	struct net_device_context *net_device_ctx = netdev_priv(dev->ndev);
+	struct netvsc_device *nvdev = rtnl_dereference(net_device_ctx->nvdev);
 	int ret;
 
-	if (dev->state != RNDIS_DEV_DATAINITIALIZED)
+	if (!nvdev || dev->state != RNDIS_DEV_DATAINITIALIZED)
 		return 0;
 
 	/* Make sure rndis_set_multicast doesn't re-enable filter! */
@@ -1004,10 +994,23 @@ static int rndis_filter_close_device(struct rndis_device *dev)
 
 	ret = rndis_filter_set_packet_filter(dev, 0);
 	if (ret == -ENODEV)
-		ret = 0;
+		ret = 0;	/* rescinded is ok */
+
+	if (ret == 0) {
+		/* Indicate that wakeup on send done is desired */
+		nvdev->destroy = true;
+
+		/* Force flag to be ordered before waiting */
+		wmb();
+
+		/* Wait for all completions */
+		wait_event(nvdev->wait_drain, netvsc_device_idle(nvdev));
+
+		/* No more wakeups please */
+		nvdev->destroy = false;
 
-	if (ret == 0)
 		dev->state = RNDIS_DEV_INITIALIZED;
+	}
 
 	return ret;
 }
@@ -1364,10 +1367,3 @@ int rndis_filter_close(struct netvsc_device *nvdev)
 
 	return rndis_filter_close_device(nvdev->extension);
 }
-
-bool rndis_filter_opened(const struct netvsc_device *nvdev)
-{
-	const struct rndis_device *dev = nvdev->extension;
-
-	return dev->state == RNDIS_DEV_DATAINITIALIZED;
-}
-- 
2.15.1

Powered by blists - more mailing lists