Pavel Cahyna <pcah8322@artax.karlin.mff.cuni.cz> writes:

>>Number:         32928
>>Category:       kern
>>Synopsis:       bpf filter can fail to extract a 32-bit quantity
>>Confidential:   no
>>Severity:       serious
>>Priority:       medium
>>Responsible:    kern-bug-people
>>State:          open
>>Class:          sw-bug
>>Submitter-Id:   net
>>Arrival-Date:   Sat Feb 25 12:20:00 +0000 2006
>>Originator:     Pavel Cahyna
>>Release:        4.3BSD NET/2 - NetBSD 3.99.15
>>Organization:
>>Environment:
> System: NetBSD beta 3.0_RC5 NetBSD 3.0_RC5 (EV56) #3: Mon Dec 12 20:28:20 CET 2005 pavel@beta:/usr/src/sys/arch/alpha/compile/EV56 alpha
> Architecture: alpha
> Machine: alpha
>>Description:
> In net/bpf_filter.c there is a function m_xword() which extracts a
> 32-bit integer from a mbuf chain at a given offset k.
>
> First, it determines where the integer starts:
> 	MINDEX(len, m, k);
> m is now the first mbuf, len is its length and k is updated to the
> offset of the data in the mbuf. Then we test if the whole integer is
> in the mbuf:
> 	if (len >= k + 4) {
> 		*err = 0;
> 		return EXTRACT_LONG(cp);
> 	}
> if it is not, we go to the next mbuf:
> 	m0 = m->m_next;
> 	if (m0 == 0 || m0->m_len + len - k < 4)
> 		goto bad;
>
> but the "goto bad" if "m0->m_len + len - k < 4" is wrong - the integer
> could legally continue in a third mbuf. Of course removing this check
> is not enough - some code must be added to handle this situation.
>>How-To-Repeat:
> Code inspection.
>
> This will be difficult to trigger (a 1- or 2- byte mbuf in the middle
> of a mbuf chain is unlikely), but with functions like bpf_mtap2()
> which prepend a short mbuf it could be possible.
>
>>Fix:
> I'm working on it. I propose to call m_xhalf() twice and concatenate
> the results.

What about:

--- bpf_filter.c.~1.29.~	2006-02-10 20:08:13.000000000 +0000
+++ bpf_filter.c	2006-02-25 12:51:07.000000000 +0000
@@ -98,9 +98,13 @@ m_xword(struct mbuf *m, uint32_t k, int 
 		*err = 0;
 		return EXTRACT_LONG(cp);
 	}
-	m0 = m->m_next;
-	if (m0 == 0 || m0->m_len + len - k < 4)
-		goto bad;
+
+	for (m0 = m->m_next; ; m0 = m0->next) {
+		if (m0 == 0)
+			goto bad;
+		if (m0->m_len + len - k >= 4)
+			break;
+	}
 	*err = 0;
 	np = mtod(m0, u_char *);
 	switch (len - k) {


-- 
  Rui Paulo			<rpaulo@{NetBSD{,-PT}.org,fnop.net}>