Skip site navigation (1)Skip section navigation (2) 
Date:      Thu, 13 Dec 2012 12:51:23 +0000 (UTC)
From:      Gleb Smirnoff <glebius@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r244185 - head/sys/netpfil/pf
Message-ID:  <201212131251.qBDCpNfg035426@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: glebius
Date: Thu Dec 13 12:51:22 2012
New Revision: 244185
URL: http://svnweb.freebsd.org/changeset/base/244185

Log:
  Merge rev. 1.119 from OpenBSD:
  
    date: 2009/03/31 01:21:29;  author: dlg;  state: Exp;  lines: +9 -16
    ...
  
    this also firms up some of the input parsing so it handles short frames a
    bit better.
  
  This actually fixes reading beyond mbuf data area in pfsync_input(), that
  may happen at certain pfsync datagrams.

Modified:
  head/sys/netpfil/pf/if_pfsync.c

Modified: head/sys/netpfil/pf/if_pfsync.c
==============================================================================
--- head/sys/netpfil/pf/if_pfsync.c	Thu Dec 13 12:48:57 2012	(r244184)
+++ head/sys/netpfil/pf/if_pfsync.c	Thu Dec 13 12:51:22 2012	(r244185)
@@ -44,6 +44,7 @@
 
 /*
  * Revisions picked from OpenBSD after revision 1.110 import:
+ * 1.119 - don't m_copydata() beyond the len of mbuf in pfsync_input()
  * 1.118, 1.124, 1.148, 1.149, 1.151, 1.171 - fixes to bulk updates
  * 1.120, 1.175 - use monotonic time_uptime
  * 1.122 - reduce number of updates for non-TCP sessions
@@ -566,7 +567,7 @@ pfsync_input(struct mbuf *m, __unused in
 	struct pfsync_header *ph;
 	struct pfsync_subheader subh;
 
-	int offset;
+	int offset, len;
 	int rv;
 	uint16_t count;
 
@@ -612,6 +613,12 @@ pfsync_input(struct mbuf *m, __unused in
 		goto done;
 	}
 
+	len = ntohs(ph->len) + offset;
+	if (m->m_pkthdr.len < len) {
+		pfsyncstats.pfsyncs_badlen++;
+		goto done;
+	}
+
 	/* Cheaper to grab this now than having to mess with mbufs later */
 	pkt.ip = ip;
 	pkt.src = ip->ip_src;
@@ -626,7 +633,7 @@ pfsync_input(struct mbuf *m, __unused in
 		pkt.flags |= PFSYNC_SI_CKSUM;
 
 	offset += sizeof(*ph);
-	for (;;) {
+	while (offset <= len - sizeof(subh)) {
 		m_copydata(m, offset, sizeof(subh), (caddr_t)&subh);
 		offset += sizeof(subh);
 
@@ -1222,8 +1229,8 @@ static int
 pfsync_in_eof(struct pfsync_pkt *pkt, struct mbuf *m, int offset, int count)
 {
 	/* check if we are at the right place in the packet */
-	if (offset != m->m_pkthdr.len - sizeof(struct pfsync_eof))
-		V_pfsyncstats.pfsyncs_badact++;
+	if (offset != m->m_pkthdr.len)
+		V_pfsyncstats.pfsyncs_badlen++;
 
 	/* we're done. free and let the caller return */
 	m_freem(m);
@@ -1592,8 +1599,6 @@ pfsync_sendout(int schedswi)
 	subh->count = htons(1);
 	V_pfsyncstats.pfsyncs_oacts[PFSYNC_ACT_EOF]++;
 
-	/* XXX write checksum in EOF here */
-
 	/* we're done, let's put it on the wire */
 	if (ifp->if_bpf) {
 		m->m_data += sizeof(*ip);

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