Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 8 Oct 2019 17:55:46 +0000 (UTC)
From:      Gleb Smirnoff <glebius@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r353314 - head/sys/net
Message-ID:  <201910081755.x98Htkuw055964@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: glebius
Date: Tue Oct  8 17:55:45 2019
New Revision: 353314
URL: https://svnweb.freebsd.org/changeset/base/353314

Log:
  Remove epoch assertion from if_setlladdr().  Originally this function was
  protected by IF_ADDR_LOCK(), which was a mutex, so that two simultaneous
  if_setlladdr() can't execute. Later it was switched to IF_ADDR_RLOCK(),
  likely by a mistake. Later it was switched to NET_EPOCH_ENTER(). Then I
  incorrectly added NET_EPOCH_ASSERT() here.
  
  In reality ifp->if_addr never goes away and never changes its length. So,
  doing bcopy() in it is always "safe", meaning it won't dereference a wrong
  pointer or write into someone's else memory. Of course doing two bcopy() in
  parallel would result in a mess of two addresses, but net epoch doesn't
  protect against that, neither IF_ADDR_RLOCK() did.
  
  So for now, just remove the assertion and leave for later a proper fix.
  
  Reported by:	markj

Modified:
  head/sys/net/if.c

Modified: head/sys/net/if.c
==============================================================================
--- head/sys/net/if.c	Tue Oct  8 16:59:17 2019	(r353313)
+++ head/sys/net/if.c	Tue Oct  8 17:55:45 2019	(r353314)
@@ -3822,26 +3822,18 @@ if_setlladdr(struct ifnet *ifp, const u_char *lladdr, 
 	struct sockaddr_dl *sdl;
 	struct ifaddr *ifa;
 	struct ifreq ifr;
-	int rc;
 
-	NET_EPOCH_ASSERT();
-
-	rc = 0;
 	ifa = ifp->if_addr;
-	if (ifa == NULL) {
-		rc = EINVAL;
-		goto out;
-	}
+	if (ifa == NULL)
+		return (EINVAL);
 
 	sdl = (struct sockaddr_dl *)ifa->ifa_addr;
-	if (sdl == NULL) {
-		rc = EINVAL;
-		goto out;
-	}
-	if (len != sdl->sdl_alen) {	/* don't allow length to change */
-		rc = EINVAL;
-		goto out;
-	}
+	if (sdl == NULL)
+		return (EINVAL);
+
+	if (len != sdl->sdl_alen)	/* don't allow length to change */
+		return (EINVAL);
+
 	switch (ifp->if_type) {
 	case IFT_ETHER:
 	case IFT_XETHER:
@@ -3851,8 +3843,7 @@ if_setlladdr(struct ifnet *ifp, const u_char *lladdr, 
 		bcopy(lladdr, LLADDR(sdl), len);
 		break;
 	default:
-		rc = ENODEV;
-		goto out;
+		return (ENODEV);
 	}
 
 	/*
@@ -3873,9 +3864,8 @@ if_setlladdr(struct ifnet *ifp, const u_char *lladdr, 
 		}
 	}
 	EVENTHANDLER_INVOKE(iflladdr_event, ifp);
+
 	return (0);
-out:
-	return (rc);
 }
 
 /*



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201910081755.x98Htkuw055964>