| Message ID | 20180608150644.15153.4135.stgit@john-Precision-Tower-5810 |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | bpf, sockmap IPv6/TCP state fixes | expand |
Hi John, On 06/08/2018 05:06 PM, John Fastabend wrote: > Per the note in the TLS ULP (which is actually a generic statement > regarding ULPs) > > /* The TLS ulp is currently supported only for TCP sockets > * in ESTABLISHED state. > * Supporting sockets in LISTEN state will require us > * to modify the accept implementation to clone rather then > * share the ulp context. > */ > > After this patch we only allow socks that are in ESTABLISHED state or > are being added via a sock_ops event that is transitioning into an > ESTABLISHED state. By allowing sock_ops events we allow users to > manage sockmaps directly from sock ops programs. The two supported > sock_ops ops are BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB and > BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB. > > Also tested with 'netserver -6' and 'netperf -H [IPv6]' as well as > 'netperf -H [IPv4]'. > > Reported-by: Eric Dumazet <edumazet@google.com> > Signed-off-by: John Fastabend <john.fastabend@gmail.com> Please also add a Fixes tag to this one. Ok to just reply with one. Thanks, Daniel
On 06/08/2018 08:06 AM, John Fastabend wrote: > Per the note in the TLS ULP (which is actually a generic statement > regarding ULPs) > > /* The TLS ulp is currently supported only for TCP sockets > * in ESTABLISHED state. > * Supporting sockets in LISTEN state will require us > * to modify the accept implementation to clone rather then > * share the ulp context. > */ > > After this patch we only allow socks that are in ESTABLISHED state or > are being added via a sock_ops event that is transitioning into an > ESTABLISHED state. By allowing sock_ops events we allow users to > manage sockmaps directly from sock ops programs. The two supported > sock_ops ops are BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB and > BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB. > > Also tested with 'netserver -6' and 'netperf -H [IPv6]' as well as > 'netperf -H [IPv4]'. > > Reported-by: Eric Dumazet <edumazet@google.com> > Signed-off-by: John Fastabend <john.fastabend@gmail.com> > --- Fixes: 174a79ff9515 ("bpf: sockmap with sk redirect support")
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c index fa9b7f3..4921fb7 100644 --- a/kernel/bpf/sockmap.c +++ b/kernel/bpf/sockmap.c @@ -1956,8 +1956,12 @@ static int sock_map_update_elem(struct bpf_map *map, return -EINVAL; } + /* ULPs are currently supported only for TCP sockets in ESTABLISHED + * state. + */ if (skops.sk->sk_type != SOCK_STREAM || - skops.sk->sk_protocol != IPPROTO_TCP) { + skops.sk->sk_protocol != IPPROTO_TCP || + skops.sk->sk_state != TCP_ESTABLISHED) { fput(socket->file); return -EOPNOTSUPP; } @@ -2318,6 +2322,16 @@ static int sock_hash_update_elem(struct bpf_map *map, return -EINVAL; } + /* ULPs are currently supported only for TCP sockets in ESTABLISHED + * state. + */ + if (skops.sk->sk_type != SOCK_STREAM || + skops.sk->sk_protocol != IPPROTO_TCP || + skops.sk->sk_state != TCP_ESTABLISHED) { + fput(socket->file); + return -EOPNOTSUPP; + } + err = sock_hash_ctx_update_elem(&skops, map, key, flags); fput(socket->file); return err; @@ -2403,10 +2417,23 @@ struct sock *__sock_hash_lookup_elem(struct bpf_map *map, void *key) .map_delete_elem = sock_hash_delete_elem, }; +static bool bpf_is_valid_sock(struct bpf_sock_ops_kern *ops) +{ + return ops->op == BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB || + ops->op == BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB; +} + BPF_CALL_4(bpf_sock_map_update, struct bpf_sock_ops_kern *, bpf_sock, struct bpf_map *, map, void *, key, u64, flags) { WARN_ON_ONCE(!rcu_read_lock_held()); + + /* ULPs are currently supported only for TCP sockets in ESTABLISHED + * state. This checks that the sock ops triggering the update is + * one indicating we are (or will be soon) in an ESTABLISHED state. + */ + if (!bpf_is_valid_sock(bpf_sock)) + return -EOPNOTSUPP; return sock_map_ctx_update_elem(bpf_sock, map, key, flags); } @@ -2425,6 +2452,9 @@ struct sock *__sock_hash_lookup_elem(struct bpf_map *map, void *key) struct bpf_map *, map, void *, key, u64, flags) { WARN_ON_ONCE(!rcu_read_lock_held()); + + if (!bpf_is_valid_sock(bpf_sock)) + return -EOPNOTSUPP; return sock_hash_ctx_update_elem(bpf_sock, map, key, flags); }
Per the note in the TLS ULP (which is actually a generic statement regarding ULPs) /* The TLS ulp is currently supported only for TCP sockets * in ESTABLISHED state. * Supporting sockets in LISTEN state will require us * to modify the accept implementation to clone rather then * share the ulp context. */ After this patch we only allow socks that are in ESTABLISHED state or are being added via a sock_ops event that is transitioning into an ESTABLISHED state. By allowing sock_ops events we allow users to manage sockmaps directly from sock ops programs. The two supported sock_ops ops are BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB and BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB. Also tested with 'netserver -6' and 'netperf -H [IPv6]' as well as 'netperf -H [IPv4]'. Reported-by: Eric Dumazet <edumazet@google.com> Signed-off-by: John Fastabend <john.fastabend@gmail.com> --- kernel/bpf/sockmap.c | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-)