From owner-svn-src-head@FreeBSD.ORG Mon Jun 22 10:23:55 2009 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 1B1B1106564A; Mon, 22 Jun 2009 10:23:55 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 079D08FC12; Mon, 22 Jun 2009 10:23:55 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id n5MANtvI034326; Mon, 22 Jun 2009 10:23:55 GMT (envelope-from rwatson@svn.freebsd.org) Received: (from rwatson@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id n5MANsF8034318; Mon, 22 Jun 2009 10:23:54 GMT (envelope-from rwatson@svn.freebsd.org) Message-Id: <200906221023.n5MANsF8034318@svn.freebsd.org> From: Robert Watson Date: Mon, 22 Jun 2009 10:23:54 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r194619 - head/sys/netatalk X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 22 Jun 2009 10:23:55 -0000 Author: rwatson Date: Mon Jun 22 10:23:54 2009 New Revision: 194619 URL: http://svn.freebsd.org/changeset/base/194619 Log: 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 Modified: head/sys/netatalk/COPYRIGHT head/sys/netatalk/aarp.c head/sys/netatalk/at_control.c head/sys/netatalk/at_var.h head/sys/netatalk/ddp_input.c head/sys/netatalk/ddp_output.c head/sys/netatalk/ddp_pcb.c Modified: head/sys/netatalk/COPYRIGHT ============================================================================== --- head/sys/netatalk/COPYRIGHT Mon Jun 22 10:11:35 2009 (r194618) +++ head/sys/netatalk/COPYRIGHT Mon Jun 22 10:23:54 2009 (r194619) @@ -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 Modified: head/sys/netatalk/aarp.c ============================================================================== --- head/sys/netatalk/aarp.c Mon Jun 22 10:11:35 2009 (r194618) +++ head/sys/netatalk/aarp.c Mon Jun 22 10:23:54 2009 (r194619) @@ -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 soc * 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 soc 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 soc 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 mb 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 mb 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 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 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 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 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 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 m */ aarptfree(aat); AARPTAB_UNLOCK(); + ifa_free(&aa->aa_ifa); m_freem(m); return; } @@ -473,6 +495,7 @@ at_aarpinput(struct ifnet *ifp, struct 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 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 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; Modified: head/sys/netatalk/at_control.c ============================================================================== --- head/sys/netatalk/at_control.c Mon Jun 22 10:11:35 2009 (r194618) +++ head/sys/netatalk/at_control.c Mon Jun 22 10:23:54 2009 (r194619) @@ -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 +__FBSDID("$FreeBSD$"); + #include #include #include +#include #include #include #include +#include #include #include #include @@ -43,7 +47,10 @@ #include #include -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 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 * 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 * * 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 */ 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 /* * 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 } } - if (aa == NULL) + if (aa == NULL) { + AT_IFADDR_WUNLOCK(); return (EADDRNOTAVAIL); + } break; } @@ -275,25 +301,31 @@ at_control(struct socket *so, u_long cmd 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 else panic("at_control"); } + AT_IFADDR_WUNLOCK(); /* * Now reclaim the reference. @@ -328,6 +361,7 @@ at_control(struct socket *so, u_long cmd 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, str (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) { Modified: head/sys/netatalk/at_var.h ============================================================================== --- head/sys/netatalk/at_var.h Mon Jun 22 10:11:35 2009 (r194618) +++ head/sys/netatalk/at_var.h Mon Jun 22 10:23:54 2009 (r194619) @@ -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_ */ Modified: head/sys/netatalk/ddp_input.c ============================================================================== --- head/sys/netatalk/ddp_input.c Mon Jun 22 10:11:35 2009 (r194618) +++ head/sys/netatalk/ddp_input.c Mon Jun 22 10:23:54 2009 (r194619) @@ -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 * * 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 * * 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 * 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 * 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 * } } } + 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 * 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 * /* * 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 * */ 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 * * 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 * 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 * 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 * */ 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 * * 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 * */ 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); } Modified: head/sys/netatalk/ddp_output.c ============================================================================== --- head/sys/netatalk/ddp_output.c Mon Jun 22 10:11:35 2009 (r194618) +++ head/sys/netatalk/ddp_output.c Mon Jun 22 10:23:54 2009 (r194619) @@ -142,12 +142,16 @@ ddp_route(struct mbuf *m, struct route * 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 * 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 * 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)); Modified: head/sys/netatalk/ddp_pcb.c ============================================================================== --- head/sys/netatalk/ddp_pcb.c Mon Jun 22 10:11:35 2009 (r194618) +++ head/sys/netatalk/ddp_pcb.c Mon Jun 22 10:23:54 2009 (r194619) @@ -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 /* * 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 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 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 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 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 RTFREE(ro->ro_rt); ro->ro_rt = NULL; } + AT_IFADDR_RUNLOCK(); } /* @@ -258,10 +267,12 @@ at_pcbconnect(struct ddpcb *ddp, struct */ 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);