bridged IPv6 packets rewritten with embedded scope IDs

In NetBSD, the 802.11 hostap bridge has started inserting
scope IDs into
link-local IPv6 addresses before retransmission.  The ICMP6
checksum on
the repeated packet is wrong.  A bridge shouldn't be
rewriting packets,
anyway.

I suspect this is fallout from recent changes to scope-ID
embedding.
ip6_input does not take sufficient care to avoid writing to
unwriteable
mbufs such as the shallow mbuf copy produced by the AP
bridging code.
In particular, it does not call m_makewritable before
embedding scope
IDs in the source and destination addresses with
in6_setscope.

This packet trace illustrates the problem:

Packet received by AP:

13:05:19.170044 00:02:6f:20:f6:2e > 33:33:00:00:00:01,
ethertype IPv6 (0x86dd), length 70: fe80::202:6fff:fe20:f62e
> ff02::1: [icmp6 sum ok] icmp6: echo request seq 1539
(len 16, hlim 64)

Repeated packet---notice the destination turned from ff02::1
to ff02:5::1:

13:05:19.170429 00:02:6f:20:f6:2e > 33:33:00:00:00:01,
ethertype IPv6 (0x86dd), length 70:
fe80:5::202:6fff:fe20:f62e > ff02:5::1: [bad icmp6 cksum
f5ff!] icmp6: echo request seq 1539 (len 16, hlim 64)

AP's reply:

13:05:19.170562 xx:yy:zz:20:44:12 > 00:02:6f:20:f6:2e,
ethertype IPv6 (0x86dd), length 70: fe80::250:43ff:fe20:4412
> fe80::202:6fff:fe20:f62e: [icmp6 sum ok] icmp6: echo
reply seq 1539 (len 16, hlim 64)

Dave

-- 
David Young             OJC Technologies
dyoungojctech.com      Urbana, IL * (217) 278-3933

On Wed, Mar 22, 2006 at 10:30:39PM -0600, David Young wrote:
> In NetBSD, the 802.11 hostap bridge has started
inserting scope IDs into
> link-local IPv6 addresses before retransmission.  The
ICMP6 checksum on
> the repeated packet is wrong.  A bridge shouldn't be
rewriting packets,
> anyway.
> 
> I suspect this is fallout from recent changes to
scope-ID embedding.
> ip6_input does not take sufficient care to avoid
writing to unwriteable
> mbufs such as the shallow mbuf copy produced by the AP
bridging code.
> In particular, it does not call m_makewritable before
embedding scope
> IDs in the source and destination addresses with
in6_setscope.
> 
> This packet trace illustrates the problem:
> 
> Packet received by AP:
> 
> 13:05:19.170044 00:02:6f:20:f6:2e >
33:33:00:00:00:01, ethertype IPv6 (0x86dd), length 70:
fe80::202:6fff:fe20:f62e > ff02::1: [icmp6 sum ok] icmp6:
echo request seq 1539 (len 16, hlim 64)
> 
> Repeated packet---notice the destination turned from
ff02::1 to ff02:5::1:
> 
> 13:05:19.170429 00:02:6f:20:f6:2e >
33:33:00:00:00:01, ethertype IPv6 (0x86dd), length 70:
fe80:5::202:6fff:fe20:f62e > ff02:5::1: [bad icmp6 cksum
f5ff!] icmp6: echo request seq 1539 (len 16, hlim 64)
> 
> AP's reply:
> 
> 13:05:19.170562 xx:yy:zz:20:44:12 >
00:02:6f:20:f6:2e, ethertype IPv6 (0x86dd), length 70:
fe80::250:43ff:fe20:4412 > fe80::202:6fff:fe20:f62e:
[icmp6 sum ok] icmp6: echo reply seq 1539 (len 16, hlim 64)

Here is an (untested) patch that may fix the problem.

Dave

