Re: [PATCH net-next-2.6] filter: add a security check at install time

[Eric Dumazet <eric.dumazet@gmail.com>]

Le jeudi 02 décembre 2010 à 17:10 +0800, Changli Gao a écrit :
> On Thu, Dec 2, 2010 at 5:00 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Le jeudi 02 décembre 2010 à 09:53 +0100, Eric Dumazet a écrit :
> >> Le jeudi 02 décembre 2010 à 16:11 +0800, Changli Gao a écrit :
> >>
> >> > It seems correct to me now.
> >> >
> >> > Acked-by: Changli Gao <xiaosuo@gmail.com>
> >> >
> >>
> >> Thanks for reviewing Changli.
> >>
> >> Now I am thinking about not denying the filter installation, but change
> >> the problematic LOAD M(1)  and LOADX M(1)  by LOADI #0 (BPF_S_LD_IMM
> >> K=0) and LOADIX #0 (BPF_S_LDX_IMM K=0)
> 
> Oops. We were wrong. The RAM of BPF machine is initialized to 0. So
> loading from a cell, in which no value is stored before, is valid. So
> we can't prevent the following instructions.
> 

It was not 'initialized to 0', thats the point of previous patches.



> jeq jt jf

> jt:
> store m[0]
> jf:
> load m[0]
> 
> After applying your patch, the third instruction will be replaced with
> load 0. It is wrong for the jt branch. So NACK.
> 
> 

But this is _exactly_ the case we want to deny (or protect)

We want to : 

- Accept valid programs generated by libpcap current and future
optimizers. Show me a real sample.

A valid program doesnt mix stores/loads like you tried.

Memories are used because of limited instruction and register (A, X)
set.

- Make sure a malicious or stupid or buggy program doesnt read garbage
from stack.

After optimizer, your program should read (no memory load/stores)

RET #0


To let your 'program' run, we could add temporary state saying :

Memory K has a known value m(k), or an unknown one.
Register A has a known value a, or an unkown one.
Register X has a known value x, or an unkown one.

And be able to "optimize" stupid "jeq jt jf" tests if value of A is
known, since we know the result of test (only one branch will be taken)

I am not sure its worth it, really, since all instruction set should be
taken into account to maintain this state. (implement kind of an
optimizer in kernel)

It's probably better to spend time in userland optimizer, and a JIT
compiler...

(By the way, I believe FreeBSD has the security problem Dan reported to
us)