Date: Wed, 24 Jun 2009 16:52:23 +0000 (UTC) From: Robert Watson <rwatson@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r194857 - head/sys/netipx Message-ID: <200906241652.n5OGqNcm021026@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: rwatson Date: Wed Jun 24 16:52:23 2009 New Revision: 194857 URL: http://svn.freebsd.org/changeset/base/194857 Log: Rework locking and reference counting in ipx_control to be consistent with the model used in in_control(). MFC after: 6 weeks Modified: head/sys/netipx/ipx.c Modified: head/sys/netipx/ipx.c ============================================================================== --- head/sys/netipx/ipx.c Wed Jun 24 16:37:44 2009 (r194856) +++ head/sys/netipx/ipx.c Wed Jun 24 16:52:23 2009 (r194857) @@ -100,11 +100,10 @@ ipx_control(struct socket *so, u_long cm { struct ifreq *ifr = (struct ifreq *)data; struct ipx_aliasreq *ifra = (struct ipx_aliasreq *)data; - struct ipx_ifaddr *ia; + struct ipx_ifaddr *ia, *ia_temp, *oia; struct ifaddr *ifa; - struct ipx_ifaddr *oia; int dstIsNew, hostIsNew; - int error = 0, priv; + int error, priv; /* * Find address for this interface, if it exists. @@ -112,11 +111,15 @@ ipx_control(struct socket *so, u_long cm if (ifp == NULL) return (EADDRNOTAVAIL); - IPX_IFADDR_WLOCK(); + IPX_IFADDR_RLOCK(); for (ia = ipx_ifaddr; ia != NULL; ia = ia->ia_next) if (ia->ia_ifp == ifp) break; + if (ia != NULL) + ifa_ref(&ia->ia_ifa); + IPX_IFADDR_RUNLOCK(); + error = 0; switch (cmd) { case SIOCGIFADDR: if (ia == NULL) { @@ -158,13 +161,21 @@ ipx_control(struct socket *so, u_long cm PRIV_NET_DELIFADDR; if (td && (error = priv_check(td, priv)) != 0) goto out; - if (ifra->ifra_addr.sipx_family == AF_IPX) - for (oia = ia; ia != NULL; ia = ia->ia_next) { - if (ia->ia_ifp == ifp && - ipx_neteq(ia->ia_addr.sipx_addr, - ifra->ifra_addr.sipx_addr)) - break; - } + + IPX_IFADDR_RLOCK(); + if (ifra->ifra_addr.sipx_family == AF_IPX) { + for (oia = ia; ia != NULL; ia = ia->ia_next) { + if (ia->ia_ifp == ifp && + ipx_neteq(ia->ia_addr.sipx_addr, + ifra->ifra_addr.sipx_addr)) + break; + } + if (oia != NULL && oia != ia) + ifa_free(&oia->ia_ifa); + if (ia != NULL && oia != ia) + ifa_ref(&ia->ia_ifa); + } + IPX_IFADDR_RUNLOCK(); if (cmd == SIOCDIFADDR && ia == NULL) { error = EADDRNOTAVAIL; goto out; @@ -176,20 +187,11 @@ ipx_control(struct socket *so, u_long cm if (td && (error = priv_check(td, PRIV_NET_SETLLADDR)) != 0) goto out; if (ia == NULL) { - oia = (struct ipx_ifaddr *) - malloc(sizeof(*ia), M_IFADDR, - M_NOWAIT | M_ZERO); - if (oia == NULL) { + ia = malloc(sizeof(*ia), M_IFADDR, M_NOWAIT | M_ZERO); + if (ia == NULL) { error = ENOBUFS; goto out; } - if ((ia = ipx_ifaddr) != NULL) { - for ( ; ia->ia_next != NULL; ia = ia->ia_next) - ; - ia->ia_next = oia; - } else - ipx_ifaddr = oia; - ia = oia; ifa = (struct ifaddr *)ia; ifa_init(ifa); ia->ia_ifp = ifp; @@ -203,6 +205,18 @@ ipx_control(struct socket *so, u_long cm ia->ia_broadaddr.sipx_addr.x_host = ipx_broadhost; } + ifa_ref(&ia->ia_ifa); /* ipx_ifaddr */ + IPX_IFADDR_WLOCK(); + if ((ia_temp = ipx_ifaddr) != NULL) { + for (; ia_temp->ia_next != NULL; + ia_temp = ia_temp->ia_next) + ; + ia_temp->ia_next = ia; + } else + ipx_ifaddr = ia; + IPX_IFADDR_WUNLOCK(); + + ifa_ref(&ia->ia_ifa); /* if_addrhead */ IF_ADDR_LOCK(ifp); TAILQ_INSERT_TAIL(&ifp->if_addrhead, ifa, ifa_link); IF_ADDR_UNLOCK(ifp); @@ -224,54 +238,43 @@ ipx_control(struct socket *so, u_long cm 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) { - ifa_free(&ia->ia_ifa); - return (error); - } + if (error) + goto out; } *(struct sockaddr *)&ia->ia_dstaddr = ifr->ifr_dstaddr; - ifa_free(&ia->ia_ifa); - return (0); + goto out; case SIOCSIFADDR: - 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); + goto out; 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); IF_ADDR_UNLOCK(ifp); - oia = ia; - if (oia == (ia = ipx_ifaddr)) { + ifa_free(ifa); /* if_addrhead */ + + IPX_IFADDR_WLOCK(); + if (ia == (ia_temp = ipx_ifaddr)) { ipx_ifaddr = ia->ia_next; } else { - while (ia->ia_next && (ia->ia_next != oia)) { - ia = ia->ia_next; - } - if (ia->ia_next) - ia->ia_next = oia->ia_next; + while (ia_temp->ia_next && (ia_temp->ia_next != ia)) + ia_temp = ia_temp->ia_next; + if (ia_temp->ia_next) + ia_temp->ia_next = ia->ia_next; else - printf("Didn't unlink ipxifadr from list\n"); + panic("Didn't unlink ipxifadr from list\n"); } - ifa_free(&oia->ia_ifa); IPX_IFADDR_WUNLOCK(); - return (0); + ifa_free(&ia->ia_ifa); /* ipx_ifaddr */ + goto out; case SIOCAIFADDR: dstIsNew = 0; @@ -284,8 +287,6 @@ 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) @@ -296,18 +297,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); + goto out; default: - IPX_IFADDR_WUNLOCK(); - if (ifp->if_ioctl == NULL) - return (EOPNOTSUPP); - return ((*ifp->if_ioctl)(ifp, cmd, data)); + if (ifp->if_ioctl == NULL) { + error = EOPNOTSUPP; + goto out; + } + error = ((*ifp->if_ioctl)(ifp, cmd, data)); } out: - IPX_IFADDR_WUNLOCK(); + if (ia != NULL) + ifa_free(&ia->ia_ifa); return (error); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200906241652.n5OGqNcm021026>