mirror of
https://github.com/torvalds/linux.git
synced 2026-11-06 21:58:01 +01:00
bpf, sockmap: Fix psock update-side data races on sk->sk_prot and progs
The update side of the sockmap psock lifecycle has several unsynchronized accesses to sk->sk_prot and psock prog state that KCSAN flags when the new update-vs-delete race selftest runs. The fix comes from Alexei [1]: * sock_map_link() installed psock programs and called sock_map_init_proto() before taking sk->sk_callback_lock, racing a concurrent teardown's tcp_bpf_update_proto() write of sk->sk_prot. Take the write_lock early, before psock_set_prog() and sock_map_init_proto(), and drop the psock->saved_data_ready gating from prog detection (it is re-checked under the lock before stopping progs). * sock_map_del_link() read psock->saved_data_ready outside the lock. Move the saved_data_ready check under write_lock_bh(&sk->sk_callback_lock) so prog teardown is serialized against the proto restore. * sock_map_psock_get_checked() and sock_map_sk_is_suitable() did plain sk->sk_prot reads. Convert to READ_ONCE() to mark the intentional racy read against the WRITE_ONCE() swap in sock_replace_proto(). With these changes the sock_map_link/psock_set_prog side of the reported KCSAN splats is silenced; the remaining teardown-vs-core-TCP pairings are the separate read-side annotation problem. [1] https://lore.kernel.org/bpf/DMBHB0TQOV1L.38MECMYFGUS5B@gmail.com Suggested-by: Alexei Starovoitov <ast@kernel.org> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> Signed-off-by: Alexei Starovoitov <ast@kernel.org> Link: https://patch.msgid.link/20261030-bpf-next-v1-2-035eabf8eae7@cloudflare.com
This commit is contained in:
parent
23503acb21
commit
bcd07fd167
|
|
@ -152,11 +152,11 @@ static void sock_map_del_link(struct sock *sk,
|
|||
struct bpf_map *map = link->map;
|
||||
struct sk_psock_progs *progs = sock_map_progs(map);
|
||||
|
||||
if (psock->saved_data_ready && progs->stream_parser)
|
||||
if (progs->stream_parser)
|
||||
strp_stop = true;
|
||||
if (psock->saved_data_ready && progs->stream_verdict)
|
||||
if (progs->stream_verdict)
|
||||
verdict_stop = true;
|
||||
if (psock->saved_data_ready && progs->skb_verdict)
|
||||
if (progs->skb_verdict)
|
||||
verdict_stop = true;
|
||||
list_del(&link->list);
|
||||
sk_psock_free_link(link);
|
||||
|
|
@ -164,8 +164,11 @@ static void sock_map_del_link(struct sock *sk,
|
|||
}
|
||||
}
|
||||
spin_unlock_bh(&psock->link_lock);
|
||||
if (strp_stop || verdict_stop) {
|
||||
write_lock_bh(&sk->sk_callback_lock);
|
||||
if (!strp_stop && !verdict_stop)
|
||||
return;
|
||||
|
||||
write_lock_bh(&sk->sk_callback_lock);
|
||||
if (psock->saved_data_ready) {
|
||||
if (strp_stop)
|
||||
sk_psock_stop_strp(sk, psock);
|
||||
if (verdict_stop)
|
||||
|
|
@ -173,8 +176,8 @@ static void sock_map_del_link(struct sock *sk,
|
|||
|
||||
if (psock->psock_update_sk_prot)
|
||||
psock->psock_update_sk_prot(sk, psock, false);
|
||||
write_unlock_bh(&sk->sk_callback_lock);
|
||||
}
|
||||
write_unlock_bh(&sk->sk_callback_lock);
|
||||
}
|
||||
|
||||
static void sock_map_unref(struct sock *sk, void *link_raw)
|
||||
|
|
@ -202,7 +205,7 @@ static struct sk_psock *sock_map_psock_get_checked(struct sock *sk)
|
|||
rcu_read_lock();
|
||||
psock = sk_psock(sk);
|
||||
if (psock) {
|
||||
if (sk->sk_prot->close != sock_map_close) {
|
||||
if (READ_ONCE(sk->sk_prot)->close != sock_map_close) {
|
||||
psock = ERR_PTR(-EBUSY);
|
||||
goto out;
|
||||
}
|
||||
|
|
@ -284,6 +287,7 @@ static int sock_map_link(struct bpf_map *map, struct sock *sk)
|
|||
}
|
||||
}
|
||||
|
||||
write_lock_bh(&sk->sk_callback_lock);
|
||||
if (msg_parser)
|
||||
psock_set_prog(&psock->progs.msg_parser, msg_parser);
|
||||
if (stream_parser)
|
||||
|
|
@ -298,11 +302,11 @@ static int sock_map_link(struct bpf_map *map, struct sock *sk)
|
|||
*/
|
||||
ret = sock_map_init_proto(sk, psock);
|
||||
if (ret < 0) {
|
||||
write_unlock_bh(&sk->sk_callback_lock);
|
||||
sk_psock_put(sk, psock);
|
||||
goto out;
|
||||
}
|
||||
|
||||
write_lock_bh(&sk->sk_callback_lock);
|
||||
if (stream_parser && stream_verdict && !psock->saved_data_ready) {
|
||||
if (sk_is_tcp(sk))
|
||||
ret = sk_psock_init_strp(sk, psock);
|
||||
|
|
@ -536,7 +540,7 @@ static bool sock_map_redirect_allowed(const struct sock *sk)
|
|||
|
||||
static bool sock_map_sk_is_suitable(const struct sock *sk)
|
||||
{
|
||||
return !!sk->sk_prot->psock_update_sk_prot;
|
||||
return !!READ_ONCE(sk->sk_prot)->psock_update_sk_prot;
|
||||
}
|
||||
|
||||
static bool sock_map_sk_state_allowed(const struct sock *sk)
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user