Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1315155

Re: [PATCH 31/33] bpf: Add __bpf_prog_run() to stacktool whitelist

Show all headers | View raw

On Thu, Jan 21, 2016 at 10:13:02PM -0600, Josh Poimboeuf wrote:
> On Thu, Jan 21, 2016 at 06:55:41PM -0800, Alexei Starovoitov wrote:
> > On Thu, Jan 21, 2016 at 04:49:35PM -0600, Josh Poimboeuf wrote:
> > > stacktool reports the following false positive warnings:
> > > 
> > >   stacktool: kernel/bpf/core.o: __bpf_prog_run()+0x5c: sibling call from callable instruction with changed frame pointer
> > >   stacktool: kernel/bpf/core.o: __bpf_prog_run()+0x60: function has unreachable instruction
> > >   stacktool: kernel/bpf/core.o: __bpf_prog_run()+0x64: function has unreachable instruction
> > >   [...]
> > > 
> > > It's confused by the following dynamic jump instruction in
> > > __bpf_prog_run()::
> > > 
> > >   jmp     *(%r12,%rax,8)
> > > 
> > > which corresponds to the following line in the C code:
> > > 
> > >   goto *jumptable[insn->code];
> > > 
> > > There's no way for stacktool to deterministically find all possible
> > > branch targets for a dynamic jump, so it can't verify this code.
> > > 
> > > In this case the jumps all stay within the function, and there's nothing
> > > unusual going on related to the stack, so we can whitelist the function.
> > 
> > well, few things are very unusual in this function.
> > did you see what JMP_CALL does? it's a call into a different function,
> > but not like typical indirect call. Will it be ok as well?
> > 
> > In general it's not possible for any tool to identify all possible
> > branch targets. bpf programs can be loaded on the fly and
> > jumping sequence will change.
> > So if this marking says 'don't bother analyzing this function because
> > it does sane stuff' that's probably not the case.
> > If this marking says 'don't bother analyzing, the stack may be crazy
> > from here on' then it's ok.
> 
> So the tool doesn't need to follow all possible call targets.  Instead
> it just verifies that all functions follow the frame pointer convention.
> That way it doesn't matter *which* function is being called because they
> all do the right thing.
> 
> But it *does* need to follow all jump targets, so that it can analyze
> all possible code paths within the function itself.  With a dynamic
> jump, it can't do that.
> 
> So the JMP_CALL is fine, but the goto *jumptable[insn->code] isn't.
> (And BTW that's the only occurrence of such a dynamic jump table in the
> entire kernel.)

Acked-by: Alexei Starovoitov <ast@kernel.org>

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[PATCH 31/33] bpf: Add __bpf_prog_run() to stacktool whitelist Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 00:00 +0100
  Re: [PATCH 31/33] bpf: Add __bpf_prog_run() to stacktool whitelist Daniel Borkmann <daniel@iogearbox.net> - 2016-01-22 00:00 +0100
  Re: [PATCH 31/33] bpf: Add __bpf_prog_run() to stacktool whitelist Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-01-22 04:00 +0100
    Re: [PATCH 31/33] bpf: Add __bpf_prog_run() to stacktool whitelist Josh Poimboeuf <jpoimboe@redhat.com> - 2016-01-22 05:20 +0100
      Re: [PATCH 31/33] bpf: Add __bpf_prog_run() to stacktool whitelist Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-01-22 18:30 +0100

csiph-web