Subject: Re: kern/32198: bpf_validate() needs to do more checks
To: Rui Paulo <firstname.lastname@example.org>
From: Guy Harris <email@example.com>
Date: 11/30/2005 10:56:59
Rui Paulo wrote:
> On 2005.11.30 11:42:00 +0000, firstname.lastname@example.org wrote:
> | >Description:
> | OpenBSD's bpf_validate() in sys/net/bpf_filter.c does some additional checks to catch:
> | BPF programs with no instructions or with more than BPF_MAXINSNS instructions;
> This is done in bpf_setf();
The check for no instructions isn't - is that check necessary?
> | BPF_STX and BPF_LDX|BPF_MEM instructions that have out-of-range offsets (which could be made to fetch or store into arbitrary memory locations);
> | BPF_DIV instructions with a constant 0 divisor (that's a check also done at run time).
> What's wrong with the current checks in bpf_validate() ?
If that question applies to both of those items:
For the first item, to quote a message from Dawson Engler:
> we're developing a symbolic execution tool to find bugs in code and it
> seems to have turned up two errors in recent BPF code that appear to
> let filters read/write arbitrary memory. Vern recommended that I ask
> you guys if these were legitimate bugs.
> If you have a filter with either:
> code == BPF_STX;
> code == BPF_LDX|BPF_MEM;
> then from what I can tell with testing bpf_validate forgets to check
> their associated offset stored in field "k" (it does check
> for BPF_ST and BPF_LD but not the *X versions):
> * Check that memory operations use valid addresses.
> if ((BPF_CLASS(p->code) == BPF_ST ||
> (BPF_CLASS(p->code) == BPF_LD &&
> (p->code & 0xe0) == BPF_MEM)) &&
> p->k >= BPF_MEMWORDS)
> return 0;
> then the interpreter lets you read/write arbitrary offsets off of
> case BPF_LDX|BPF_MEM:
> X = mem[pc->k];
> case BPF_STX:
> mem[pc->k] = X;
Did he miss something, or is that, indeed, a risk?
For the second item:
The install-time check isn't necessary, but it makes the code more
closely resemble the code in OpenBSD, to make keeping the code in sync