From patchwork Fri Oct 27 16:45:34 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 831413 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="pDKdEsnx"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3yNqWN1zVrz9t4c for ; Sat, 28 Oct 2017 03:45:52 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932414AbdJ0Qpu (ORCPT ); Fri, 27 Oct 2017 12:45:50 -0400 Received: from mail-pg0-f67.google.com ([74.125.83.67]:57309 "EHLO mail-pg0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932343AbdJ0Qps (ORCPT ); Fri, 27 Oct 2017 12:45:48 -0400 Received: by mail-pg0-f67.google.com with SMTP id m18so5687747pgd.13 for ; Fri, 27 Oct 2017 09:45:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:from:to:cc:date:message-id:in-reply-to:references :user-agent:mime-version:content-transfer-encoding; bh=8uvX0Z/qwTiZQw90FLIWbVkOvlkaWXKo2PU3YTakR78=; b=pDKdEsnxpMJJRu4jaaampd3iwUCWdmAX2fTCesvVnX0UB6cr5qealyPKTSuxRL4hd/ 8A7an0SBmehiTAj+ZEN8Vj4014QcIhXTk5bR7dsTZLxlresF4mOFG9nMuhiCtSgl5X4/ HnBcetRpdnG4+oYv1V91zf0D3hKNtQOIf9pqNFyt/oq65folHxNPNtQbtfLFJE+8YhBR CERDH1CakptDWMIkTM7tlKH2wPNA3Rmq9zSyyaaGtM5yuypJ+skcBP85bg/Vfsm9Wc+f muxlBqeaK2JfBeApGdc/jcosNuBCX5U7uo6WN5FIqC5MzfkYUwQRpt2s5b2OGuIQYoP7 FB9A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:cc:date:message-id:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=8uvX0Z/qwTiZQw90FLIWbVkOvlkaWXKo2PU3YTakR78=; b=rKhpDsFfhF4zNKoaKsdq8dp5/jjLkQVId1zC+EimUDozMUDZBHu1CAPO9JX8ygVMkC eawf9dE3Z0y5hiJuy3MIV2AcYuvTTFFFL4vfdUh6/owyXbaiKIWoU94HfLoMoxuNamrz reGHRV+axaIS8faK1db2tZgb+OvHaTuEQmbTkPHqt7VP5SKrWay7pBt1VzGiHU5HJyll nQZ2/uD2Q5t5qKYmLiJ290apyfI0fVodkKp/LgVrivSaDzHQNiHS7PezCAJeDP7UJSPq AUnNobO781jwk2XsRZIFKLwZrnR6tg3uMbDoC1sG6lbKfLMTmskbFEi/XiVo42R489sM ekZQ== X-Gm-Message-State: AMCzsaWd/UsbYqHDMnh35LyUarQKeqnwIOTBsG2rJwBbTjjx4cFAbKg3 S37qRmX+AoFvv6Ys+pKOrxuyJQ== X-Google-Smtp-Source: ABhQp+Q6MCabMY0jRQnGeQ5lmM8dzZSm1TGZerGqksMUZyDKG8cPJ5JIh7Y8ueR3NWx/Tth7kEUXcg== X-Received: by 10.84.224.206 with SMTP id k14mr803172pln.403.1509122747730; Fri, 27 Oct 2017 09:45:47 -0700 (PDT) Received: from [127.0.1.1] ([72.168.144.206]) by smtp.gmail.com with ESMTPSA id h1sm15969421pfg.143.2017.10.27.09.45.41 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 27 Oct 2017 09:45:47 -0700 (PDT) Subject: [net PATCH 1/2] bpf: bpf_compute_data uses incorrect cb structure From: John Fastabend To: alexei.starovoitov@gmail.com, davem@davemloft.net Cc: netdev@vger.kernel.org, edumazet@google.com, daniel@iogearbox.net Date: Fri, 27 Oct 2017 09:45:34 -0700 Message-ID: <20171027164534.28498.39170.stgit@john-Precision-Tower-5810> In-Reply-To: <20171027163830.28498.82997.stgit@john-Precision-Tower-5810> References: <20171027163830.28498.82997.stgit@john-Precision-Tower-5810> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org SK_SKB program types use bpf_compute_data to store the end of the packet data. However, bpf_compute_data assumes the cb is stored in the qdisc layer format. But, for SK_SKB this is the wrong layer of the stack for this type. It happens to work (sort of!) because in most cases nothing happens to be overwritten today. This is very fragile and error prone. Fortunately, we have another hole in tcp_skb_cb we can use so lets put the data_end value there. Note, SK_SKB program types do not use data_meta, they are failed by sk_skb_is_valid_access(). Signed-off-by: John Fastabend Acked-by: Alexei Starovoitov --- include/net/tcp.h | 1 + kernel/bpf/sockmap.c | 12 ++++++++++-- net/core/filter.c | 27 ++++++++++++++++++++++++++- 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/include/net/tcp.h b/include/net/tcp.h index b1ef98e..33599d17 100644 --- a/include/net/tcp.h +++ b/include/net/tcp.h @@ -844,6 +844,7 @@ struct tcp_skb_cb { __u32 key; __u32 flags; struct bpf_map *map; + void *data_end; } bpf; }; }; diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c index 2b6eb35..6778fb7 100644 --- a/kernel/bpf/sockmap.c +++ b/kernel/bpf/sockmap.c @@ -93,6 +93,14 @@ static inline struct smap_psock *smap_psock_sk(const struct sock *sk) return rcu_dereference_sk_user_data(sk); } +/* compute the linear packet data range [data, data_end) for skb when + * sk_skb type programs are in use. + */ +static inline void bpf_compute_data_end_sk_skb(struct sk_buff *skb) +{ + TCP_SKB_CB(skb)->bpf.data_end = skb->data + skb_headlen(skb); +} + static int smap_verdict_func(struct smap_psock *psock, struct sk_buff *skb) { struct bpf_prog *prog = READ_ONCE(psock->bpf_verdict); @@ -108,7 +116,7 @@ static int smap_verdict_func(struct smap_psock *psock, struct sk_buff *skb) */ TCP_SKB_CB(skb)->bpf.map = NULL; skb->sk = psock->sock; - bpf_compute_data_end(skb); + bpf_compute_data_end_sk_skb(skb); preempt_disable(); rc = (*prog->bpf_func)(skb, prog->insnsi); preempt_enable(); @@ -368,7 +376,7 @@ static int smap_parse_func_strparser(struct strparser *strp, * any socket yet. */ skb->sk = psock->sock; - bpf_compute_data_end(skb); + bpf_compute_data_end_sk_skb(skb); rc = (*prog->bpf_func)(skb, prog->insnsi); skb->sk = NULL; rcu_read_unlock(); diff --git a/net/core/filter.c b/net/core/filter.c index aa02659..68eaa2f 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -4243,6 +4243,31 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type, return insn - insn_buf; } +static u32 sk_skb_convert_ctx_access(enum bpf_access_type type, + const struct bpf_insn *si, + struct bpf_insn *insn_buf, + struct bpf_prog *prog, u32 *target_size) +{ + struct bpf_insn *insn = insn_buf; + int off; + + switch (si->off) { + case offsetof(struct __sk_buff, data_end): + off = si->off; + off -= offsetof(struct __sk_buff, data_end); + off += offsetof(struct sk_buff, cb); + off += offsetof(struct tcp_skb_cb, bpf.data_end); + *insn++ = BPF_LDX_MEM(BPF_SIZEOF(void *), si->dst_reg, + si->src_reg, off); + break; + default: + return bpf_convert_ctx_access(type, si, insn_buf, prog, + target_size); + } + + return insn - insn_buf; +} + const struct bpf_verifier_ops sk_filter_prog_ops = { .get_func_proto = sk_filter_func_proto, .is_valid_access = sk_filter_is_valid_access, @@ -4301,7 +4326,7 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type, const struct bpf_verifier_ops sk_skb_prog_ops = { .get_func_proto = sk_skb_func_proto, .is_valid_access = sk_skb_is_valid_access, - .convert_ctx_access = bpf_convert_ctx_access, + .convert_ctx_access = sk_skb_convert_ctx_access, .gen_prologue = sk_skb_prologue, }; From patchwork Fri Oct 27 16:45:53 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Fastabend X-Patchwork-Id: 831414 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="ZRK85Hab"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3yNqWk6Y05z9t4c for ; Sat, 28 Oct 2017 03:46:10 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932417AbdJ0QqI (ORCPT ); Fri, 27 Oct 2017 12:46:08 -0400 Received: from mail-pf0-f196.google.com ([209.85.192.196]:50142 "EHLO mail-pf0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752014AbdJ0QqG (ORCPT ); Fri, 27 Oct 2017 12:46:06 -0400 Received: by mail-pf0-f196.google.com with SMTP id i5so5339769pfe.6 for ; Fri, 27 Oct 2017 09:46:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:from:to:cc:date:message-id:in-reply-to:references :user-agent:mime-version:content-transfer-encoding; bh=L7+oMm2hQIzMY3zPnBwhI6QtZlBBgzDqwNYlEffK0qA=; b=ZRK85HabnGwbq55yJ8ApRWL2kP2WzNJdNPvqs52g41QHHUXxcg5bkvKTCV1ESfwzxf lfH709e3GnAz/8KRpTPnWTvNb6gN4BVrwS6K+aCTpVj3Qg7Eb37/q6EZGWQxu/PLyiWf pl/yg7hnyNdAzqVLLqO9jPYpm5UdZGRJ7iKnkEnjavlg2ud+BbBK81M19Zl2b9LTEhFB 8AOY1ie30IUBOQVGd8xxv4S/UcXlwhG/4aRBrp6i6Un9d/0Y1Emlo5BVTrUI5X8cVrf0 4aDPjydjsqHpCB+uH0UyWc8UPnMRG557duDE+jd3ZNP70Nwr/of4vpxJ7gONZsK68A/R HW4Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:cc:date:message-id:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=L7+oMm2hQIzMY3zPnBwhI6QtZlBBgzDqwNYlEffK0qA=; b=ljCmkLVVM/qnB10EvHuFRdLcw0Xp+Nzibarib3vQ/R7aXZq/0bziJJxYluuwWoayxk I4oOqHtWVP/pr3QcjZAfOKj/gFQ7UD5CWa/MvD+bnXmNPhMY1rNh+rgO8wJBnvhUrKwI nHuhJY4UuCVmUelxBm7Yj88Fta159fR2FbI85hCbkt/ibqoTmE730guEEVDBMjwyHS8I L6Nz91YWj8NBeLkmBZPJxSNLBn6Oi7+1kmtJWBaeBZlYclYhLAlF3eO9OCI6YaIq+cGW Y8DCB5dWrP+iFIB+V5hEP6UikyhdhzBlWqxezWKrNdyQMYwmSU9vTDbOQs50U2cL0udc DF7g== X-Gm-Message-State: AMCzsaXWyZ0nTgyrKvnftxkQhFq5VNUzuy7CgRSlNjL8gYyb+iQWBlzk NwuptGT8w4DrDlJ0+jOizkoAZQ== X-Google-Smtp-Source: ABhQp+RPfmra54rU1gvjT+0jIxbON1FdF8J/PElb9sYWTVsPgCmRmn9MqJUdyi7XqUNauzCEgI6TxA== X-Received: by 10.101.73.136 with SMTP id r8mr752053pgs.411.1509122765368; Fri, 27 Oct 2017 09:46:05 -0700 (PDT) Received: from [127.0.1.1] ([72.168.144.206]) by smtp.gmail.com with ESMTPSA id m195sm16281398pga.13.2017.10.27.09.45.59 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 27 Oct 2017 09:46:04 -0700 (PDT) Subject: [net PATCH 2/2] bpf: rename sk_actions to align with bpf infrastructure From: John Fastabend To: alexei.starovoitov@gmail.com, davem@davemloft.net Cc: netdev@vger.kernel.org, edumazet@google.com, daniel@iogearbox.net Date: Fri, 27 Oct 2017 09:45:53 -0700 Message-ID: <20171027164553.28498.47472.stgit@john-Precision-Tower-5810> In-Reply-To: <20171027163830.28498.82997.stgit@john-Precision-Tower-5810> References: <20171027163830.28498.82997.stgit@john-Precision-Tower-5810> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Recent additions to support multiple programs in cgroups impose a strict requirement, "all yes is yes, any no is no". To enforce this the infrastructure requires the 'no' return code, SK_DROP in this case, to be 0. To apply these rules to SK_SKB program types the sk_actions return codes need to be adjusted. This fix adds SK_PASS and makes 'SK_DROP = 0'. Finally, remove SK_ABORTED to remove any chance that the API may allow aborted program flows to be passed up the stack. This would be incorrect behavior and allow programs to break existing policies. Signed-off-by: John Fastabend Acked-by: Alexei Starovoitov --- include/uapi/linux/bpf.h | 6 +++--- kernel/bpf/sockmap.c | 3 ++- net/core/filter.c | 5 +++-- tools/include/uapi/linux/bpf.h | 4 ++-- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index f90860d..0d7948c 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -575,7 +575,7 @@ enum bpf_attach_type { * @map: pointer to sockmap * @key: key to lookup sock in map * @flags: reserved for future use - * Return: SK_REDIRECT + * Return: SK_PASS * * int bpf_sock_map_update(skops, map, key, flags) * @skops: pointer to bpf_sock_ops @@ -786,8 +786,8 @@ struct xdp_md { }; enum sk_action { - SK_ABORTED = 0, - SK_DROP, + SK_DROP = 0, + SK_PASS, SK_REDIRECT, }; diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c index 6778fb7..66f00a2 100644 --- a/kernel/bpf/sockmap.c +++ b/kernel/bpf/sockmap.c @@ -122,7 +122,8 @@ static int smap_verdict_func(struct smap_psock *psock, struct sk_buff *skb) preempt_enable(); skb->sk = NULL; - return rc; + return rc == SK_PASS ? + (TCP_SKB_CB(skb)->bpf.map ? SK_REDIRECT : SK_PASS) : SK_DROP; } static void smap_do_verdict(struct smap_psock *psock, struct sk_buff *skb) diff --git a/net/core/filter.c b/net/core/filter.c index 68eaa2f..6ae94f8 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -1844,14 +1844,15 @@ int skb_do_redirect(struct sk_buff *skb) { struct tcp_skb_cb *tcb = TCP_SKB_CB(skb); + /* If user passes invalid input drop the packet. */ if (unlikely(flags)) - return SK_ABORTED; + return SK_DROP; tcb->bpf.key = key; tcb->bpf.flags = flags; tcb->bpf.map = map; - return SK_REDIRECT; + return SK_PASS; } struct sock *do_sk_redirect_map(struct sk_buff *skb) diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index 24b35a1..c174971 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -787,8 +787,8 @@ struct xdp_md { }; enum sk_action { - SK_ABORTED = 0, - SK_DROP, + SK_DROP = 0, + SK_PASS, SK_REDIRECT, };