svn commit: r308580 - head/sys/net

[Ryan Stone]

Author: rstone
Date: Sat Nov 12 19:03:23 2016
New Revision: 308580
URL: https://svnweb.freebsd.org/changeset/base/308580

Log:
  Don't read if_counters with if_addr_lock held
  
  Calling into an ifnet implementation with the if_addr_lock already
  held can cause a LOR and potentially a deadlock, as ifnet
  implementations typically can take the if_addr_lock after their
  own locks during configuration.  Refactor a sysctl handler that
  was violating this to read if_counter data in a temporary buffer
  before the if_addr_lock is taken, and then copying the data
  in its final location later, when the if_addr_lock is held.
  
  PR: 194109
  Reported by: Jean-Sebastien Pedron
  MFC after: 2 weeks
  Differential Revision:	https://reviews.freebsd.org/D8498
  Reviewed by: sbruno

Modified:
  head/sys/net/rtsock.c

Modified: head/sys/net/rtsock.c
==============================================================================
--- head/sys/net/rtsock.c	Sat Nov 12 18:57:38 2016	(r308579)
+++ head/sys/net/rtsock.c	Sat Nov 12 19:03:23 2016	(r308580)
@@ -1566,8 +1566,8 @@ sysctl_dumpentry(struct radix_node *rn, 
 }
 
 static int
-sysctl_iflist_ifml(struct ifnet *ifp, struct rt_addrinfo *info,
-    struct walkarg *w, int len)
+sysctl_iflist_ifml(struct ifnet *ifp, const struct if_data *src_ifd,
+    struct rt_addrinfo *info, struct walkarg *w, int len)
 {
 	struct if_msghdrl *ifm;
 	struct if_data *ifd;
@@ -1598,14 +1598,14 @@ sysctl_iflist_ifml(struct ifnet *ifp, st
 		ifd = &ifm->ifm_data;
 	}
 
-	if_data_copy(ifp, ifd);
+	memcpy(ifd, src_ifd, sizeof(*ifd));
 
 	return (SYSCTL_OUT(w->w_req, (caddr_t)ifm, len));
 }
 
 static int
-sysctl_iflist_ifm(struct ifnet *ifp, struct rt_addrinfo *info,
-    struct walkarg *w, int len)
+sysctl_iflist_ifm(struct ifnet *ifp, const struct if_data *src_ifd,
+    struct rt_addrinfo *info, struct walkarg *w, int len)
 {
 	struct if_msghdr *ifm;
 	struct if_data *ifd;
@@ -1630,7 +1630,7 @@ sysctl_iflist_ifm(struct ifnet *ifp, str
 		ifd = &ifm->ifm_data;
 	}
 
-	if_data_copy(ifp, ifd);
+	memcpy(ifd, src_ifd, sizeof(*ifd));
 
 	return (SYSCTL_OUT(w->w_req, (caddr_t)ifm, len));
 }
@@ -1705,15 +1705,18 @@ sysctl_iflist(int af, struct walkarg *w)
 {
 	struct ifnet *ifp;
 	struct ifaddr *ifa;
+	struct if_data ifd;
 	struct rt_addrinfo info;
 	int len, error = 0;
 	struct sockaddr_storage ss;
 
 	bzero((caddr_t)&info, sizeof(info));
+	bzero(&ifd, sizeof(ifd));
 	IFNET_RLOCK_NOSLEEP();
 	TAILQ_FOREACH(ifp, &V_ifnet, if_link) {
 		if (w->w_arg && w->w_arg != ifp->if_index)
 			continue;
+		if_data_copy(ifp, &ifd);
 		IF_ADDR_RLOCK(ifp);
 		ifa = ifp->if_addr;
 		info.rti_info[RTAX_IFP] = ifa->ifa_addr;
@@ -1723,9 +1726,11 @@ sysctl_iflist(int af, struct walkarg *w)
 		info.rti_info[RTAX_IFP] = NULL;
 		if (w->w_req && w->w_tmem) {
 			if (w->w_op == NET_RT_IFLISTL)
-				error = sysctl_iflist_ifml(ifp, &info, w, len);
+				error = sysctl_iflist_ifml(ifp, &ifd, &info, w,
+				    len);
 			else
-				error = sysctl_iflist_ifm(ifp, &info, w, len);
+				error = sysctl_iflist_ifm(ifp, &ifd, &info, w,
+				    len);
 			if (error)
 				goto done;
 		}