Add a global rwlock, at_ifaddr_rw, to protect the global netatalk

address lists, at_ifaddr_list.  Acquire the lock, and use ifaddr
refcounts where necessary, to close most known address-related
races in netatalk.

Annotate one potential race in at_control() where we acquire an
ifaddr reference, drop the global lock, and scrub the address from
the ifnet before re-acquiring the global lock, which could allow
for a writer-writer race.

MFC after:	3 weeks

7 files changed, 143 insertions, 76 deletions

diff --git a/sys/netatalk/COPYRIGHT b/sys/netatalk/COPYRIGHT
index f47cdd1..e8445a1 100644
--- a/sys/netatalk/COPYRIGHT
+++ b/sys/netatalk/COPYRIGHT
@@ -1,5 +1,5 @@
 /*-
- * Copyright (c) 2004-2005 Robert N. M. Watson
+ * Copyright (c) 2004-2009 Robert N. M. Watson
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
diff --git a/sys/netatalk/aarp.c b/sys/netatalk/aarp.c
index d682230..203a0d4 100644
--- a/sys/netatalk/aarp.c
+++ b/sys/netatalk/aarp.c
@@ -1,5 +1,5 @@
 /*-
- * Copyright (c) 2004-2005 Robert N. M. Watson
+ * Copyright (c) 2004-2009 Robert N. M. Watson
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -149,6 +149,8 @@ at_ifawithnet(struct sockaddr_at  *sat)
 	struct at_ifaddr *aa;
 	struct sockaddr_at *sat2;
 
+	AT_IFADDR_LOCK_ASSERT();
+
 	for (aa = at_ifaddr_list; aa != NULL; aa = aa->aa_next) {
 		sat2 = &(aa->aa_addr);
 		if (sat2->sat_addr.s_net == sat->sat_addr.s_net)
@@ -199,7 +201,9 @@ aarpwhohas(struct ifnet *ifp, struct sockaddr_at *sat)
 	 * same address as we're looking for.  If the net is phase 2,
 	 * generate an 802.2 and SNAP header.
 	 */
+	AT_IFADDR_RLOCK();
 	if ((aa = at_ifawithnet(sat)) == NULL) {
+		AT_IFADDR_RUNLOCK();
 		m_freem(m);
 		return;
 	}
@@ -212,8 +216,10 @@ aarpwhohas(struct ifnet *ifp, struct sockaddr_at *sat)
 		eh->ether_type = htons(sizeof(struct llc) +
 		    sizeof(struct ether_aarp));
 		M_PREPEND(m, sizeof(struct llc), M_DONTWAIT);
-		if (m == NULL)
+		if (m == NULL) {
+			AT_IFADDR_RUNLOCK();
 			return;
+		}
 		llc = mtod(m, struct llc *);
 		llc->llc_dsap = llc->llc_ssap = LLC_SNAP_LSAP;
 		llc->llc_control = LLC_UI;
@@ -238,6 +244,7 @@ aarpwhohas(struct ifnet *ifp, struct sockaddr_at *sat)
 	printf("aarp: sending request for %u.%u\n",
 	    ntohs(AA_SAT(aa)->sat_addr.s_net), AA_SAT(aa)->sat_addr.s_node);
 #endif /* NETATALKDEBUG */
+	AT_IFADDR_RUNLOCK();
 
 	sa.sa_len = sizeof(struct sockaddr);
 	sa.sa_family = AF_UNSPEC;
