netdev - Re: [PATCH net-next 1/4] bpf: allow bpf programs to tail-call other bpf programs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <555E0A93.2020803@plumgrid.com>
Date:	Thu, 21 May 2015 09:40:51 -0700
From:	Alexei Starovoitov <ast@...mgrid.com>
To:	Andy Lutomirski <luto@...capital.net>
CC:	"David S. Miller" <davem@...emloft.net>,
	Ingo Molnar <mingo@...nel.org>,
	Daniel Borkmann <daniel@...earbox.net>,
	Michael Holzheu <holzheu@...ux.vnet.ibm.com>,
	Zi Shen Lim <zlim.lnx@...il.com>,
	Linux API <linux-api@...r.kernel.org>,
	Network Development <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next 1/4] bpf: allow bpf programs to tail-call other
 bpf programs

On 5/21/15 9:20 AM, Andy Lutomirski wrote:
>
> What I mean is: why do we need the interface to be "look up this index
> in an array and just to what it references" as a single atomic
> instruction?  Can't we break it down into first "look up this index in
> an array" and then "do this tail call"?

I've actually considered to do this split and do first part as map 
lookup and 2nd as 'tail call to this ptr' insn, but it turned out to be
painful: verifier gets more complicated, ctx pointer needs to kept
somewhere, JITs need to special case two things instead of one.
Also I couldn't see a use case for exposing program pointer to the
program itself. I've explored this path only because it felt more
traditional 'goto *ptr' like, but adding new PTR_TO_PROG type to
verifier looked wasteful.

> I don't see why everything needs to be a map.

I mentioned the reasons to use map abstraction in the commit log:
"- jump table is implemented as BPF_MAP_TYPE_PROG_ARRAY to reuse 'map'
   abstraction, its user space API and all of verifier logic.
   It's in the existing arraymap.c file, since several functions are
   shared with regular array map."

The other alternative would be to add new thing just for jump table,
but it means extending syscall commands and propagating the callchain
through several files plus adding all new interfaces to user space.
I think 'map' abstraction fits very well. We have 'array' map
which is one-to-one to normal C array. This is just different type
of array that stores prog_fds.
When in C you're creating 'void *jmptable[] = { &&label1, &&label2};'
it is still an array. So here you have special type PROG_ARRAY for it
to make verifier recognize it.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html