From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Borkmann Subject: [PATCH bpf 3/5] bpf: guarantee r1 to be ctx in case of bpf_helper_changes_pkt_data Date: Thu, 14 Dec 2017 21:07:25 +0100 Message-ID: <20171214200727.22230-4-daniel@iogearbox.net> References: <20171214200727.22230-1-daniel@iogearbox.net> Cc: holzheu@linux.vnet.ibm.com, naveen.n.rao@linux.vnet.ibm.com, davem@davemloft.net, netdev@vger.kernel.org, Daniel Borkmann To: ast@fb.com Return-path: Received: from www62.your-server.de ([213.133.104.62]:32795 "EHLO www62.your-server.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752160AbdLNUHk (ORCPT ); Thu, 14 Dec 2017 15:07:40 -0500 In-Reply-To: <20171214200727.22230-1-daniel@iogearbox.net> Sender: netdev-owner@vger.kernel.org List-ID: Some JITs don't cache skb context on stack in prologue, so when LD_ABS/IND is used and helper calls yield bpf_helper_changes_pkt_data() as true, then they temporarily save/restore skb pointer. However, the assumption that skb always has to be in r1 is a bit of a gamble. Right now it turned out to be true for all helpers listed in bpf_helper_changes_pkt_data(), but lets enforce that from verifier side, so that we make this a guarantee and bail out if the func proto is misconfigured in future helpers. In case of BPF helper calls from cBPF, bpf_helper_changes_pkt_data() is completely unrelevant here (since cBPF is context read-only) and therefore always false. Signed-off-by: Daniel Borkmann Acked-by: Alexei Starovoitov --- kernel/bpf/verifier.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index d459357..e39b013 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -1674,7 +1674,13 @@ static int check_call(struct bpf_verifier_env *env, int func_id, int insn_idx) return -EINVAL; } + /* With LD_ABS/IND some JITs save/restore skb from r1. */ changes_data = bpf_helper_changes_pkt_data(fn->func); + if (changes_data && fn->arg1_type != ARG_PTR_TO_CTX) { + verbose(env, "kernel subsystem misconfigured func %s#%d: r1 != ctx\n", + func_id_name(func_id), func_id); + return -EINVAL; + } memset(&meta, 0, sizeof(meta)); meta.pkt_access = fn->pkt_access; -- 2.9.5