@@ -251,9 +258,11 @@ aarpresolve(struct ifnet *ifp, struct mbuf *m, struct sockaddr_at *destsat,
 	struct at_ifaddr *aa;
 	struct aarptab *aat;
 
+	AT_IFADDR_RLOCK();
 	if (at_broadcast(destsat)) {
 		m->m_flags |= M_BCAST;
 		if ((aa = at_ifawithnet(destsat)) == NULL)  {
+			AT_IFADDR_RUNLOCK();
 			m_freem(m);
 			return (0);
 		}
@@ -263,8 +272,10 @@ aarpresolve(struct ifnet *ifp, struct mbuf *m, struct sockaddr_at *destsat,
 		else
 			bcopy(ifp->if_broadcastaddr, (caddr_t)desten,
 			    sizeof(ifp->if_addrlen));
+		AT_IFADDR_RUNLOCK();
 		return (1);
 	}
+	AT_IFADDR_RUNLOCK();
 
 	AARPTAB_LOCK();
 	AARPTAB_LOOK(aat, destsat->sat_addr);
@@ -368,10 +379,14 @@ at_aarpinput(struct ifnet *ifp, struct mbuf *m)
 		sat.sat_len = sizeof(struct sockaddr_at);
 		sat.sat_family = AF_APPLETALK;
 		sat.sat_addr.s_net = net;
+		AT_IFADDR_RLOCK();
 		if ((aa = at_ifawithnet(&sat)) == NULL) {
+			AT_IFADDR_RUNLOCK();
 			m_freem(m);
 			return;
 		}
+		ifa_ref(&aa->aa_ifa);
+		AT_IFADDR_RUNLOCK();
 		bcopy(ea->aarp_spnet, &spa.s_net, sizeof(spa.s_net));
 		bcopy(ea->aarp_tpnet, &tpa.s_net, sizeof(tpa.s_net));
 	} else {
@@ -379,6 +394,7 @@ at_aarpinput(struct ifnet *ifp, struct mbuf *m)
 		 * Since we don't know the net, we just look for the first
 		 * phase 1 address on the interface.
 		 */
+		IF_ADDR_LOCK(ifp);
 		for (aa = (struct at_ifaddr *)TAILQ_FIRST(&ifp->if_addrhead);
 		    aa;
 		    aa = (struct at_ifaddr *)aa->aa_ifa.ifa_link.tqe_next) {
@@ -388,9 +404,12 @@ at_aarpinput(struct ifnet *ifp, struct mbuf *m)
 			}
 		}
 		if (aa == NULL) {
+			IF_ADDR_UNLOCK(ifp);
 			m_freem(m);
 			return;
 		}
+		ifa_ref(&aa->aa_ifa);
+		IF_ADDR_UNLOCK(ifp);
 		tpa.s_net = spa.s_net = AA_SAT(aa)->sat_addr.s_net;
 	}
 
@@ -411,6 +430,7 @@ at_aarpinput(struct ifnet *ifp, struct mbuf *m)
 	    		 */
 			callout_stop(&aa->aa_callout);
 			wakeup(aa);
+			ifa_free(&aa->aa_ifa);
 			m_freem(m);
 			return;
 		} else if (op != AARPOP_PROBE) {
@@ -419,6 +439,7 @@ at_aarpinput(struct ifnet *ifp, struct mbuf *m)
 			 * means that someone's saying they have the same
 			 * source address as the one we're using.  Get upset.
 			 */
+			ifa_free(&aa->aa_ifa);
 			log(LOG_ERR,
 			    "aarp: duplicate AT address!! %x:%x:%x:%x:%x:%x\n",
 			    ea->aarp_sha[0], ea->aarp_sha[1],
@@ -440,6 +461,7 @@ at_aarpinput(struct ifnet *ifp, struct mbuf *m)
 			 */
 			aarptfree(aat);
 			AARPTAB_UNLOCK();
+			ifa_free(&aa->aa_ifa);
 			m_freem(m);
 			return;
 		}
@@ -473,6 +495,7 @@ at_aarpinput(struct ifnet *ifp, struct mbuf *m)
 	 */
 	if (tpa.s_net != ma.s_net || tpa.s_node != ma.s_node ||
 	    op == AARPOP_RESPONSE || (aa->aa_flags & AFA_PROBING)) {
+		ifa_free(&aa->aa_ifa);
 		m_freem(m);
 		return;
 	}
@@ -490,8 +513,10 @@ at_aarpinput(struct ifnet *ifp, struct mbuf *m)
 		eh->ether_type = htons(sizeof(struct llc) +
 		    sizeof(struct ether_aarp));
 		M_PREPEND(m, sizeof(struct llc), M_DONTWAIT);
-		if (m == NULL)
+		if (m == NULL) {
+			ifa_free(&aa->aa_ifa);
 			return;
+		}
 		llc = mtod(m, struct llc *);
 		llc->llc_dsap = llc->llc_ssap = LLC_SNAP_LSAP;
 		llc->llc_control = LLC_UI;
@@ -504,6 +529,7 @@ at_aarpinput(struct ifnet *ifp, struct mbuf *m)
 		bcopy(&ma.s_net, ea->aarp_spnet, sizeof(ea->aarp_spnet));
 	} else
 		eh->ether_type = htons(ETHERTYPE_AARP);
