1918 stack overflow from mac_promisc_dispatch()

Reviewed by: Robert Mustacchi <rm@fingolfin.org>
Reviewed by: Michael Speer <michael.speer@pluribusnetworks.com>
Approved by: Richard Lowe <richlowe@richlowe.net>

2 files changed, 49 insertions, 32 deletions

diff --git a/usr/src/uts/common/io/dld/dld_proto.c b/usr/src/uts/common/io/dld/dld_proto.c
index 2fba6f2a25..3ef3ce1eaf 100644
--- a/usr/src/uts/common/io/dld/dld_proto.c
+++ b/usr/src/uts/common/io/dld/dld_proto.c
@@ -20,6 +20,7 @@
  */
 /*
  * Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright 2012, Nexenta Systems, Inc. All rights reserved.
  */
 
 /*
@@ -573,7 +574,7 @@ proto_promiscon_req(dld_str_t *dsp, mblk_t *mp)
 	dl_promiscon_req_t *dlp = (dl_promiscon_req_t *)mp->b_rptr;
 	int		err = 0;
 	t_uscalar_t	dl_err;
-	uint32_t	promisc_saved;
+	uint32_t	new_flags, promisc_saved;
 	queue_t		*q = dsp->ds_wq;
 	mac_perim_handle_t	mph;
 
@@ -588,18 +589,20 @@ proto_promiscon_req(dld_str_t *dsp, mblk_t *mp)
 		goto failed;
 	}
 
-	promisc_saved = dsp->ds_promisc;
+	mac_perim_enter_by_mh(dsp->ds_mh, &mph);
+
+	new_flags = promisc_saved = dsp->ds_promisc;
 	switch (dlp->dl_level) {
 	case DL_PROMISC_SAP:
-		dsp->ds_promisc |= DLS_PROMISC_SAP;
+		new_flags |= DLS_PROMISC_SAP;
 		break;
 
 	case DL_PROMISC_MULTI:
-		dsp->ds_promisc |= DLS_PROMISC_MULTI;
+		new_flags |= DLS_PROMISC_MULTI;
 		break;
 
 	case DL_PROMISC_PHYS:
-		dsp->ds_promisc |= DLS_PROMISC_PHYS;
+		new_flags |= DLS_PROMISC_PHYS;
 		break;
 
 	default:
@@ -607,10 +610,8 @@ proto_promiscon_req(dld_str_t *dsp, mblk_t *mp)
 		goto failed;
 	}
 
-	mac_perim_enter_by_mh(dsp->ds_mh, &mph);
-
 	if ((promisc_saved == 0) && (err = dls_active_set(dsp)) != 0) {
-		dsp->ds_promisc = promisc_saved;
+		ASSERT(dsp->ds_promisc == promisc_saved);
 		dl_err = DL_SYSERR;
 		goto failed2;
 	}
@@ -618,7 +619,7 @@ proto_promiscon_req(dld_str_t *dsp, mblk_t *mp)
 	/*
 	 * Adjust channel promiscuity.
 	 */
-	err = dls_promisc(dsp, promisc_saved);
+	err = dls_promisc(dsp, new_flags);
 
 	if (err != 0) {
 		dl_err = DL_SYSERR;
@@ -648,7 +649,7 @@ proto_promiscoff_req(dld_str_t *dsp, mblk_t *mp)
 	dl_promiscoff_req_t *dlp = (dl_promiscoff_req_t *)mp->b_rptr;
 	int		err = 0;
 	t_uscalar_t	dl_err;
-	uint32_t	promisc_saved;
+	uint32_t	new_flags;
 	queue_t		*q = dsp->ds_wq;
 	mac_perim_handle_t	mph;
 
@@ -663,14 +664,16 @@ proto_promiscoff_req(dld_str_t *dsp, mblk_t *mp)
 		goto failed;
 	}
 
