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@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe live-patching" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
