Module Name: src Committed By: martin Date: Thu Jun 7 17:48:31 UTC 2018
Modified Files: src/sys/netinet6 [netbsd-8]: in6.c in6_var.h mld6.c nd6.c Log Message: Pull up following revision(s) (requested by ozaki-r in ticket #842): sys/netinet6/mld6.c: revision 1.93-1.99 sys/netinet6/in6_var.h: revision 1.99,1.100 sys/netinet6/in6.c: revision 1.267,1.268 sys/netinet6/nd6.c: revision 1.249 Don't hold softnet_lock in mld_timeo Then we can get rid of remaining abuses of mutex_owned(softnet_lock). Release in6_multilock on callout_halt of mld_timeo to avoid a deadlock Improve atomicity of in6_leavegroup and in6_delmulti Avoid NULL pointer dereference on imm->i6mm_maddr Make a refcount decrement and a removal from a list of an item atomic in6m_refcount of an in6m can be incremented if the in6m is on the list (if_multiaddrs) in in6_addmulti or mld_input. So we must avoid such an increment when we try to destroy an in6m. To this end we must make an in6m_refcount decrement and a removal of an in6m from if_multiaddrs atomic. Make a deletion of in6m in nd6_rtrequest atomic Move LIST_REMOVE mld_stoptimer releases in6_multilock temporarily, so we must LIST_REMOVE first. Avoid double LIST_REMOVE which corrupts lists Mark in6m as used for non-DIAGNOSTIC builds. To generate a diff of this commit: cvs rdiff -u -r1.245.2.10 -r1.245.2.11 src/sys/netinet6/in6.c cvs rdiff -u -r1.97 -r1.97.6.1 src/sys/netinet6/in6_var.h cvs rdiff -u -r1.89.2.1 -r1.89.2.2 src/sys/netinet6/mld6.c cvs rdiff -u -r1.232.2.7 -r1.232.2.8 src/sys/netinet6/nd6.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/sys/netinet6/in6.c diff -u src/sys/netinet6/in6.c:1.245.2.10 src/sys/netinet6/in6.c:1.245.2.11 --- src/sys/netinet6/in6.c:1.245.2.10 Sun Apr 8 06:09:12 2018 +++ src/sys/netinet6/in6.c Thu Jun 7 17:48:31 2018 @@ -1,4 +1,4 @@ -/* $NetBSD: in6.c,v 1.245.2.10 2018/04/08 06:09:12 snj Exp $ */ +/* $NetBSD: in6.c,v 1.245.2.11 2018/06/07 17:48:31 martin Exp $ */ /* $KAME: in6.c,v 1.198 2001/07/18 09:12:38 itojun Exp $ */ /* @@ -62,7 +62,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: in6.c,v 1.245.2.10 2018/04/08 06:09:12 snj Exp $"); +__KERNEL_RCSID(0, "$NetBSD: in6.c,v 1.245.2.11 2018/06/07 17:48:31 martin Exp $"); #ifdef _KERNEL_OPT #include "opt_inet.h" @@ -1407,9 +1407,11 @@ in6_purgeaddr(struct ifaddr *ifa) again: mutex_enter(&in6_ifaddr_lock); while ((imm = LIST_FIRST(&ia->ia6_memberships)) != NULL) { + struct in6_multi *in6m __diagused = imm->i6mm_maddr; + KASSERT(in6m == NULL || in6m->in6m_ifp == ifp); LIST_REMOVE(imm, i6mm_chain); mutex_exit(&in6_ifaddr_lock); - KASSERT(imm->i6mm_maddr->in6m_ifp == ifp); + in6_leavegroup(imm); goto again; } Index: src/sys/netinet6/in6_var.h diff -u src/sys/netinet6/in6_var.h:1.97 src/sys/netinet6/in6_var.h:1.97.6.1 --- src/sys/netinet6/in6_var.h:1.97 Thu Mar 2 09:48:20 2017 +++ src/sys/netinet6/in6_var.h Thu Jun 7 17:48:31 2018 @@ -1,4 +1,4 @@ -/* $NetBSD: in6_var.h,v 1.97 2017/03/02 09:48:20 ozaki-r Exp $ */ +/* $NetBSD: in6_var.h,v 1.97.6.1 2018/06/07 17:48:31 martin Exp $ */ /* $KAME: in6_var.h,v 1.81 2002/06/08 11:16:51 itojun Exp $ */ /* @@ -691,6 +691,9 @@ void in6_purge_multi(struct ifnet *); struct in6_multi *in6_addmulti(struct in6_addr *, struct ifnet *, int *, int); void in6_delmulti(struct in6_multi *); +void in6_delmulti_locked(struct in6_multi *); +void in6_lookup_and_delete_multi(const struct in6_addr *, + const struct ifnet *); struct in6_multi_mship *in6_joingroup(struct ifnet *, struct in6_addr *, int *, int); int in6_leavegroup(struct in6_multi_mship *); Index: src/sys/netinet6/mld6.c diff -u src/sys/netinet6/mld6.c:1.89.2.1 src/sys/netinet6/mld6.c:1.89.2.2 --- src/sys/netinet6/mld6.c:1.89.2.1 Tue Jan 2 10:20:34 2018 +++ src/sys/netinet6/mld6.c Thu Jun 7 17:48:31 2018 @@ -1,4 +1,4 @@ -/* $NetBSD: mld6.c,v 1.89.2.1 2018/01/02 10:20:34 snj Exp $ */ +/* $NetBSD: mld6.c,v 1.89.2.2 2018/06/07 17:48:31 martin Exp $ */ /* $KAME: mld6.c,v 1.25 2001/01/16 14:14:18 itojun Exp $ */ /* @@ -102,7 +102,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: mld6.c,v 1.89.2.1 2018/01/02 10:20:34 snj Exp $"); +__KERNEL_RCSID(0, "$NetBSD: mld6.c,v 1.89.2.2 2018/06/07 17:48:31 martin Exp $"); #ifdef _KERNEL_OPT #include "opt_inet.h" @@ -225,10 +225,7 @@ mld_stoptimer(struct in6_multi *in6m) rw_exit(&in6_multilock); - if (mutex_owned(softnet_lock)) - callout_halt(&in6m->in6m_timer_ch, softnet_lock); - else - callout_halt(&in6m->in6m_timer_ch, NULL); + callout_halt(&in6m->in6m_timer_ch, NULL); rw_enter(&in6_multilock, RW_WRITER); @@ -242,7 +239,7 @@ mld_timeo(void *arg) KASSERT(in6m->in6m_refcount > 0); - SOFTNET_KERNEL_LOCK_UNLESS_NET_MPSAFE(); + KERNEL_LOCK_UNLESS_NET_MPSAFE(); rw_enter(&in6_multilock, RW_WRITER); if (in6m->in6m_timer == IN6M_TIMER_UNDEF) goto out; @@ -260,7 +257,7 @@ mld_timeo(void *arg) out: rw_exit(&in6_multilock); - SOFTNET_KERNEL_UNLOCK_UNLESS_NET_MPSAFE(); + KERNEL_UNLOCK_UNLESS_NET_MPSAFE(); } static u_long @@ -786,15 +783,19 @@ in6m_destroy(struct in6_multi *in6m) KASSERT(in6m->in6m_refcount == 0); /* - * No remaining claims to this record; let MLD6 know - * that we are leaving the multicast group. + * Unlink from list if it's listed. This must be done before + * mld_stop_listening because it releases in6_multilock and that allows + * someone to look up the removing in6m from the list and add a + * reference to the entry unexpectedly. */ - mld_stop_listening(in6m); + if (in6_lookup_multi(&in6m->in6m_addr, in6m->in6m_ifp) != NULL) + LIST_REMOVE(in6m, in6m_entry); /* - * Unlink from list. + * No remaining claims to this record; let MLD6 know + * that we are leaving the multicast group. */ - LIST_REMOVE(in6m, in6m_entry); + mld_stop_listening(in6m); /* * Delete all references of this multicasting group from @@ -811,25 +812,25 @@ in6m_destroy(struct in6_multi *in6m) /* Tell mld_timeo we're halting the timer */ in6m->in6m_timer = IN6M_TIMER_UNDEF; - if (mutex_owned(softnet_lock)) - callout_halt(&in6m->in6m_timer_ch, softnet_lock); - else - callout_halt(&in6m->in6m_timer_ch, NULL); + + rw_exit(&in6_multilock); + callout_halt(&in6m->in6m_timer_ch, NULL); callout_destroy(&in6m->in6m_timer_ch); free(in6m, M_IPMADDR); + rw_enter(&in6_multilock, RW_WRITER); } /* * Delete a multicast address record. */ void -in6_delmulti(struct in6_multi *in6m) +in6_delmulti_locked(struct in6_multi *in6m) { + KASSERT(rw_write_held(&in6_multilock)); KASSERT(in6m->in6m_refcount > 0); - rw_enter(&in6_multilock, RW_WRITER); /* * The caller should have a reference to in6m. So we don't need to care * of releasing the lock in mld_stoptimer. @@ -837,6 +838,14 @@ in6_delmulti(struct in6_multi *in6m) mld_stoptimer(in6m); if (--in6m->in6m_refcount == 0) in6m_destroy(in6m); +} + +void +in6_delmulti(struct in6_multi *in6m) +{ + + rw_enter(&in6_multilock, RW_WRITER); + in6_delmulti_locked(in6m); rw_exit(&in6_multilock); } @@ -859,6 +868,19 @@ in6_lookup_multi(const struct in6_addr * return in6m; } +void +in6_lookup_and_delete_multi(const struct in6_addr *addr, + const struct ifnet *ifp) +{ + struct in6_multi *in6m; + + rw_enter(&in6_multilock, RW_WRITER); + in6m = in6_lookup_multi(addr, ifp); + if (in6m != NULL) + in6_delmulti_locked(in6m); + rw_exit(&in6_multilock); +} + bool in6_multi_group(const struct in6_addr *addr, const struct ifnet *ifp) { @@ -881,6 +903,7 @@ in6_purge_multi(struct ifnet *ifp) rw_enter(&in6_multilock, RW_WRITER); LIST_FOREACH_SAFE(in6m, &ifp->if_multiaddrs, in6m_entry, next) { + LIST_REMOVE(in6m, in6m_entry); /* * Normally multicast addresses are already purged at this * point. Remaining references aren't accessible via ifp, @@ -888,7 +911,6 @@ in6_purge_multi(struct ifnet *ifp) * accessed via in6m by removing it from the list of ifp. */ mld_stoptimer(in6m); - LIST_REMOVE(in6m, in6m_entry); } rw_exit(&in6_multilock); } @@ -947,12 +969,13 @@ in6_leavegroup(struct in6_multi_mship *i { struct in6_multi *in6m; - rw_enter(&in6_multilock, RW_READER); + rw_enter(&in6_multilock, RW_WRITER); in6m = imm->i6mm_maddr; - rw_exit(&in6_multilock); + imm->i6mm_maddr = NULL; if (in6m != NULL) { - in6_delmulti(in6m); + in6_delmulti_locked(in6m); } + rw_exit(&in6_multilock); free(imm, M_IPMADDR); return 0; } Index: src/sys/netinet6/nd6.c diff -u src/sys/netinet6/nd6.c:1.232.2.7 src/sys/netinet6/nd6.c:1.232.2.8 --- src/sys/netinet6/nd6.c:1.232.2.7 Tue Mar 13 13:27:10 2018 +++ src/sys/netinet6/nd6.c Thu Jun 7 17:48:31 2018 @@ -1,4 +1,4 @@ -/* $NetBSD: nd6.c,v 1.232.2.7 2018/03/13 13:27:10 martin Exp $ */ +/* $NetBSD: nd6.c,v 1.232.2.8 2018/06/07 17:48:31 martin Exp $ */ /* $KAME: nd6.c,v 1.279 2002/06/08 11:16:51 itojun Exp $ */ /* @@ -31,7 +31,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: nd6.c,v 1.232.2.7 2018/03/13 13:27:10 martin Exp $"); +__KERNEL_RCSID(0, "$NetBSD: nd6.c,v 1.232.2.8 2018/06/07 17:48:31 martin Exp $"); #ifdef _KERNEL_OPT #include "opt_net_mpsafe.h" @@ -1607,18 +1607,14 @@ nd6_rtrequest(int req, struct rtentry *r if ((rt->rt_flags & RTF_ANNOUNCE) != 0 && (ifp->if_flags & IFF_MULTICAST) != 0) { struct in6_addr llsol; - struct in6_multi *in6m; llsol = satocsin6(rt_getkey(rt))->sin6_addr; llsol.s6_addr32[0] = htonl(0xff020000); llsol.s6_addr32[1] = 0; llsol.s6_addr32[2] = htonl(1); llsol.s6_addr8[12] = 0xff; - if (in6_setscope(&llsol, ifp, NULL) == 0) { - in6m = in6_lookup_multi(&llsol, ifp); - if (in6m) - in6_delmulti(in6m); - } + if (in6_setscope(&llsol, ifp, NULL) == 0) + in6_lookup_and_delete_multi(&llsol, ifp); } break; }