Skip site navigation (1)Skip section navigation (2) 
Date:      Tue, 27 Apr 2010 13:50:16 +0000 (UTC)
From:      Bruce M Simpson <bms@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org
Subject:   svn commit: r207274 - stable/8/sys/netinet
Message-ID:  <201004271350.o3RDoGFw025776@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: bms
Date: Tue Apr 27 13:50:15 2010
New Revision: 207274
URL: http://svn.freebsd.org/changeset/base/207274

Log:
  MFC 206452:
    Fix a few issues related to the legacy 4.4 BSD multicast APIs.
  
    IPv4 addresses can and do change during normal operation. Testing by
    pfSense developers exposed an issue where OpenOSPFD was using the IPv4
    address to leave the OSPF link-scope multicast groups on a dynamic
    OpenVPN tun interface, rather than using RFC 3678 with the interface
    index, which won't be raced when the interface's addresses change.
  
    In inp_join_group():
     If we are already a member of an ASM group, and IP_ADD_MEMBERSHIP or
     MCAST_JOIN_GROUP ioctls are re-issued, return EADDRINUSE as per the
     legacy 4.4BSD multicast API. This bends RFC 3678 slightly, but does
     not violate POLA for apps using the old API.
     It also stops us falling through to kicking IGMP state transactions
     in what is otherwise a no-op case.
     [This has already been dealt with in HEAD, but make it explicit before
      we MFC the change to 8.]
  
    In inp_leave_group():
     Fix a bogus conditional.
     Move the ifp null check to ioctls MCAST_LEAVE* in the switch..case
     where it actually belongs.
     If an interface was specified, by primary IPv4 address, for ioctl
     IP_DROP_MEMBERSHIP or MCAST_LEAVE_GROUP (an ASM full leave operation),
     then and only then should we look up the ifp from the IPv4 address in
     mreqs.imr_interface.
     If not, we fall through to imo_match_group() as before, but only in
     the IP_DROP_MEMBERSHIP case.
  
    With these changes, the legacy 4.4BSD multicast API idempotence should
    be mostly preserved in the SSM enabled IPv4 stack.
  
    [Note: this is not a straight svn merge as head and 8 differ slightly]
  
  Found by:	ermal (with pfSense)

Modified:
  stable/8/sys/netinet/in_mcast.c
Directory Properties:
  stable/8/sys/   (props changed)
  stable/8/sys/amd64/include/xen/   (props changed)
  stable/8/sys/cddl/contrib/opensolaris/   (props changed)
  stable/8/sys/contrib/dev/acpica/   (props changed)
  stable/8/sys/contrib/pf/   (props changed)
  stable/8/sys/dev/xen/xenpci/   (props changed)
  stable/8/sys/geom/sched/   (props changed)

Modified: stable/8/sys/netinet/in_mcast.c
==============================================================================
--- stable/8/sys/netinet/in_mcast.c	Tue Apr 27 13:27:51 2010	(r207273)
+++ stable/8/sys/netinet/in_mcast.c	Tue Apr 27 13:50:15 2010	(r207274)
@@ -1991,6 +1991,17 @@ inp_join_group(struct inpcb *inp, struct
 				error = EINVAL;
 				goto out_inp_locked;
 			}
+			/*
+			 * MCAST_JOIN_GROUP on an existing exclusive
+			 * membership is an error; return EADDRINUSE
+			 * to preserve 4.4BSD API idempotence, and
+			 * avoid tedious detour to code below.
+			 * NOTE: This is bending RFC 3678 a bit.
+			 */
+			if (imf->imf_st[1] == MCAST_EXCLUDE) {
+				error = EADDRINUSE;
+				goto out_inp_locked;
+			}
 		}
 	}
 
@@ -2161,7 +2172,14 @@ inp_leave_group(struct inpcb *inp, struc
 			ssa->sin.sin_addr = mreqs.imr_sourceaddr;
 		}
 
-		if (!in_nullhost(gsa->sin.sin_addr))
+		/*
+		 * Attempt to look up hinted ifp from interface address.
+		 * Fallthrough with null ifp iff lookup fails, to
+		 * preserve 4.4BSD mcast API idempotence.
+		 * XXX NOTE WELL: The RFC 3678 API is preferred because
+		 * using an IPv4 address as a key is racy.
+		 */
+		if (!in_nullhost(mreqs.imr_interface))
 			INADDR_TO_IFP(mreqs.imr_interface, ifp);
 
 		CTR3(KTR_IGMPV3, "%s: imr_interface = %s, ifp = %p",
@@ -2197,6 +2215,9 @@ inp_leave_group(struct inpcb *inp, struc
 			return (EADDRNOTAVAIL);
 
 		ifp = ifnet_byindex(gsr.gsr_interface);
+
+		if (ifp == NULL)
+			return (EADDRNOTAVAIL);
 		break;
 
 	default:
@@ -2209,9 +2230,6 @@ inp_leave_group(struct inpcb *inp, struc
 	if (!IN_MULTICAST(ntohl(gsa->sin.sin_addr.s_addr)))
 		return (EINVAL);
 
-	if (ifp == NULL)
-		return (EADDRNOTAVAIL);
-
 	/*
 	 * Find the membership in the membership array.
 	 */

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201004271350.o3RDoGFw025776>