[PATCH bpf-next v2 05/11] bpf, sockmap: Allow inserting listening TCP sockets into sockmap
From: Jakub Sitnicki <jakub@cloudflare.com>
Date: 2020-01-10 10:50:49
Also in:
bpf
Subsystem:
bpf [general] (safe dynamic programs and tools), bpf [l7 framework] (sockmap), bpf [selftests] (test runners & infrastructure), kernel selftest framework, networking [general], networking [sockets], the rest · Maintainers:
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, John Fastabend, Jakub Sitnicki, Jiayuan Chen, Shuah Khan, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Kuniyuki Iwashima, Willem de Bruijn, Linus Torvalds
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 that the socket can also be in listening state. If the state is not white-listed, return -EINVAL to be consistent with REUSEPORT_SOCKARRY map type. Since it doesn't make sense to redirect with sockmap to listening sockets, add appropriate socket state checks to BPF redirect helpers too. Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> --- net/core/sock_map.c | 46 ++++++++++++++++++++----- tools/testing/selftests/bpf/test_maps.c | 6 +--- 2 files changed, 39 insertions(+), 13 deletions(-)
diff --git a/net/core/sock_map.c b/net/core/sock_map.c
index eb114ee419b6..99daea502508 100644
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c@@ -396,6 +396,23 @@ static bool sock_map_sk_is_suitable(const struct sock *sk) sk->sk_protocol == IPPROTO_TCP; } +/* Is sock in a state that allows inserting into the map? + * SYN_RECV is needed for updates on BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB. + */ +static bool sock_map_update_okay(const struct sock *sk) +{ + return (1 << sk->sk_state) & (TCPF_ESTABLISHED | + TCPF_SYN_RECV | + TCPF_LISTEN); +} + +/* Is sock in a state that allows redirecting into it? */ +static bool sock_map_redirect_okay(const struct sock *sk) +{ + return (1 << sk->sk_state) & (TCPF_ESTABLISHED | + TCPF_SYN_RECV); +} + static int sock_map_update_elem(struct bpf_map *map, void *key, void *value, u64 flags) {
@@ -413,11 +430,14 @@ 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; } + if (!sock_map_update_okay(sk)) { + ret = -EINVAL; + goto out; + } sock_map_sk_acquire(sk); ret = sock_map_update_common(map, idx, sk, flags);
@@ -433,6 +453,7 @@ BPF_CALL_4(bpf_sock_map_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_update_okay(sops->sk) && sock_map_op_okay(sops))) return sock_map_update_common(map, *(u32 *)key, sops->sk, flags);
@@ -454,13 +475,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 (!sk || !sock_map_redirect_okay(sk)) return SK_DROP; + + tcb->bpf.flags = flags; + tcb->bpf.sk_redir = sk; return SK_PASS; }
@@ -477,12 +502,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 (!sk || !sock_map_redirect_okay(sk)) return SK_DROP; + + msg->flags = flags; + msg->sk_redir = sk; return SK_PASS; }
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