Fix a lock-order reversal in the net driver by dropping the lock

and holding a reference prior to calling further into the hyperv
stack.

Added missing FreeBSD idents.

Submitted by:	Microsoft hyperv dev team
Approved by:	re@ (gjb)

2 files changed, 86 insertions, 19 deletions

diff --git a/sys/dev/hyperv/netvsc/hv_net_vsc.h b/sys/dev/hyperv/netvsc/hv_net_vsc.h
index f7e7d00..b9f3a1e 100644
--- a/sys/dev/hyperv/netvsc/hv_net_vsc.h
+++ b/sys/dev/hyperv/netvsc/hv_net_vsc.h
@@ -24,6 +24,8 @@
  * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
  * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * $FreeBSD$
  */
 
 /*
@@ -970,6 +972,8 @@ typedef struct hn_softc {
 	int             hn_if_flags;
 	struct mtx      hn_lock;
 	int             hn_initdone;
+	/* See hv_netvsc_drv_freebsd.c for rules on how to use */
+	int             temp_unusable;
 	struct hv_device  *hn_dev_obj;
 	netvsc_dev  	*net_dev;
 } hn_softc_t;
diff --git a/sys/dev/hyperv/netvsc/hv_netvsc_drv_freebsd.c b/sys/dev/hyperv/netvsc/hv_netvsc_drv_freebsd.c
index 7470006..f6ace58 100644
--- a/sys/dev/hyperv/netvsc/hv_netvsc_drv_freebsd.c
+++ b/sys/dev/hyperv/netvsc/hv_netvsc_drv_freebsd.c
@@ -52,6 +52,9 @@
  * SUCH DAMAGE.
  */
 
+#include <sys/cdefs.h>
+__FBSDID("$FreeBSD$");
+
 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/sockio.h>
@@ -702,6 +705,17 @@ netvsc_recv(struct hv_device *device_ctx, netvsc_packet *packet)
 }
 
 /*
+ * Rules for using sc->temp_unusable:
+ * 1.  sc->temp_unusable can only be read or written while holding NV_LOCK()
+ * 2.  code reading sc->temp_unusable under NV_LOCK(), and finding 
+ *     sc->temp_unusable set, must release NV_LOCK() and exit
+ * 3.  to retain exclusive control of the interface,
+ *     sc->temp_unusable must be set by code before releasing NV_LOCK()
+ * 4.  only code setting sc->temp_unusable can clear sc->temp_unusable
+ * 5.  code setting sc->temp_unusable must eventually clear sc->temp_unusable
+ */
+
+/*
  * Standard ioctl entry point.  Called when the user wants to configure
  * the interface.
  */
@@ -713,7 +727,8 @@ hn_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
 	netvsc_device_info device_info;
 	struct hv_device *hn_dev;
 	int mask, error = 0;
