From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.9 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7B36BC2D0DB for ; Wed, 22 Jan 2020 13:06:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 45B3524683 for ; Wed, 22 Jan 2020 13:06:03 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=cloudflare.com header.i=@cloudflare.com header.b="Hg2SMdcH" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729195AbgAVNGC (ORCPT ); Wed, 22 Jan 2020 08:06:02 -0500 Received: from mail-lf1-f66.google.com ([209.85.167.66]:46487 "EHLO mail-lf1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729137AbgAVNGB (ORCPT ); Wed, 22 Jan 2020 08:06:01 -0500 Received: by mail-lf1-f66.google.com with SMTP id z26so5223515lfg.13 for ; Wed, 22 Jan 2020 05:06:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cloudflare.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=3Tuq3upHhrC9mfKoTXSq3gzV/lZ6UhhXE1I0CiNF+d4=; b=Hg2SMdcHrmbB+Rjj9qm8DzLYnMmLo9p6Y36i7FERwDjnRYHWSFwQTv26jjPZBDeIT3 2x3+1D5Fy4lcJcktHM5KNqzyWx2sfQauk1BOM5OT9K22Wlz5pCeeYb3k30A8IV5/721W KkuGbGjXl3eSsZt3o9lpgxNlTnocHZJ2hA6lw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=3Tuq3upHhrC9mfKoTXSq3gzV/lZ6UhhXE1I0CiNF+d4=; b=VzM/RmYNMe8AHV4vIC18p+b0S06UsXHVHu/jgO5nzzCod7yX2x8MukZSybzgFc37Gy zJqRHJANxqWRAID2vIju5B8y7Fp+9TIXtD8wvZM2YQUse/Px3m52McKaHhWmvtbhrl6J DrWJab9WzH1rn9pTYIIQ7NObw1ZPQ1SLYyu/OgAAx40qXisUZGeaAJfIG6U1xf6p0SEm C738JzISufYmBM/Ksk3einRqb5zHN4W038/kM/yHbRs4lj+Z9DQGvhG24yLQ8arNXMYl 10Sw71K6kPJ2cuRresee0T+ZQtiYChGzRJmV8vkJYmfMk7r3kJpcR4HOMKxrgG9Pc0fz ZtIQ== X-Gm-Message-State: APjAAAXF8xd35gndpl64jiSfCJywEab+DtW106/93/YstbWc9dXHXrb7 Upu2tZfv8suOQ2vN3QaEYFH69tb3sBkTJA== X-Google-Smtp-Source: APXvYqw2aIUZt086l9I7n69eaN/oox0LrAUoEnniiu+OQXpG/62yxRUgImZoCioUFn3+Qba9XwXGxA== X-Received: by 2002:ac2:58fa:: with SMTP id v26mr1757005lfo.84.1579698358952; Wed, 22 Jan 2020 05:05:58 -0800 (PST) Received: from cloudflare.com ([2a02:a310:c262:aa00:b35e:8938:2c2a:ba8b]) by smtp.gmail.com with ESMTPSA id t1sm20274973lji.98.2020.01.22.05.05.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Jan 2020 05:05:58 -0800 (PST) From: Jakub Sitnicki To: bpf@vger.kernel.org Cc: netdev@vger.kernel.org, kernel-team@cloudflare.com, Alexei Starovoitov , Daniel Borkmann , John Fastabend , Lorenz Bauer , Martin Lau Subject: [PATCH bpf-next v3 05/12] bpf, sockmap: Allow inserting listening TCP sockets into sockmap Date: Wed, 22 Jan 2020 14:05:42 +0100 Message-Id: <20200122130549.832236-6-jakub@cloudflare.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200122130549.832236-1-jakub@cloudflare.com> References: <20200122130549.832236-1-jakub@cloudflare.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: bpf-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org In order for sockmap type to become a generic collection for storing TCP sockets we need to loosen the checks during map update, while tightening the checks in redirect helpers. Currently sockmap requires the TCP socket to be in established state (or transitioning out of SYN_RECV into established state when done from BPF), which prevents inserting listening sockets. Change the update pre-checks so the socket can also be in listening state. Since it doesn't make sense to redirect with sockmap to listening sockets, add appropriate socket state checks to BPF redirect helpers too. We leave sockhash as is for the moment, with no support for holding listening sockets. Therefore sockhash needs its own set of checks. Signed-off-by: Jakub Sitnicki --- net/core/sock_map.c | 62 +++++++++++++++++++------ tools/testing/selftests/bpf/test_maps.c | 6 +-- 2 files changed, 50 insertions(+), 18 deletions(-) diff --git a/net/core/sock_map.c b/net/core/sock_map.c index eb114ee419b6..97bdceb29f09 100644 --- a/net/core/sock_map.c +++ b/net/core/sock_map.c @@ -385,15 +385,44 @@ static int sock_map_update_common(struct bpf_map *map, u32 idx, } static bool sock_map_op_okay(const struct bpf_sock_ops_kern *ops) +{ + return ops->op == BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB || + ops->op == BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB || + ops->op == BPF_SOCK_OPS_TCP_LISTEN_CB; +} + +static bool sock_hash_op_okay(const struct bpf_sock_ops_kern *ops) { return ops->op == BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB || ops->op == BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB; } +/* Only TCP sockets can be inserted into the map. They must be either + * in established or listening state. SYN_RECV is also allowed because + * BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB happens just before socket + * enters established state. + */ static bool sock_map_sk_is_suitable(const struct sock *sk) { return sk->sk_type == SOCK_STREAM && - sk->sk_protocol == IPPROTO_TCP; + sk->sk_protocol == IPPROTO_TCP && + (1 << sk->sk_state) & (TCPF_ESTABLISHED | + TCPF_SYN_RECV | + TCPF_LISTEN); +} + +static bool sock_hash_sk_is_suitable(const struct sock *sk) +{ + return sk->sk_type == SOCK_STREAM && + sk->sk_protocol == IPPROTO_TCP && + (1 << sk->sk_state) & (TCPF_ESTABLISHED | + TCPF_SYN_RECV); +} + +/* Is sock in a state that allows redirecting into it? */ +static bool sock_map_redirect_okay(const struct sock *sk) +{ + return sk->sk_state != TCP_LISTEN; } static int sock_map_update_elem(struct bpf_map *map, void *key, @@ -413,8 +442,7 @@ static int sock_map_update_elem(struct bpf_map *map, void *key, ret = -EINVAL; goto out; } - if (!sock_map_sk_is_suitable(sk) || - sk->sk_state != TCP_ESTABLISHED) { + if (!sock_map_sk_is_suitable(sk)) { ret = -EOPNOTSUPP; goto out; } @@ -454,13 +482,17 @@ BPF_CALL_4(bpf_sk_redirect_map, struct sk_buff *, skb, struct bpf_map *, map, u32, key, u64, flags) { struct tcp_skb_cb *tcb = TCP_SKB_CB(skb); + struct sock *sk; if (unlikely(flags & ~(BPF_F_INGRESS))) return SK_DROP; - tcb->bpf.flags = flags; - tcb->bpf.sk_redir = __sock_map_lookup_elem(map, key); - if (!tcb->bpf.sk_redir) + + sk = __sock_map_lookup_elem(map, key); + if (unlikely(!sk || !sock_map_redirect_okay(sk))) return SK_DROP; + + tcb->bpf.flags = flags; + tcb->bpf.sk_redir = sk; return SK_PASS; } @@ -477,12 +509,17 @@ const struct bpf_func_proto bpf_sk_redirect_map_proto = { BPF_CALL_4(bpf_msg_redirect_map, struct sk_msg *, msg, struct bpf_map *, map, u32, key, u64, flags) { + struct sock *sk; + if (unlikely(flags & ~(BPF_F_INGRESS))) return SK_DROP; - msg->flags = flags; - msg->sk_redir = __sock_map_lookup_elem(map, key); - if (!msg->sk_redir) + + sk = __sock_map_lookup_elem(map, key); + if (unlikely(!sk || !sock_map_redirect_okay(sk))) return SK_DROP; + + msg->flags = flags; + msg->sk_redir = sk; return SK_PASS; } @@ -736,8 +773,7 @@ static int sock_hash_update_elem(struct bpf_map *map, void *key, ret = -EINVAL; goto out; } - if (!sock_map_sk_is_suitable(sk) || - sk->sk_state != TCP_ESTABLISHED) { + if (!sock_hash_sk_is_suitable(sk)) { ret = -EOPNOTSUPP; goto out; } @@ -882,8 +918,8 @@ BPF_CALL_4(bpf_sock_hash_update, struct bpf_sock_ops_kern *, sops, { WARN_ON_ONCE(!rcu_read_lock_held()); - if (likely(sock_map_sk_is_suitable(sops->sk) && - sock_map_op_okay(sops))) + if (likely(sock_hash_sk_is_suitable(sops->sk) && + sock_hash_op_okay(sops))) return sock_hash_update_common(map, key, sops->sk, flags); return -EOPNOTSUPP; } diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c index 02eae1e864c2..c6766b2cff85 100644 --- a/tools/testing/selftests/bpf/test_maps.c +++ b/tools/testing/selftests/bpf/test_maps.c @@ -756,11 +756,7 @@ static void test_sockmap(unsigned int tasks, void *data) /* Test update without programs */ for (i = 0; i < 6; i++) { err = bpf_map_update_elem(fd, &i, &sfd[i], BPF_ANY); - if (i < 2 && !err) { - printf("Allowed update sockmap '%i:%i' not in ESTABLISHED\n", - i, sfd[i]); - goto out_sockmap; - } else if (i >= 2 && err) { + if (err) { printf("Failed noprog update sockmap '%i:%i'\n", i, sfd[i]); goto out_sockmap; -- 2.24.1