svn commit: r194610 - head/sys/netipx

Robert Watson rwatson at FreeBSD.org
Sun Jun 21 21:42:30 UTC 2009


Author: rwatson
Date: Sun Jun 21 21:42:29 2009
New Revision: 194610
URL: http://svn.freebsd.org/changeset/base/194610

Log:
  Add ipx_ifaddr locking to ipx_control(), which should close most
  remaining potential races in ifconfig's management of IPX addresses.
  
  This is largely accomplished by dropping a global write lock for the
  IPX address list over the body of in_control(), although there are
  some places we bump the refcount on an ifaddr of interest while
  calling out to the routing code or link layer code, which might
  require revisiting.
  
  Annotate one as a potential race if two simultaneous delete ioctls
  are issued for the same IPX addresses at once.
  
  MFC after:	3 weeks

Modified:
  head/sys/netipx/ipx.c

Modified: head/sys/netipx/ipx.c
==============================================================================
--- head/sys/netipx/ipx.c	Sun Jun 21 21:38:12 2009	(r194609)
+++ head/sys/netipx/ipx.c	Sun Jun 21 21:42:29 2009	(r194610)
@@ -1,6 +1,8 @@
 /*-
  * Copyright (c) 1984, 1985, 1986, 1987, 1993
- *	The Regents of the University of California.  All rights reserved.
+ *	The Regents of the University of California.
+ * Copyright (c) 2009 Robert N. M. Watson
+ * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -109,32 +111,44 @@ ipx_control(struct socket *so, u_long cm
 	 */
 	if (ifp == NULL)
 		return (EADDRNOTAVAIL);