+	ifa_free(&aa->aa_ifa);
 
 	ea->aarp_tpnode = ea->aarp_spnode;
 	ea->aarp_spnode = ma.s_node;
@@ -602,11 +628,14 @@ aarpprobe(void *arg)
 		return;
 	} else
 		callout_reset(&aa->aa_callout, hz / 5, aarpprobe, ifp);
+	ifa_ref(&aa->aa_ifa);
 	AARPTAB_UNLOCK();
 
 	m = m_gethdr(M_DONTWAIT, MT_DATA);
-	if (m == NULL)
+	if (m == NULL) {
+		ifa_free(&aa->aa_ifa);
 		return;
+	}
 #ifdef MAC
 	mac_netatalk_aarp_send(ifp, m);
 #endif
@@ -657,6 +686,7 @@ aarpprobe(void *arg)
 	printf("aarp: sending probe for %u.%u\n",
 	    ntohs(AA_SAT(aa)->sat_addr.s_net), AA_SAT(aa)->sat_addr.s_node);
 #endif /* NETATALKDEBUG */
+	ifa_free(&aa->aa_ifa);
 
 	sa.sa_len = sizeof(struct sockaddr);
 	sa.sa_family = AF_UNSPEC;
diff --git a/sys/netatalk/at_control.c b/sys/netatalk/at_control.c
index 56b750f..ae38bcc 100644
--- a/sys/netatalk/at_control.c
+++ b/sys/netatalk/at_control.c
@@ -1,5 +1,6 @@
 /*-
  * Copyright (c) 1990,1991 Regents of The University of Michigan.
+ * Copyright (c) 2009 Robert N. M. Watson
  * All Rights Reserved.
  *
  * Permission to use, copy, modify, and distribute this software and
@@ -22,16 +23,19 @@
  *	Ann Arbor, Michigan
  *	+1-313-764-2278
  *	netatalk@umich.edu
- *
- * $FreeBSD$
  */
 
+#include <sys/cdefs.h>
+__FBSDID("$FreeBSD$");
+
 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/sockio.h>
+#include <sys/lock.h>
 #include <sys/malloc.h>
 #include <sys/kernel.h>
 #include <sys/priv.h>
+#include <sys/rwlock.h>
 #include <sys/socket.h>
 #include <net/if.h>
 #include <net/route.h>
@@ -43,7 +47,10 @@
 #include <netatalk/at_var.h>
 #include <netatalk/at_extern.h>
 
-struct at_ifaddr *at_ifaddr_list;
+struct rwlock		 at_ifaddr_rw;
+struct at_ifaddr	*at_ifaddr_list;
+
+RW_SYSINIT(at_ifaddr_rw, &at_ifaddr_rw, "at_ifaddr_rw");
 
 static int aa_dorangeroute(struct ifaddr *ifa, u_int first, u_int last,
 	    int cmd);
