Subject: Re: kern/32928: bpf filter can fail to extract a 32-bit quantity
To: None <netbsd-bugs@netbsd.org>
From: Rui Paulo <rpaulo@fnop.net>
List: netbsd-bugs
Date: 02/25/2006 12:51:33
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}>