-- 
David Young             OJC Technologies
dyoungojctech.com      Urbana, IL * (217) 278-3933
Index: in6.h
============================================================
=======
RCS file: /cvsroot/src/sys/netinet6/in6.h,v
retrieving revision 1.52
diff -u -p -u -p -r1.52 in6.h
--- in6.h	16 Feb 2006 20:17:20 -0000	1.52
+++ in6.h	23 Mar 2006 05:14:02 -0000
 -359,6
+359,9  extern const struct in6_addr in6addr_lin
 	((IN6_IS_ADDR_LINKLOCAL(a)) ||	\
 	 (IN6_IS_ADDR_MC_LINKLOCAL(a)))
 
+#define	IN6_IS_SCOPE_EMBEDDABLE(__a)	\
+    (IN6_IS_SCOPE_LINKLOCAL(__a) ||
IN6_IS_ADDR_MC_INTFACELOCAL(__a))
+
 #define IFA6_IS_DEPRECATED(a) \
 	((a)->ia6_lifetime.ia6t_pltime != ND6_INFINITE_LIFETIME
&& \
 	 (u_int32_t)((time.tv_sec - (a)->ia6_updatetime)) >
\
Index: ip6_input.c
============================================================
=======
RCS file: /cvsroot/src/sys/netinet6/ip6_input.c,v
retrieving revision 1.83
diff -u -p -u -p -r1.83 ip6_input.c
--- ip6_input.c	5 Mar 2006 23:47:08 -0000	1.83
+++ ip6_input.c	23 Mar 2006 05:14:02 -0000
 -234,7
+234,7  void
 ip6_input(m)
 	struct mbuf *m;
 {
-	struct ip6_hdr *ip6;
+	struct ip6_hdr *ip6, *tmp;
 	int off = sizeof(struct ip6_hdr), nest;
 	u_int32_t plen;
 	u_int32_t rtalert = ~0;
 -305,7
+305,24  ip6_input(m)
 		}
 	}
 
