Rui Paulo wrote:
> On 2005.11.30 11:42:00 +0000, guy@alum.mit.edu 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;
>   or
>         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
> mem:
>
>                 case BPF_LDX|BPF_MEM:
>                         X = mem[pc->k];
>                         continue;
>
>                 case BPF_STX:
>                         mem[pc->k] = X;
>                         continue;
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 
easier.