[Fwd: Panic: if_freemulti: protospec not NULL]
Andre Oppermann
andre at freebsd.org
Wed Mar 28 14:30:08 UTC 2007
Bruce M. Simpson wrote:
> Patch for this condition is attached.
This patch fixes the panic for me.
--
Andre
> This particular bug is irritating: the IPv4 stack joins 224.0.0.1 once
> for every IPv4 unicast address configured in the stack. This is
> incorrect behaviour. The implementation of refcounting has exposed this
> bug. The fix is not particularly elegant.
>
> It is most likely a left-over from the FreeBSD 2.x/3.x era when IPv4
> 'aliases' were first introduced. At that point in time the IGMP code in
> FreeBSD would allow groups to be joined on a per-IPv4-address basis,
> which is inconsistent with the IGMPv2/v3 specified behaviour (and indeed
> that addressed in future multicast RFCs).
>
> It seems that there are other situations where the stack is not
> adequately equipped to deal with interfaces going away unexpectedly. We
> can't afford to be complacent about multicast code on the basis of 'it's
> not the critical path', because it is an integral component of IPv6, and
> many ideas which people are trying to implement in IPv4 also require
> that the multicast code is fit for purpose.
>
> We would do well to have more people available to help on reviewing and
> possibly rewriting parts of the network stack from a perspective of
> correctness, not just performance. If this interests you please consider
> signing up to the Wiki and updating the page at
> http://wiki.freebsd.org/NetworkRFCCompliance.
>
> Regards,
> BMS
>
>
> ------------------------------------------------------------------------
>
> ==== //depot/user/bms/netdev/sys/netinet/in.c#11 - /home/bms/p4/netdev/sys/netinet/in.c ====
> --- /tmp/tmp.1127.0 Tue Mar 27 16:43:56 2007
> +++ /home/bms/p4/netdev/sys/netinet/in.c Tue Mar 27 16:40:04 2007
> @@ -212,6 +212,11 @@
> /*
> * Generic internet control operations (ioctl's).
> * Ifp is 0 if not an interface-specific ioctl.
> + *
> + * XXX: This code has some nice big juicy bugs related to interface
> + * attach and detach. Specifically, the 224.0.0.1 group is now only
> + * joined on the first IPv4 address configured on an interface, and
> + * only left when all IPv4 state is torn down for an interface.
> */
> /* ARGSUSED */
> int
> @@ -230,7 +235,9 @@
> struct in_aliasreq *ifra = (struct in_aliasreq *)data;
> struct sockaddr_in oldaddr;
> int error, hostIsNew, iaIsNew, maskIsNew, s;
> + int iaIsFirst;
>
> + iaIsFirst = 0;
> iaIsNew = 0;
>
> switch (cmd) {
> @@ -282,6 +289,8 @@
> break;
> }
> }
> + if (ia == NULL)
> + iaIsFirst = 1;
> }
>
> switch (cmd) {
> @@ -423,8 +432,20 @@
> (struct sockaddr_in *) &ifr->ifr_addr, 1);
> if (error != 0 && iaIsNew)
> break;
> - if (error == 0)
> + if (error == 0) {
> + /*
> + * Only join 224.0.0.1 if this is the very first
> + * IPv4 unicast address which has been configured
> + * on this ifnet.
> + */
> + if (iaIsFirst && (ifp->if_flags & IFF_MULTICAST) != 0) {
> + struct in_addr addr;
> +
> + addr.s_addr = htonl(INADDR_ALLHOSTS_GROUP);
> + in_addmulti(&addr, ifp);
> + }
> EVENTHANDLER_INVOKE(ifaddr_event, ifp);
> + }
> return (0);
>
> case SIOCSIFNETMASK:
> @@ -467,8 +488,20 @@
> if ((ifp->if_flags & IFF_BROADCAST) &&
> (ifra->ifra_broadaddr.sin_family == AF_INET))
> ia->ia_broadaddr = ifra->ifra_broadaddr;
> - if (error == 0)
> + if (error == 0) {
> + /*
> + * Only join 224.0.0.1 if this is the very first
> + * IPv4 unicast address which has been configured
> + * on this ifnet.
> + */
> + if (iaIsFirst && (ifp->if_flags & IFF_MULTICAST) != 0) {
> + struct in_addr addr;
> +
> + addr.s_addr = htonl(INADDR_ALLHOSTS_GROUP);
> + in_addmulti(&addr, ifp);
> + }
> EVENTHANDLER_INVOKE(ifaddr_event, ifp);
> + }
> return (error);
>
> case SIOCDIFADDR:
> @@ -503,8 +536,33 @@
> s = splnet();
> TAILQ_REMOVE(&ifp->if_addrhead, &ia->ia_ifa, ifa_link);
> TAILQ_REMOVE(&in_ifaddrhead, ia, ia_link);
> - if (ia->ia_addr.sin_family == AF_INET)
> + if (ia->ia_addr.sin_family == AF_INET) {
> + struct in_ifaddr *pia;
> +
> LIST_REMOVE(ia, ia_hash);
> + /*
> + * Only purge the 224.0.0.1 membership if the last IPv4
> + * unicast address configured on this ifnet is removed.
> + *
> + * XXX: This currently involves trawling through the
> + * in_ifaddrhead list looking for a match, which is
> + * inefficient, though this is only done when an interface
> + * address goes away. This could be done much more elegantly.
> + */
> + pia = NULL;
> + IFP_TO_IA(ifp, pia);
> + if (pia == NULL) {
> + struct in_multi *inm;
> + struct in_addr addr;
> +
> + addr.s_addr = htonl(INADDR_ALLHOSTS_GROUP);
> + IN_MULTI_LOCK();
> + IN_LOOKUP_MULTI(addr, ifp, inm);
> + if (inm != NULL)
> + in_delmulti(inm);
> + IN_MULTI_UNLOCK();
> + }
> + }
> IFAFREE(&ia->ia_ifa);
> splx(s);
>
> @@ -793,16 +851,6 @@
> if ((error = in_addprefix(ia, flags)) != 0)
> return (error);
>
> - /*
> - * If the interface supports multicast, join the "all hosts"
> - * multicast group on that interface.
> - */
> - if (ifp->if_flags & IFF_MULTICAST) {
> - struct in_addr addr;
> -
> - addr.s_addr = htonl(INADDR_ALLHOSTS_GROUP);
> - in_addmulti(&addr, ifp);
> - }
> return (error);
> }
>
> @@ -1114,6 +1162,9 @@
> igmp_leavegroup(inm);
>
> ifma = inm->inm_ifma;
> +#ifdef DIAGNOSTIC
> + printf("%s: purging ifma %p\n", __func__, ifma);
> +#endif
> KASSERT(ifma->ifma_protospec == inm,
> ("%s: ifma_protospec != inm", __func__));
> ifma->ifma_protospec = NULL;
> @@ -1135,6 +1186,9 @@
> struct in_multi *inm;
> struct in_multi *oinm;
>
> +#ifdef DIAGNOSTIC
> + printf("%s: purging ifp %p\n", __func__, ifp);
> +#endif
> IFF_LOCKGIANT(ifp);
> IN_MULTI_LOCK();
> LIST_FOREACH_SAFE(inm, &in_multihead, inm_link, oinm) {
More information about the freebsd-current
mailing list