From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Dumazet Subject: Re: [PATCH net-next-2.6] filter: add a security check at install time Date: Thu, 02 Dec 2010 10:54:53 +0100 Message-ID: <1291283693.2871.48.camel@edumazet-laptop> References: <1291227893.2856.1039.camel@edumazet-laptop> <20101201.104450.183053379.davem@davemloft.net> <1291232937.2856.1042.camel@edumazet-laptop> <20101201.122312.229751364.davem@davemloft.net> <1291236342.2856.1057.camel@edumazet-laptop> <1291272384.2856.1074.camel@edumazet-laptop> <1291280000.2871.16.camel@edumazet-laptop> <1291280402.2871.20.camel@edumazet-laptop> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: David Miller , hagen@jauu.net, wirelesser@gmail.com, netdev@vger.kernel.org, Dan Rosenberg To: Changli Gao Return-path: Received: from mail-ww0-f42.google.com ([74.125.82.42]:57184 "EHLO mail-ww0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751043Ab0LBJy6 (ORCPT ); Thu, 2 Dec 2010 04:54:58 -0500 Received: by wwb39 with SMTP id 39so871537wwb.1 for ; Thu, 02 Dec 2010 01:54:57 -0800 (PST) In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: Le jeudi 02 d=C3=A9cembre 2010 =C3=A0 17:10 +0800, Changli Gao a =C3=A9= crit : > On Thu, Dec 2, 2010 at 5:00 PM, Eric Dumazet = wrote: > > Le jeudi 02 d=C3=A9cembre 2010 =C3=A0 09:53 +0100, Eric Dumazet a =C3= =A9crit : > >> Le jeudi 02 d=C3=A9cembre 2010 =C3=A0 16:11 +0800, Changli Gao a =C3= =A9crit : > >> > >> > It seems correct to me now. > >> > > >> > Acked-by: Changli Gao > >> > > >> > >> Thanks for reviewing Changli. > >> > >> Now I am thinking about not denying the filter installation, but c= hange > >> the problematic LOAD M(1) and LOADX M(1) by LOADI #0 (BPF_S_LD_I= MM > >> K=3D0) and LOADIX #0 (BPF_S_LDX_IMM K=3D0) >=20 > 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. >=20 It was not 'initialized to 0', thats the point of previous patches. > jeq jt jf > jt: > store m[0] > jf: > load m[0] >=20 > After applying your patch, the third instruction will be replaced wit= h > load 0. It is wrong for the jt branch. So NACK. >=20 >=20 But this is _exactly_ the case we want to deny (or protect) We want to :=20 - 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)