Skip to content

Commit

Permalink
bpf, sockmap: Use stricter sk state checks in sk_lookup_assign
Browse files Browse the repository at this point in the history
In order to fix an issue with sockets in TCP sockmap redirect cases we plan
to allow CLOSE state sockets to exist in the sockmap. However, the check in
bpf_sk_lookup_assign() currently only invalidates sockets in the
TCP_ESTABLISHED case relying on the checks on sockmap insert to ensure we
never SOCK_CLOSE state sockets in the map.

To prepare for this change we flip the logic in bpf_sk_lookup_assign() to
explicitly test for the accepted cases. Namely, a tcp socket in TCP_LISTEN
or a udp socket in TCP_CLOSE state. This also makes the code more resilent
to future changes.

Suggested-by: Jakub Sitnicki <jakub@cloudflare.com>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Reviewed-by: Jakub Sitnicki <jakub@cloudflare.com>
Link: https://lore.kernel.org/bpf/20211103204736.248403-2-john.fastabend@gmail.com
  • Loading branch information
jrfastab authored and borkmann committed Nov 8, 2021
1 parent 47b3708 commit 40a3412
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 8 deletions.
12 changes: 12 additions & 0 deletions include/linux/skmsg.h
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,18 @@ static inline bool sk_psock_strp_enabled(struct sk_psock *psock)
return !!psock->saved_data_ready;
}

static inline bool sk_is_tcp(const struct sock *sk)
{
return sk->sk_type == SOCK_STREAM &&
sk->sk_protocol == IPPROTO_TCP;
}

static inline bool sk_is_udp(const struct sock *sk)
{
return sk->sk_type == SOCK_DGRAM &&
sk->sk_protocol == IPPROTO_UDP;
}

#if IS_ENABLED(CONFIG_NET_SOCK_MSG)

#define BPF_F_STRPARSER (1UL << 1)
Expand Down
6 changes: 4 additions & 2 deletions net/core/filter.c
Original file line number Diff line number Diff line change
Expand Up @@ -10423,8 +10423,10 @@ BPF_CALL_3(bpf_sk_lookup_assign, struct bpf_sk_lookup_kern *, ctx,
return -EINVAL;
if (unlikely(sk && sk_is_refcounted(sk)))
return -ESOCKTNOSUPPORT; /* reject non-RCU freed sockets */
if (unlikely(sk && sk->sk_state == TCP_ESTABLISHED))
return -ESOCKTNOSUPPORT; /* reject connected sockets */
if (unlikely(sk && sk_is_tcp(sk) && sk->sk_state != TCP_LISTEN))
return -ESOCKTNOSUPPORT; /* only accept TCP socket in LISTEN */
if (unlikely(sk && sk_is_udp(sk) && sk->sk_state != TCP_CLOSE))
return -ESOCKTNOSUPPORT; /* only accept UDP socket in CLOSE */

/* Check if socket is suitable for packet L3/L4 protocol */
if (sk && sk->sk_protocol != ctx->protocol)
Expand Down
6 changes: 0 additions & 6 deletions net/core/sock_map.c
Original file line number Diff line number Diff line change
Expand Up @@ -511,12 +511,6 @@ static bool sock_map_op_okay(const struct bpf_sock_ops_kern *ops)
ops->op == BPF_SOCK_OPS_TCP_LISTEN_CB;
}

static bool sk_is_tcp(const struct sock *sk)
{
return sk->sk_type == SOCK_STREAM &&
sk->sk_protocol == IPPROTO_TCP;
}

static bool sock_map_redirect_allowed(const struct sock *sk)
{
if (sk_is_tcp(sk))
Expand Down

0 comments on commit 40a3412

Please sign in to comment.