+
+	IPX_IFADDR_WLOCK();
 	for (ia = ipx_ifaddr; ia != NULL; ia = ia->ia_next)
 		if (ia->ia_ifp == ifp)
 			break;
 
 	switch (cmd) {
 	case SIOCGIFADDR:
-		if (ia == NULL)
-			return (EADDRNOTAVAIL);
+		if (ia == NULL) {
+			error = EADDRNOTAVAIL;
+			goto out;
+		}
 		*(struct sockaddr_ipx *)&ifr->ifr_addr = ia->ia_addr;
-		return (0);
+		goto out;
 
 	case SIOCGIFBRDADDR:
-		if (ia == NULL)
-			return (EADDRNOTAVAIL);
-		if ((ifp->if_flags & IFF_BROADCAST) == 0)
-			return (EINVAL);
+		if (ia == NULL) {
+			error = EADDRNOTAVAIL;
+			goto out;
+		}
+		if ((ifp->if_flags & IFF_BROADCAST) == 0) {
+			error = EINVAL;
+			goto out;
+		}
 		*(struct sockaddr_ipx *)&ifr->ifr_dstaddr = ia->ia_broadaddr;
-		return (0);
+		goto out;
 
 	case SIOCGIFDSTADDR:
-		if (ia == NULL)
-			return (EADDRNOTAVAIL);
-		if ((ifp->if_flags & IFF_POINTOPOINT) == 0)
-			return (EINVAL);
+		if (ia == NULL) {
+			error = EADDRNOTAVAIL;
+			goto out;
+		}
+		if ((ifp->if_flags & IFF_POINTOPOINT) == 0) {
+			error = EINVAL;
+			goto out;
+		}
 		*(struct sockaddr_ipx *)&ifr->ifr_dstaddr = ia->ia_dstaddr;
-		return (0);
+		goto out;
 	}
 
 	switch (cmd) {
@@ -143,7 +157,7 @@ ipx_control(struct socket *so, u_long cm
 		priv = (cmd == SIOCAIFADDR) ? PRIV_NET_ADDIFADDR :
 		    PRIV_NET_DELIFADDR;
 		if (td && (error = priv_check(td, priv)) != 0)
-			return (error);
+			goto out;
 		if (ifra->ifra_addr.sipx_family == AF_IPX)
 		    for (oia = ia; ia != NULL; ia = ia->ia_next) {
 			if (ia->ia_ifp == ifp  &&
@@ -151,20 +165,24 @@ ipx_control(struct socket *so, u_long cm
 				  ifra->ifra_addr.sipx_addr))
 			    break;
 		    }
-		if (cmd == SIOCDIFADDR && ia == NULL)
-			return (EADDRNOTAVAIL);
+		if (cmd == SIOCDIFADDR && ia == NULL) {
+			error = EADDRNOTAVAIL;
+			goto out;
+		}
 		/* FALLTHROUGH */
 
 	case SIOCSIFADDR:
 	case SIOCSIFDSTADDR:
 		if (td && (error = priv_check(td, PRIV_NET_SETLLADDR)) != 0)
-			return (error);
+			goto out;
 		if (ia == NULL) {
 			oia = (struct ipx_ifaddr *)
 				malloc(sizeof(*ia), M_IFADDR,
-				M_WAITOK | M_ZERO);
-			if (oia == NULL)
-				return (ENOBUFS);
+				M_NOWAIT | M_ZERO);
+			if (oia == NULL) {
+				error = ENOBUFS;
+				goto out;
+			}
 			if ((ia = ipx_ifaddr) != NULL) {
 				for ( ; ia->ia_next != NULL; ia = ia->ia_next)
 					;
@@ -193,32 +211,48 @@ ipx_control(struct socket *so, u_long cm
 
 	default:
 		if (td && (error = priv_check(td, PRIV_NET_HWIOCTL)) != 0)
-			return (error);
+			goto out;
 	}
 
 	switch (cmd) {
 	case SIOCSIFDSTADDR:
-		if ((ifp->if_flags & IFF_POINTOPOINT) == 0)
-			return (EINVAL);
+		if ((ifp->if_flags & IFF_POINTOPOINT) == 0) {
+			error = EINVAL;
+			goto out;
+		}
 		if (ia->ia_flags & IFA_ROUTE) {
 			rtinit(&(ia->ia_ifa), (int)RTM_DELETE, RTF_HOST);
 			ia->ia_flags &= ~IFA_ROUTE;
 		}
+		ifa_ref(&ia->ia_ifa);
+		IPX_IFADDR_WUNLOCK();
 		if (ifp->if_ioctl) {
 			error = (*ifp->if_ioctl)(ifp, SIOCSIFDSTADDR,
 			    (void *)ia);
-			if (error)
+			if (error) {
+				ifa_free(&ia->ia_ifa);
 				return (error);
+			}
 		}
 		*(struct sockaddr *)&ia->ia_dstaddr = ifr->ifr_dstaddr;
+		ifa_free(&ia->ia_ifa);
 		return (0);
 
 	case SIOCSIFADDR:
-		return (ipx_ifinit(ifp, ia,
-				(struct sockaddr_ipx *)&ifr->ifr_addr, 1));
+		ifa_ref(&ia->ia_ifa);
+		IPX_IFADDR_WUNLOCK();
+		error = ipx_ifinit(ifp, ia,
+		    (struct sockaddr_ipx *)&ifr->ifr_addr, 1);
+		ifa_free(&ia->ia_ifa);
+		return (error);
 
 	case SIOCDIFADDR:
+		/* XXXRW: Potential race here while ipx_ifaddr_rw is dropped. */
+		ifa_ref(&ia->ia_ifa);
+		IPX_IFADDR_WUNLOCK();
 		ipx_ifscrub(ifp, ia);
+		IPX_IFADDR_WLOCK();
+		ifa_free(&ia->ia_ifa);
 		ifa = (struct ifaddr *)ia;
 		IF_ADDR_LOCK(ifp);
 		TAILQ_REMOVE(&ifp->if_addrhead, ifa, ifa_link);
@@ -236,6 +270,7 @@ ipx_control(struct socket *so, u_long cm
 				printf("Didn't unlink ipxifadr from list\n");
 		}
 		ifa_free(&oia->ia_ifa);
+		IPX_IFADDR_WUNLOCK();
 		return (0);
 
 	case SIOCAIFADDR:
@@ -249,6 +284,8 @@ ipx_control(struct socket *so, u_long cm
 					 ia->ia_addr.sipx_addr))
 				hostIsNew = 0;
 		}
+		ifa_ref(&ia->ia_ifa);
+		IPX_IFADDR_WUNLOCK();
 		if ((ifp->if_flags & IFF_POINTOPOINT) &&
 		    (ifra->ifra_dstaddr.sipx_family == AF_IPX)) {
 			if (hostIsNew == 0)
@@ -259,13 +296,19 @@ ipx_control(struct socket *so, u_long cm
 		if (ifra->ifra_addr.sipx_family == AF_IPX &&
 					    (hostIsNew || dstIsNew))
 			error = ipx_ifinit(ifp, ia, &ifra->ifra_addr, 0);
+		ifa_free(&ia->ia_ifa);
 		return (error);
 
 	default:
+		IPX_IFADDR_WUNLOCK();
 		if (ifp->if_ioctl == NULL)
 			return (EOPNOTSUPP);
 		return ((*ifp->if_ioctl)(ifp, cmd, data));
 	}
+
+out:
+	IPX_IFADDR_WUNLOCK();
+	return (error);
 }
 
 /*


More information about the svn-src-all mailing list