Subject: icmp6_redirect_output() writes to free memory
To: None <tech-net@netbsd.org>
From: Manuel Bouyer <bouyer@antioche.eu.org>
List: tech-net
Date: 10/16/2005 17:31:48
--x+6KMIRAuhnl3hBn
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hi,
I found a bug in icmp6_redirect_output() thanks to Xen (on a domU Xen,
the data portion of a mbuf may be unmapped from kernel VM space when
the mbuf is freed, so using a free mbuf will panic the kernel).

In icmp6_redirect_output(), sip6 points to the data part of m0. But sip6 will
be used after m0 is possibly freed, just after the noredhdropt label:

If m0 is not NULL at this point it's explicitly freed here. If it's NULL,
it's because it has been m_cat()'d to m, which may also free it.
Does the attached patch make sense (reinitializing sip6 with m instead of
m0) ? I'm not sure if, in the case where m0 has been m_cat()'d to m, we
want to changes the addresses in the ip6 header in m, or the original
addresses in m0 (if so, more code will be needed to make sip6 point to the
right place in m).

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

--x+6KMIRAuhnl3hBn
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=diff

Index: icmp6.c
===================================================================
RCS file: /cvsroot/src/sys/netinet6/icmp6.c,v
retrieving revision 1.108
diff -u -r1.108 icmp6.c
--- icmp6.c	17 Jan 2005 10:16:07 -0000	1.108
+++ icmp6.c	16 Oct 2005 15:22:13 -0000
@@ -2618,6 +2618,7 @@
 		m0 = NULL;
 	}
 
+	sip6 = mtod(m, struct ip6_hdr *);
 	if (IN6_IS_ADDR_LINKLOCAL(&sip6->ip6_src))
 		sip6->ip6_src.s6_addr16[1] = 0;
 	if (IN6_IS_ADDR_LINKLOCAL(&sip6->ip6_dst))

--x+6KMIRAuhnl3hBn--