-	promisc_saved = dsp->ds_promisc;
+	mac_perim_enter_by_mh(dsp->ds_mh, &mph);
+
+	new_flags = dsp->ds_promisc;
 	switch (dlp->dl_level) {
 	case DL_PROMISC_SAP:
 		if (!(dsp->ds_promisc & DLS_PROMISC_SAP)) {
 			dl_err = DL_NOTENAB;
 			goto failed;
 		}
-		dsp->ds_promisc &= ~DLS_PROMISC_SAP;
+		new_flags &= ~DLS_PROMISC_SAP;
 		break;
 
 	case DL_PROMISC_MULTI:
@@ -678,7 +681,7 @@ proto_promiscoff_req(dld_str_t *dsp, mblk_t *mp)
 			dl_err = DL_NOTENAB;
 			goto failed;
 		}
-		dsp->ds_promisc &= ~DLS_PROMISC_MULTI;
+		new_flags &= ~DLS_PROMISC_MULTI;
 		break;
 
 	case DL_PROMISC_PHYS:
@@ -686,7 +689,7 @@ proto_promiscoff_req(dld_str_t *dsp, mblk_t *mp)
 			dl_err = DL_NOTENAB;
 			goto failed;
 		}
-		dsp->ds_promisc &= ~DLS_PROMISC_PHYS;
+		new_flags &= ~DLS_PROMISC_PHYS;
 		break;
 
 	default:
@@ -694,11 +697,10 @@ proto_promiscoff_req(dld_str_t *dsp, mblk_t *mp)
 		goto failed;
 	}
 
-	mac_perim_enter_by_mh(dsp->ds_mh, &mph);
 	/*
 	 * Adjust channel promiscuity.
 	 */
-	err = dls_promisc(dsp, promisc_saved);
+	err = dls_promisc(dsp, new_flags);
 
 	if (err != 0) {
 		mac_perim_exit(mph);
@@ -706,6 +708,7 @@ proto_promiscoff_req(dld_str_t *dsp, mblk_t *mp)
 		goto failed;
 	}
 
+	ASSERT(dsp->ds_promisc == new_flags);
 	if (dsp->ds_promisc == 0)
 		dls_active_clear(dsp, B_FALSE);
 
diff --git a/usr/src/uts/common/io/dls/dls.c b/usr/src/uts/common/io/dls/dls.c
index e29b1ee6ac..f90adbf27a 100644
--- a/usr/src/uts/common/io/dls/dls.c
+++ b/usr/src/uts/common/io/dls/dls.c
@@ -21,6 +21,7 @@
 /*
  * Copyright 2009 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
+ * Copyright 2012, Nexenta Systems, Inc. All rights reserved.
  */
 
 /*
@@ -82,7 +83,6 @@ dls_close(dld_str_t *dsp)
 	dls_link_t		*dlp = dsp->ds_dlp;
 	dls_multicst_addr_t	*p;
 	dls_multicst_addr_t	*nextp;
-	uint32_t		old_flags;
 
 	ASSERT(dsp->ds_datathr_cnt == 0);
 	ASSERT(MAC_PERIM_HELD(dsp->ds_mh));
@@ -119,9 +119,7 @@ dls_close(dld_str_t *dsp)
 	 * If the MAC has been set in promiscuous mode then disable it.
 	 * This needs to be done before resetting ds_rx.
 	 */
-	old_flags = dsp->ds_promisc;
-	dsp->ds_promisc = 0;
-	(void) dls_promisc(dsp, old_flags);
+	(void) dls_promisc(dsp, 0);
 
 	/*
 	 * At this point we have cutoff inbound packet flow from the mac
@@ -234,51 +232,60 @@ dls_unbind(dld_str_t *dsp)
 	dsp->ds_sap = 0;
 }
 
+/*
+ * In order to prevent promiscuous-mode processing with dsp->ds_promisc
+ * set to inaccurate values, this function sets dsp->ds_promisc with new
+ * flags.  For enabling (mac_promisc_add), the flags are set prior to the
+ * actual enabling.  For disabling (mac_promisc_remove), the flags are set
+ * after the actual disabling.
+ */
 int