@@ -75,10 +82,12 @@ at_control(struct socket *so, u_long cmd, caddr_t data, struct ifnet *ifp,
 	struct at_ifaddr *aa0;
 	struct at_ifaddr *aa = NULL;
 	struct ifaddr *ifa, *ifa0;
+	int error;
 
 	/*
 	 * If we have an ifp, then find the matching at_ifaddr if it exists
 	 */
+	AT_IFADDR_WLOCK();
 	if (ifp != NULL) {
 		for (aa = at_ifaddr_list; aa != NULL; aa = aa->aa_next) {
 			if (aa->aa_ifp == ifp)
@@ -112,8 +121,10 @@ at_control(struct socket *so, u_long cmd, caddr_t data, struct ifnet *ifp,
 		 * If we a retrying to delete an addres but didn't find such,
 		 * then rewurn with an error
 		 */
-		if (cmd == SIOCDIFADDR && aa == NULL)
+		if (cmd == SIOCDIFADDR && aa == NULL) {
+			AT_IFADDR_WUNLOCK();
 			return (EADDRNOTAVAIL);
+		}
 		/*FALLTHROUGH*/
 
 	case SIOCSIFADDR:
@@ -122,8 +133,10 @@ at_control(struct socket *so, u_long cmd, caddr_t data, struct ifnet *ifp,
 		 *
 		 * XXXRW: Layering?
 		 */
-		if (priv_check(td, PRIV_NET_ADDIFADDR))
+		if (priv_check(td, PRIV_NET_ADDIFADDR)) {
+			AT_IFADDR_WUNLOCK();
 			return (EPERM);
+		}
 
 		sat = satosat(&ifr->ifr_addr);
 		nr = (struct netrange *)sat->sat_zero;
@@ -160,7 +173,11 @@ at_control(struct socket *so, u_long cmd, caddr_t data, struct ifnet *ifp,
 		 */
 		if (aa == NULL) {
 			aa0 = malloc(sizeof(struct at_ifaddr), M_IFADDR,
-			    M_WAITOK | M_ZERO);
+			    M_NOWAIT | M_ZERO);
+			if (aa0 == NULL) {
+				AT_IFADDR_WUNLOCK();
+				return (ENOBUFS);
+			}
 			callout_init(&aa0->aa_callout, CALLOUT_MPSAFE);
 			if ((aa = at_ifaddr_list) != NULL) {
 				/*
@@ -219,8 +236,15 @@ at_control(struct socket *so, u_long cmd, caddr_t data, struct ifnet *ifp,
 			/*
 			 * If we DID find one then we clobber any routes
 			 * dependent on it..
+			 *
+			 * XXXRW: While we ref the ifaddr, there are
+			 * potential races here still.
 			 */
+			ifa_ref(&aa->aa_ifa);
+			AT_IFADDR_WUNLOCK();
 			at_scrub(ifp, aa);
+			AT_IFADDR_WLOCK();
+			ifa_free(&aa->aa_ifa);
 		}
 		break;
 
@@ -248,8 +272,10 @@ at_control(struct socket *so, u_long cmd, caddr_t data, struct ifnet *ifp,
 			}
 		}
 
-		if (aa == NULL)
+		if (aa == NULL) {
+			AT_IFADDR_WUNLOCK();
 			return (EADDRNOTAVAIL);
+		}
 		break;
 	}
 
@@ -275,25 +301,31 @@ at_control(struct socket *so, u_long cmd, caddr_t data, struct ifnet *ifp,
 		    aa->aa_firstnet;
 		((struct netrange *)&sat->sat_zero)->nr_lastnet =
 		    aa->aa_lastnet;
+		AT_IFADDR_WUNLOCK();
 		break;
 
 	case SIOCSIFADDR:
-		return (at_ifinit(ifp, aa,
-		    (struct sockaddr_at *)&ifr->ifr_addr));
+		ifa_ref(&aa->aa_ifa);
+		AT_IFADDR_WUNLOCK();
+		error = at_ifinit(ifp, aa,
+		    (struct sockaddr_at *)&ifr->ifr_addr);
+		ifa_free(&aa->aa_ifa);
+		return (error);
 
 	case SIOCAIFADDR:
-		if (sateqaddr(&ifra->ifra_addr, &aa->aa_addr))
+		if (sateqaddr(&ifra->ifra_addr, &aa->aa_addr)) {
+			AT_IFADDR_WUNLOCK();
 			return (0);
-		return (at_ifinit(ifp, aa,
-		    (struct sockaddr_at *)&ifr->ifr_addr));
+		}
+		ifa_ref(&aa->aa_ifa);
+		AT_IFADDR_WUNLOCK();
+		error = at_ifinit(ifp, aa,
+		    (struct sockaddr_at *)&ifr->ifr_addr);
+		ifa_free(&aa->aa_ifa);
+		return (error);
 
 	case SIOCDIFADDR:
 		/*
-		 * scrub all routes.. didn't we just DO this? XXX yes, del it
-		 */
-		at_scrub(ifp, aa);
-
-		/*
 		 * remove the ifaddr from the interface
 		 */
 		ifa0 = (struct ifaddr *)aa;
@@ -320,6 +352,7 @@ at_control(struct socket *so, u_long cmd, caddr_t data, struct ifnet *ifp,
 			else
 				panic("at_control");
 		}
+		AT_IFADDR_WUNLOCK();
 
 		/*
 		 * Now reclaim the reference.
@@ -328,6 +361,7 @@ at_control(struct socket *so, u_long cmd, caddr_t data, struct ifnet *ifp,
 		break;
 
 	default:
+		AT_IFADDR_WUNLOCK();
 		if (ifp == NULL || ifp->if_ioctl == NULL)
 			return (EOPNOTSUPP);
 		return ((*ifp->if_ioctl)(ifp, cmd, data));
@@ -673,6 +707,8 @@ at_broadcast(struct sockaddr_at *sat)
 {
 	struct at_ifaddr *aa;
 
+	AT_IFADDR_LOCK_ASSERT();
+
 	/*
 	 * If the node is not right, it can't be a broadcast 
 	 */
@@ -807,36 +843,6 @@ aa_dosingleroute(struct ifaddr *ifa, struct at_addr *at_addr,
 	    (struct sockaddr *) &mask, flags, NULL));
 }
 
-#if 0
-
-static void
-aa_clean(void)
-{
-	struct at_ifaddr *aa;
-	struct ifaddr *ifa;
-	struct ifnet *ifp;
-
-	while ((aa = at_ifaddr_list) != NULL) {
-		ifp = aa->aa_ifp;
-		at_scrub(ifp, aa);
-		at_ifaddr_list = aa->aa_next;
-		if ((ifa = ifp->if_addrlist) == (struct ifaddr *)aa)
-			ifp->if_addrlist = ifa->ifa_next;
-		else {
-			while (ifa->ifa_next &&
-			    (ifa->ifa_next != (struct ifaddr *)aa))
-				ifa = ifa->ifa_next;
-			if (ifa->ifa_next)
-				ifa->ifa_next =
-				    ((struct ifaddr *)aa)->ifa_next;
-			else
-				panic("at_entry");
-		}
-	}
-}
-
-#endif
-
 static int
 aa_claim_addr(struct ifaddr *ifa, struct sockaddr *gw0)
 {
diff --git a/sys/netatalk/at_var.h b/sys/netatalk/at_var.h
index 0ad1334..3ab73a2 100644
--- a/sys/netatalk/at_var.h
+++ b/sys/netatalk/at_var.h
@@ -61,7 +61,15 @@ struct at_aliasreq {
 #define	AFA_PHASE2	0x0004
 
 #ifdef _KERNEL
+extern struct rwlock	 at_ifaddr_rw;
 extern struct at_ifaddr	*at_ifaddr_list;
+
+#define	AT_IFADDR_LOCK_INIT()	rw_init(&at_ifaddr_rw, "at_ifaddr_rw")
+#define	AT_IFADDR_LOCK_ASSERT()	rw_assert(&at_ifaddr_rw, RA_LOCKED)
+#define	AT_IFADDR_RLOCK()	rw_rlock(&at_ifaddr_rw)
+#define	AT_IFADDR_RUNLOCK()	rw_runlock(&at_ifaddr_rw)
+#define	AT_IFADDR_WLOCK()	rw_wlock(&at_ifaddr_rw)
+#define	AT_IFADDR_WUNLOCK()	rw_wunlock(&at_ifaddr_rw)
 #endif
 
 #endif /* _NETATALK_AT_VAR_H_ */
diff --git a/sys/netatalk/ddp_input.c b/sys/netatalk/ddp_input.c
index 2e82a2a..49a1908 100644
--- a/sys/netatalk/ddp_input.c
+++ b/sys/netatalk/ddp_input.c
@@ -1,5 +1,5 @@
 /*-
- * Copyright (c) 2004 Robert N. M. Watson
+ * Copyright (c) 2004-2009 Robert N. M. Watson
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -161,6 +161,7 @@ ddp_input(struct mbuf *m, struct ifnet *ifp, struct elaphdr *elh, int phase)
 		 * Make sure that we point to the phase1 ifaddr info and that
 		 * it's valid for this packet.
 		 */
+		AT_IFADDR_RLOCK();
 		for (aa = at_ifaddr_list; aa != NULL; aa = aa->aa_next) {
 			if ((aa->aa_ifp == ifp)
 			    && ((aa->aa_flags & AFA_PHASE2) == 0)
@@ -173,6 +174,7 @@ ddp_input(struct mbuf *m, struct ifnet *ifp, struct elaphdr *elh, int phase)
 		 * maybe we got a broadcast not meant for us.. ditch it.
 		 */
 		if (aa == NULL) {
+			AT_IFADDR_RUNLOCK();
 			m_freem(m);
 			return;
 		}
@@ -187,6 +189,7 @@ ddp_input(struct mbuf *m, struct ifnet *ifp, struct elaphdr *elh, int phase)
 
 		if (m->m_len < sizeof(struct ddpehdr) &&
 		    ((m = m_pullup(m, sizeof(struct ddpehdr))) == NULL)) {
+			AT_IFADDR_RUNLOCK();
 			ddpstat.ddps_tooshort++;
 			return;
 		}
@@ -206,6 +209,7 @@ ddp_input(struct mbuf *m, struct ifnet *ifp, struct elaphdr *elh, int phase)
 		to.sat_addr.s_node = ddpe.deh_dnode;
 		to.sat_port = ddpe.deh_dport;
 
+		AT_IFADDR_RLOCK();
 		if (to.sat_addr.s_net == ATADDR_ANYNET) {
 			/*
 			 * The TO address doesn't specify a net, so by
@@ -278,6 +282,9 @@ ddp_input(struct mbuf *m, struct ifnet *ifp, struct elaphdr *elh, int phase)
 			}
 		}
 	}
+	if (aa != NULL)
+		ifa_ref(&aa->aa_ifa);
+	AT_IFADDR_RUNLOCK();
 
 	/*
 	 * Adjust the length, removing any padding that may have been added
@@ -287,8 +294,7 @@ ddp_input(struct mbuf *m, struct ifnet *ifp, struct elaphdr *elh, int phase)
 	mlen = m->m_pkthdr.len;
 	if (mlen < dlen) {
 		ddpstat.ddps_toosmall++;
-		m_freem(m);
-		return;
+		goto out;
 	}
 	if (mlen > dlen)
 		m_adj(m, dlen - mlen);
@@ -304,10 +310,8 @@ ddp_input(struct mbuf *m, struct ifnet *ifp, struct elaphdr *elh, int phase)
 		/* 
 		 * If we've explicitly disabled it, don't route anything.
 		 */
-		if (ddp_forward == 0) {
-			m_freem(m);
-			return;
-		}
+		if (ddp_forward == 0)
+			goto out;
 
 		/* 
 		 * If the cached forwarding route is still valid, use it.
@@ -346,10 +350,8 @@ ddp_input(struct mbuf *m, struct ifnet *ifp, struct elaphdr *elh, int phase)
 		 */
 		if ((to.sat_addr.s_net !=
 		    satosat(&forwro.ro_dst)->sat_addr.s_net) &&
-		    (ddpe.deh_hops == DDP_MAXHOPS)) {
-			m_freem(m);
-			return;
-		}
+		    (ddpe.deh_hops == DDP_MAXHOPS))
+			goto out;
 
 		/*
 		 * A ddp router might use the same interface to forward the
@@ -357,10 +359,8 @@ ddp_input(struct mbuf *m, struct ifnet *ifp, struct elaphdr *elh, int phase)
 		 * to cross from one interface to another however.
 		 */
 		if (ddp_firewall && ((forwro.ro_rt == NULL) ||
-		    (forwro.ro_rt->rt_ifp != ifp))) {
-			m_freem(m);
-			return;
-		}
+		    (forwro.ro_rt->rt_ifp != ifp)))
+			goto out;
 
 		/*
 		 * Adjust the header.  If it was a short header then it would
@@ -377,6 +377,8 @@ ddp_input(struct mbuf *m, struct ifnet *ifp, struct elaphdr *elh, int phase)
 			ddpstat.ddps_cantforward++;
 		else
 			ddpstat.ddps_forward++;
+		if (aa != NULL)
+			ifa_free(&aa->aa_ifa);
 		return;
 	}
 
@@ -393,8 +395,7 @@ ddp_input(struct mbuf *m, struct ifnet *ifp, struct elaphdr *elh, int phase)
 		if (ddp_cksum && cksum && cksum !=
 		    at_cksum(m, sizeof(int))) {
 			ddpstat.ddps_badsum++;
-			m_freem(m);
-			return;
+			goto out;
 		}
 		m_adj(m, sizeof(struct ddpehdr));
 	} else
@@ -405,11 +406,11 @@ ddp_input(struct mbuf *m, struct ifnet *ifp, struct elaphdr *elh, int phase)
 	 */
 	DDP_LIST_SLOCK();
 	if ((ddp = ddp_search(&from, &to, aa)) == NULL)
-		goto out;
+		goto out_unlock;
 
 #ifdef MAC
 	if (mac_socket_check_deliver(ddp->ddp_socket, m) != 0)
-		goto out;
+		goto out_unlock;
 #endif
 
 	/* 
@@ -423,7 +424,7 @@ ddp_input(struct mbuf *m, struct ifnet *ifp, struct elaphdr *elh, int phase)
 		 * If the socket is full (or similar error) dump the packet.
 		 */
 		ddpstat.ddps_nosockspace++;
-		goto out;
+		goto out_unlock;
 	}
 
 	/*
@@ -431,8 +432,11 @@ ddp_input(struct mbuf *m, struct ifnet *ifp, struct elaphdr *elh, int phase)
 	 */
 	sorwakeup_locked(ddp->ddp_socket);
 	m = NULL;
-out:
+out_unlock:
 	DDP_LIST_SUNLOCK();
+out:
+	if (aa != NULL)
+		ifa_free(&aa->aa_ifa);
 	if (m != NULL)
 		m_freem(m);
 }
diff --git a/sys/netatalk/ddp_output.c b/sys/netatalk/ddp_output.c
index 6fa75ce..a820f24 100644
--- a/sys/netatalk/ddp_output.c
+++ b/sys/netatalk/ddp_output.c
@@ -142,12 +142,16 @@ ddp_route(struct mbuf *m, struct route *ro)
 	if ((ro->ro_rt != NULL) && (ro->ro_rt->rt_ifa) &&
 	    (ifp = ro->ro_rt->rt_ifa->ifa_ifp)) {
 		net = ntohs(satosat(ro->ro_rt->rt_gateway)->sat_addr.s_net);
+		AT_IFADDR_RLOCK();
 		for (aa = at_ifaddr_list; aa != NULL; aa = aa->aa_next) {
 			if (((net == 0) || (aa->aa_ifp == ifp)) &&
 			    net >= ntohs(aa->aa_firstnet) &&
 			    net <= ntohs(aa->aa_lastnet))
 				break;
 		}
+		if (aa != NULL)
+			ifa_ref(&aa->aa_ifa);
+		AT_IFADDR_RUNLOCK();
 	} else {
 		m_freem(m);
 #ifdef NETATALK_DEBUG
@@ -199,6 +203,7 @@ ddp_route(struct mbuf *m, struct route *ro)
 	if (!(aa->aa_flags & AFA_PHASE2)) {
 		MGET(m0, M_DONTWAIT, MT_DATA);
 		if (m0 == NULL) {
+			ifa_free(&aa->aa_ifa);
 			m_freem(m);
 			printf("ddp_route: no buffers\n");
 			return (ENOBUFS);
@@ -231,8 +236,11 @@ ddp_route(struct mbuf *m, struct route *ro)
 	if ((satosat(&aa->aa_addr)->sat_addr.s_net ==
 	    satosat(&ro->ro_dst)->sat_addr.s_net) &&
 	    (satosat(&aa->aa_addr)->sat_addr.s_node ==
-	    satosat(&ro->ro_dst)->sat_addr.s_node))
+	    satosat(&ro->ro_dst)->sat_addr.s_node)) {
+		ifa_free(&aa->aa_ifa);
 		return (if_simloop(ifp, m, gate.sat_family, 0));
+	}
+	ifa_free(&aa->aa_ifa);
 
 	/* XXX */
 	return ((*ifp->if_output)(ifp, m, (struct sockaddr *)&gate, NULL));
diff --git a/sys/netatalk/ddp_pcb.c b/sys/netatalk/ddp_pcb.c
index e40c84b..a793a4f 100644
--- a/sys/netatalk/ddp_pcb.c
+++ b/sys/netatalk/ddp_pcb.c
@@ -1,5 +1,5 @@
 /*-
- * Copyright (c) 2004-2005 Robert N. M. Watson
+ * Copyright (c) 2004-2009 Robert N. M. Watson
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -104,6 +104,7 @@ at_pcbsetaddr(struct ddpcb *ddp, struct sockaddr *addr, struct thread *td)
 	/*
 	 * Validate passed address.
 	 */
+	aa = NULL;
 	if (addr != NULL) {
 		sat = (struct sockaddr_at *)addr;
 		if (sat->sat_family != AF_APPLETALK)
@@ -111,6 +112,7 @@ at_pcbsetaddr(struct ddpcb *ddp, struct sockaddr *addr, struct thread *td)
 
 		if (sat->sat_addr.s_node != ATADDR_ANYNODE ||
 		    sat->sat_addr.s_net != ATADDR_ANYNET) {
+			AT_IFADDR_RLOCK();
 			for (aa = at_ifaddr_list; aa != NULL;
 			    aa = aa->aa_next) {
 				if ((sat->sat_addr.s_net ==
@@ -119,6 +121,7 @@ at_pcbsetaddr(struct ddpcb *ddp, struct sockaddr *addr, struct thread *td)
 				    AA_SAT(aa)->sat_addr.s_node))
 					break;
 			}
+			AT_IFADDR_RUNLOCK();
 			if (aa == NULL)
 				return (EADDRNOTAVAIL);
 		}
@@ -142,9 +145,13 @@ at_pcbsetaddr(struct ddpcb *ddp, struct sockaddr *addr, struct thread *td)
 
 	if (sat->sat_addr.s_node == ATADDR_ANYNODE &&
 	    sat->sat_addr.s_net == ATADDR_ANYNET) {
-		if (at_ifaddr_list == NULL)
+		AT_IFADDR_RLOCK();
+		if (at_ifaddr_list == NULL) {
+			AT_IFADDR_RUNLOCK();
 			return (EADDRNOTAVAIL);
+		}
 		sat->sat_addr = AA_SAT(at_ifaddr_list)->sat_addr;
+		AT_IFADDR_RUNLOCK();
 	}
 	ddp->ddp_lsat = *sat;
 
@@ -220,6 +227,7 @@ at_pcbconnect(struct ddpcb *ddp, struct sockaddr *addr, struct thread *td)
 		else
 			net = sat->sat_addr.s_net;
 		aa = NULL;
+		AT_IFADDR_RLOCK();
 		if ((ifp = ro->ro_rt->rt_ifp) != NULL) {
 			for (aa = at_ifaddr_list; aa != NULL;
 			    aa = aa->aa_next) {
@@ -236,6 +244,7 @@ at_pcbconnect(struct ddpcb *ddp, struct sockaddr *addr, struct thread *td)
 			RTFREE(ro->ro_rt);
 			ro->ro_rt = NULL;
 		}
+		AT_IFADDR_RUNLOCK();
 	}
 
 	/*
@@ -258,10 +267,12 @@ at_pcbconnect(struct ddpcb *ddp, struct sockaddr *addr, struct thread *td)
 	 */
 	aa = NULL;
 	if (ro->ro_rt && (ifp = ro->ro_rt->rt_ifp)) {
+		AT_IFADDR_RLOCK();
 		for (aa = at_ifaddr_list; aa != NULL; aa = aa->aa_next) {
 			if (aa->aa_ifp == ifp)
 				break;
 		}
+		AT_IFADDR_RUNLOCK();
 	}
 	if (aa == NULL)
 		return (ENETUNREACH);