bpf, sockmap: Attach map progs to psock early for feature probes

When a TCP socket is added to a sock map we look at the programs attached
to the map to determine what proto op hooks need to be changed. Before
the patch in the 'fixes' tag there were only two categories -- the empty
set of programs or a TX policy. In any case the base set handled the
receive case.

After the fix we have an optimized program for receive that closes a small,
but possible, race on receive. This program is loaded only when the map the
psock is being added to includes a RX policy. Otherwise, the race is not
possible so we don't need to handle the race condition.

In order for the call to sk_psock_init() to correctly evaluate the above
conditions all progs need to be set in the psock before the call. However,
in the current code this is not the case. We end up evaluating the
requirements on the old prog state. If your psock is attached to multiple
maps -- for example a tx map and rx map -- then the second update would pull
in the correct maps. But, the other pattern with a single rx enabled map
the correct receive hooks are not used. The result is the race fixed by the
patch in the fixes tag below may still be seen in this case.

To fix we simply set all psock->progs before doing the call into
sock_map_init(). With this the init() call gets the full list of programs
and chooses the correct proto ops on the first iteration instead of
requiring the second update to pull them in. This fixes the race case when
only a single map is used.

Fixes: c5d2177a72a16 ("bpf, sockmap: Fix race in ingress receive verdict with redirect to self")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/bpf/20211119181418.353932-2-john.fastabend@gmail.com

diff --git a/net/core/sock_map.c b/net/core/sock_map.c
index f39ef79ced679dd19b2b3746363a7b9e6ef7b047..9b528c644fb7d5439b40c581cbeafa145c093ad5 100644 (file)
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c
@@ -282,6 +282,12 @@ static int sock_map_link(struct bpf_map *map, struct sock *sk)
 
        if (msg_parser)
                psock_set_prog(&psock->progs.msg_parser, msg_parser);
+       if (stream_parser)
+               psock_set_prog(&psock->progs.stream_parser, stream_parser);
+       if (stream_verdict)
+               psock_set_prog(&psock->progs.stream_verdict, stream_verdict);
+       if (skb_verdict)
+               psock_set_prog(&psock->progs.skb_verdict, skb_verdict);
 
        ret = sock_map_init_proto(sk, psock);
        if (ret < 0)
@@ -292,14 +298,10 @@ static int sock_map_link(struct bpf_map *map, struct sock *sk)
                ret = sk_psock_init_strp(sk, psock);
                if (ret)
                        goto out_unlock_drop;
-               psock_set_prog(&psock->progs.stream_verdict, stream_verdict);
-               psock_set_prog(&psock->progs.stream_parser, stream_parser);
                sk_psock_start_strp(sk, psock);
        } else if (!stream_parser && stream_verdict && !psock->saved_data_ready) {
-               psock_set_prog(&psock->progs.stream_verdict, stream_verdict);
                sk_psock_start_verdict(sk,psock);
        } else if (!stream_verdict && skb_verdict && !psock->saved_data_ready) {
-               psock_set_prog(&psock->progs.skb_verdict, skb_verdict);
                sk_psock_start_verdict(sk, psock);
        }
        write_unlock_bh(&sk->sk_callback_lock);