When generating the ICMP6 response to an IPv6 packet, the kernel

could use mbuf memory after freeing it.  If m_pullup() allocates a
new mbuf, the caller uses the old pointer.
found and reported by Maxime Villard, thanks
OK claudio@ markus@ denis@

2 files changed, 11 insertions, 10 deletions

diff --git a/sys/netinet/icmp6.h b/sys/netinet/icmp6.h
index ce28ea9cb10..468b45ca43f 100644
--- a/sys/netinet/icmp6.h
+++ b/sys/netinet/icmp6.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: icmp6.h,v 1.49 2020/09/01 01:53:13 gnezdo Exp $	*/
+/*	$OpenBSD: icmp6.h,v 1.50 2020/10/28 17:27:35 bluhm Exp $	*/
 /*	$KAME: icmp6.h,v 1.84 2003/04/23 10:26:51 itojun Exp $	*/
 
 /*
@@ -594,7 +594,7 @@ struct mbuf	*icmp6_do_error(struct mbuf *, int, int, int);
 void		 icmp6_error(struct mbuf *, int, int, int);
 int		 icmp6_input(struct mbuf **, int *, int, int);
 void		 icmp6_fasttimo(void);
-int		 icmp6_reflect(struct mbuf *, size_t, struct sockaddr *);
+int		 icmp6_reflect(struct mbuf **, size_t, struct sockaddr *);
 void		 icmp6_prepare(struct mbuf *);
 void		 icmp6_redirect_input(struct mbuf *, int);
 void		 icmp6_redirect_output(struct mbuf *, struct rtentry *);
diff --git a/sys/netinet6/icmp6.c b/sys/netinet6/icmp6.c
index a483c17e5ab..e27dbf9b696 100644
--- a/sys/netinet6/icmp6.c
+++ b/sys/netinet6/icmp6.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: icmp6.c,v 1.232 2020/09/01 01:53:13 gnezdo Exp $	*/
+/*	$OpenBSD: icmp6.c,v 1.233 2020/10/28 17:27:35 bluhm Exp $	*/
 /*	$KAME: icmp6.c,v 1.217 2001/06/20 15:03:29 jinmei Exp $	*/
 
 /*
@@ -379,7 +379,7 @@ icmp6_error(struct mbuf *m, int type, int code, int param)
 	n = icmp6_do_error(m, type, code, param);
 	if (n != NULL) {
 		/* header order: IPv6 - ICMPv6 */
-		if (!icmp6_reflect(n, sizeof(struct ip6_hdr), NULL))
+		if (!icmp6_reflect(&n, sizeof(struct ip6_hdr), NULL))
 			ip6_send(n);
 	}
 }                                                                    
@@ -609,7 +609,7 @@ icmp6_input(struct mbuf **mp, int *offp, int proto, int af)
 			nicmp6->icmp6_code = 0;
 			icmp6stat_inc(icp6s_reflect);
 			icmp6stat_inc(icp6s_outhist + ICMP6_ECHO_REPLY);
-			if (!icmp6_reflect(n, noff, NULL))
+			if (!icmp6_reflect(&n, noff, NULL))
 				ip6_send(n);
 		}
 		if (!m)
@@ -1044,8 +1044,9 @@ icmp6_mtudisc_update(struct ip6ctlparam *ip6cp, int validated)
  * OFF points to the icmp6 header, counted from the top of the mbuf.
  */
 int
-icmp6_reflect(struct mbuf *m, size_t off, struct sockaddr *sa)
+icmp6_reflect(struct mbuf **mp, size_t off, struct sockaddr *sa)
 {
+	struct mbuf *m = *mp;
 	struct rtentry *rt = NULL;
 	struct ip6_hdr *ip6;
 	struct icmp6_hdr *icmp6;
@@ -1065,7 +1066,7 @@ icmp6_reflect(struct mbuf *m, size_t off, struct sockaddr *sa)
 	}
 
 	if (m->m_pkthdr.ph_loopcnt++ >= M_MAXLOOP) {
-		m_freem(m);
+		m_freemp(mp);
 		return (ELOOP);
 	}
 	rtableid = m->m_pkthdr.ph_rtableid;
@@ -1085,7 +1086,7 @@ icmp6_reflect(struct mbuf *m, size_t off, struct sockaddr *sa)
 		m_adj(m, l);
 		l = sizeof(struct ip6_hdr) + sizeof(struct icmp6_hdr);
 		if (m->m_len < l) {
-			if ((m = m_pullup(m, l)) == NULL)
+			if ((m = *mp = m_pullup(m, l)) == NULL)
 				return (EMSGSIZE);
 		}
 		memcpy(mtod(m, caddr_t), &nip6, sizeof(nip6));
@@ -1093,7 +1094,7 @@ icmp6_reflect(struct mbuf *m, size_t off, struct sockaddr *sa)
 		size_t l;
 		l = sizeof(struct ip6_hdr) + sizeof(struct icmp6_hdr);
 		if (m->m_len < l) {
-			if ((m = m_pullup(m, l)) == NULL)
+			if ((m = *mp = m_pullup(m, l)) == NULL)
 				return (EMSGSIZE);
 		}
 	}
@@ -1189,7 +1190,7 @@ icmp6_reflect(struct mbuf *m, size_t off, struct sockaddr *sa)
 	return (0);
 
  bad:
-	m_freem(m);
+	m_freemp(mp);
 	return (EHOSTUNREACH);
 }