-dls_promisc(dld_str_t *dsp, uint32_t old_flags)
+dls_promisc(dld_str_t *dsp, uint32_t new_flags)
 {
-	int		err = 0;
+	int err = 0;
+	uint32_t old_flags = dsp->ds_promisc;
 
 	ASSERT(MAC_PERIM_HELD(dsp->ds_mh));
-	ASSERT(!(dsp->ds_promisc & ~(DLS_PROMISC_SAP | DLS_PROMISC_MULTI |
+	ASSERT(!(new_flags & ~(DLS_PROMISC_SAP | DLS_PROMISC_MULTI |
 	    DLS_PROMISC_PHYS)));
 
-	if (old_flags == 0 && dsp->ds_promisc != 0) {
+	if (dsp->ds_promisc == 0 && new_flags != 0) {
 		/*
 		 * If only DLS_PROMISC_SAP, we don't turn on the
 		 * physical promisc mode
 		 */
+		dsp->ds_promisc = new_flags;
 		err = mac_promisc_add(dsp->ds_mch, MAC_CLIENT_PROMISC_ALL,
 		    dls_rx_promisc, dsp, &dsp->ds_mph,
-		    (dsp->ds_promisc != DLS_PROMISC_SAP) ? 0 :
+		    (new_flags != DLS_PROMISC_SAP) ? 0 :
 		    MAC_PROMISC_FLAGS_NO_PHYS);
-		if (err != 0)
+		if (err != 0) {
+			dsp->ds_promisc = old_flags;
 			return (err);
+		}
 
 		/* Remove vlan promisc handle to avoid sending dup copy up */
 		if (dsp->ds_vlan_mph != NULL) {
 			mac_promisc_remove(dsp->ds_vlan_mph);
 			dsp->ds_vlan_mph = NULL;
 		}
-	} else if (old_flags != 0 && dsp->ds_promisc == 0) {
+	} else if (dsp->ds_promisc != 0 && new_flags == 0) {
 		ASSERT(dsp->ds_mph != NULL);
 
 		mac_promisc_remove(dsp->ds_mph);
+		dsp->ds_promisc = new_flags;
 		dsp->ds_mph = NULL;
 
 		if (dsp->ds_sap == ETHERTYPE_VLAN &&
 		    dsp->ds_dlstate != DL_UNBOUND) {
-			int err;
-
 			if (dsp->ds_vlan_mph != NULL)
 				return (EINVAL);
 			err = mac_promisc_add(dsp->ds_mch,
 			    MAC_CLIENT_PROMISC_ALL, dls_rx_vlan_promisc, dsp,
 			    &dsp->ds_vlan_mph, MAC_PROMISC_FLAGS_NO_PHYS);
-			return (err);
 		}
-	} else if (old_flags == DLS_PROMISC_SAP && dsp->ds_promisc != 0 &&
-	    dsp->ds_promisc != old_flags) {
+	} else if (dsp->ds_promisc == DLS_PROMISC_SAP && new_flags != 0 &&
+	    new_flags != dsp->ds_promisc) {
 		/*
 		 * If the old flag is PROMISC_SAP, but the current flag has
 		 * changed to some new non-zero value, we need to turn the
@@ -286,8 +293,15 @@ dls_promisc(dld_str_t *dsp, uint32_t old_flags)
 		 */
 		ASSERT(dsp->ds_mph != NULL);
 		mac_promisc_remove(dsp->ds_mph);
+		/* Honors both after-remove and before-add semantics! */
+		dsp->ds_promisc = new_flags;
 		err = mac_promisc_add(dsp->ds_mch, MAC_CLIENT_PROMISC_ALL,
 		    dls_rx_promisc, dsp, &dsp->ds_mph, 0);
+		if (err != 0)
+			dsp->ds_promisc = old_flags;
+	} else {
+		/* No adding or removing, but record the new flags anyway. */
+		dsp->ds_promisc = new_flags;
 	}
 
 	return (err);