-	ip6 = mtod(m, struct ip6_hdr *);
+        /* If we will embed a scope identifier in either
the source or
+         * destination address or both, then make them both
writable.
+	 * We have to do this because m may be a shallow copy of a
packet
+	 * that a link-layer bridge will repeat.
+	 */
+	tmp = mtod(m, struct ip6_hdr *);
+	if (!(IN6_IS_SCOPE_EMBEDDABLE(&tmp->ip6_src) ||
+	      IN6_IS_SCOPE_EMBEDDABLE(&tmp->ip6_dst)))
+		ip6 = tmp;
+	else if (m_makewritable(&m, offsetof(struct ip6_hdr,
ip6_src),
+	    sizeof(struct ip6_hdr) - offsetof(struct ip6_hdr,
ip6_src),
+	    M_DONTWAIT) != 0) {
+		struct ifnet *inifp = m->m_pkthdr.rcvif;
+		ip6stat.ip6s_toosmall++;	/* XXXDCY new stat */
+		in6_ifstat_inc(inifp, ifs6_in_hdrerr);	/* XXX DCY new
stat */
+		goto bad;
+	} else
+		ip6 = mtod(m, struct ip6_hdr *);
 
 	if ((ip6->ip6_vfc & IPV6_VERSION_MASK) !=
IPV6_VERSION) {
 		ip6stat.ip6s_badvers++;
Index: scope6.c
============================================================
=======
RCS file: /cvsroot/src/sys/netinet6/scope6.c,v
retrieving revision 1.2
diff -u -p -u -p -r1.2 scope6.c
--- scope6.c	5 Mar 2006 01:28:51 -0000	1.2
+++ scope6.c	23 Mar 2006 05:14:02 -0000
 -427,7
+427,7  in6_setscope(struct in6_addr *in6, struc
 	if (ret_id != NULL)
 		*ret_id = zoneid;
 
-	if (IN6_IS_SCOPE_LINKLOCAL(in6) ||
IN6_IS_ADDR_MC_INTFACELOCAL(in6))
+	if (IN6_IS_SCOPE_EMBEDDABLE(in6))
 		in6->s6_addr16[1] = htons(zoneid & 0xffff); /* XXX
*/
 
 	return (0);

On Wed, Mar 22, 2006 at 11:26:37PM -0600, David Young wrote:
> Here is an (untested) patch that may fix the problem.

I think that m_makewritable() should be moved closer to the
place where
it will be written. With your patch, the packet may be
dropped
immediatly after being made writable.

What about the attached patch (also untested) instead ?
Also note that this may also be a problem with Xen, which
may pass
mbufs pointing to read-only mapped areas to the network
stacks.

-- 
Manuel Bouyer <bouyerantioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la
difference
--
Index: in6.h
============================================================
=======
RCS file: /cvsroot/src/sys/netinet6/in6.h,v
retrieving revision 1.52
diff -u -r1.52 in6.h
--- in6.h	16 Feb 2006 20:17:20 -0000	1.52
+++ in6.h	23 Mar 2006 13:39:22 -0000
 -359,6
+359,9 
 	((IN6_IS_ADDR_LINKLOCAL(a)) ||	\
 	 (IN6_IS_ADDR_MC_LINKLOCAL(a)))
 
+#define	IN6_IS_SCOPE_EMBEDDABLE(__a)	\
+    (IN6_IS_SCOPE_LINKLOCAL(__a) ||
IN6_IS_ADDR_MC_INTFACELOCAL(__a))
+
 #define IFA6_IS_DEPRECATED(a) \
 	((a)->ia6_lifetime.ia6t_pltime != ND6_INFINITE_LIFETIME
&& \
 	 (u_int32_t)((time.tv_sec - (a)->ia6_updatetime)) >
\
Index: ip6_input.c
============================================================
=======
RCS file: /cvsroot/src/sys/netinet6/ip6_input.c,v
retrieving revision 1.83
diff -u -r1.83 ip6_input.c
--- ip6_input.c	5 Mar 2006 23:47:08 -0000	1.83
+++ ip6_input.c	23 Mar 2006 13:39:22 -0000
 -399,6
+399,22 
 	}
 #endif
 
+        /*
+	 * If we will embed a scope identifier in either the
source or
+	 * destination address or both, then make them both
writable.
+	 */
+	if (IN6_IS_SCOPE_EMBEDDABLE(&ip6->ip6_src) ||
+	      IN6_IS_SCOPE_EMBEDDABLE(&ip6->ip6_dst)) {
+		if (m_makewritable(&m, offsetof(struct ip6_hdr,
ip6_src),
+		    sizeof(struct ip6_hdr) - offsetof(struct ip6_hdr,
ip6_src),
+		    M_DONTWAIT) != 0) {
+			struct ifnet *inifp = m->m_pkthdr.rcvif;
+			ip6stat.ip6s_toosmall++;	/* XXXDCY new stat */
+			in6_ifstat_inc(inifp, ifs6_in_hdrerr);	/* XXX DCY new
stat */
+			goto bad;
+		} else
+			ip6 = mtod(m, struct ip6_hdr *);
+	}
 	/*
 	 * Disambiguate address scope zones (if there is
ambiguity).
 	 * We first make sure that the original source or
destination address
Index: scope6.c
============================================================
=======
RCS file: /cvsroot/src/sys/netinet6/scope6.c,v
retrieving revision 1.2
diff -u -r1.2 scope6.c
--- scope6.c	5 Mar 2006 01:28:51 -0000	1.2
+++ scope6.c	23 Mar 2006 13:39:22 -0000
 -427,7
+427,7 
 	if (ret_id != NULL)
 		*ret_id = zoneid;
 
-	if (IN6_IS_SCOPE_LINKLOCAL(in6) ||
IN6_IS_ADDR_MC_INTFACELOCAL(in6))
+	if (IN6_IS_SCOPE_EMBEDDABLE(in6))
 		in6->s6_addr16[1] = htons(zoneid & 0xffff); /* XXX
*/
 
 	return (0);