-
+	int retry_cnt = 500;
+	
 	switch(cmd) {
 
 	case SIOCSIFADDR:
@@ -723,38 +738,80 @@ hn_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
 	case SIOCSIFMTU:
 		hn_dev = vmbus_get_devctx(sc->hn_dev);
 
-		NV_LOCK(sc);
+		/* Check MTU value change */
+		if (ifp->if_mtu == ifr->ifr_mtu)
+			break;
 
 		if (ifr->ifr_mtu > NETVSC_MAX_CONFIGURABLE_MTU) {
 			error = EINVAL;
-			NV_UNLOCK(sc);
 			break;
 		}
+
 		/* Obtain and record requested MTU */
 		ifp->if_mtu = ifr->ifr_mtu;
+ 		
+		do {
+			NV_LOCK(sc);
+			if (!sc->temp_unusable) {
+				sc->temp_unusable = TRUE;
+				retry_cnt = -1;
+			}
+			NV_UNLOCK(sc);
+			if (retry_cnt > 0) {
+				retry_cnt--;
+				DELAY(5 * 1000);
+			}
+		} while (retry_cnt > 0);
 
-		/*
-		 * We must remove and add back the device to cause the new
+		if (retry_cnt == 0) {
+			error = EINVAL;
+			break;
+		}
+
+		/* We must remove and add back the device to cause the new
 		 * MTU to take effect.  This includes tearing down, but not
 		 * deleting the channel, then bringing it back up.
 		 */
 		error = hv_rf_on_device_remove(hn_dev, HV_RF_NV_RETAIN_CHANNEL);
 		if (error) {
+			NV_LOCK(sc);
+			sc->temp_unusable = FALSE;
 			NV_UNLOCK(sc);
 			break;
 		}
 		error = hv_rf_on_device_add(hn_dev, &device_info);
 		if (error) {
+			NV_LOCK(sc);
+			sc->temp_unusable = FALSE;
 			NV_UNLOCK(sc);
 			break;
 		}
 
 		hn_ifinit_locked(sc);
 
+		NV_LOCK(sc);
+		sc->temp_unusable = FALSE;
 		NV_UNLOCK(sc);
 		break;
 	case SIOCSIFFLAGS:
-		NV_LOCK(sc);
+		do {
+                       NV_LOCK(sc);
+                       if (!sc->temp_unusable) {
+                               sc->temp_unusable = TRUE;
+                               retry_cnt = -1;
+                       }
+                       NV_UNLOCK(sc);
+                       if (retry_cnt > 0) {
+                      	        retry_cnt--;
+                        	DELAY(5 * 1000);
+                       }
+                } while (retry_cnt > 0);
+
+                if (retry_cnt == 0) {
+                       error = EINVAL;
+                       break;
+                }
+
 		if (ifp->if_flags & IFF_UP) {
 			/*
 			 * If only the state of the PROMISC flag changed,
@@ -766,21 +823,14 @@ hn_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
 			 */
 #ifdef notyet
 			/* Fixme:  Promiscuous mode? */
-			/* No promiscuous mode with Xen */
 			if (ifp->if_drv_flags & IFF_DRV_RUNNING &&
 			    ifp->if_flags & IFF_PROMISC &&
 			    !(sc->hn_if_flags & IFF_PROMISC)) {
 				/* do something here for Hyper-V */
-				;
-/*				XN_SETBIT(sc, XN_RX_MODE,		*/
-/*					  XN_RXMODE_RX_PROMISC);	*/
 			} else if (ifp->if_drv_flags & IFF_DRV_RUNNING &&
-				   !(ifp->if_flags & IFF_PROMISC) &&
-				   sc->hn_if_flags & IFF_PROMISC) {
+			    !(ifp->if_flags & IFF_PROMISC) &&
+			    sc->hn_if_flags & IFF_PROMISC) {
 				/* do something here for Hyper-V */
-				;
-/*				XN_CLRBIT(sc, XN_RX_MODE,		*/
-/*					  XN_RXMODE_RX_PROMISC);	*/
 			} else
 #endif
 				hn_ifinit_locked(sc);
@@ -789,8 +839,10 @@ hn_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
 				hn_stop(sc);
 			}
 		}
-		sc->hn_if_flags = ifp->if_flags;
+		NV_LOCK(sc);
+		sc->temp_unusable = FALSE;
 		NV_UNLOCK(sc);
+		sc->hn_if_flags = ifp->if_flags;
 		error = 0;
 		break;
 	case SIOCSIFCAP:
@@ -838,7 +890,6 @@ hn_stop(hn_softc_t *sc)
 	int ret;
 	struct hv_device *device_ctx = vmbus_get_devctx(sc->hn_dev);
 
-	NV_LOCK_ASSERT(sc);
 	ifp = sc->hn_ifp;
 
 	printf(" Closing Device ...\n");
@@ -859,6 +910,10 @@ hn_start(struct ifnet *ifp)
 
 	sc = ifp->if_softc;
 	NV_LOCK(sc);
+	if (sc->temp_unusable) {
+		NV_UNLOCK(sc);
+		return;
+	}
 	hn_start_locked(ifp);
 	NV_UNLOCK(sc);
 }
@@ -873,8 +928,6 @@ hn_ifinit_locked(hn_softc_t *sc)
 	struct hv_device *device_ctx = vmbus_get_devctx(sc->hn_dev);
 	int ret;
 
-	NV_LOCK_ASSERT(sc);
-
 	ifp = sc->hn_ifp;
 
 	if (ifp->if_drv_flags & IFF_DRV_RUNNING) {
@@ -902,7 +955,17 @@ hn_ifinit(void *xsc)
 	hn_softc_t *sc = xsc;
 
 	NV_LOCK(sc);
+	if (sc->temp_unusable) {
+		NV_UNLOCK(sc);
+		return;
+	}
+	sc->temp_unusable = TRUE;
+	NV_UNLOCK(sc);
+
 	hn_ifinit_locked(sc);
+
+	NV_LOCK(sc);
+	sc->temp_unusable = FALSE;
 	NV_UNLOCK(sc);
 }