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);
}
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
