From 7592284998074c80e40e7d3e22f183ea764e85af Mon Sep 17 00:00:00 2001 From: John Fastabend Date: Tue, 9 May 2023 20:14:20 +0800 Subject: [PATCH] bpf, sockmap: Attach map progs to psock early for feature probes mainline inclusion from mainline-v5.16-rc5 commit 38207a5e81230d6ffbdd51e5fa5681be5116dcae category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/I65HYE Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=38207a5e81230d6ffbdd51e5fa5681be5116dcae --------------------------- 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 Signed-off-by: Daniel Borkmann Link: https://lore.kernel.org/bpf/20211119181418.353932-2-john.fastabend@gmail.com (cherry picked from commit 38207a5e81230d6ffbdd51e5fa5681be5116dcae) Signed-off-by: Liu Jian Conflicts: net/core/sock_map.c Reviewed-by: Yue Haibing Signed-off-by: Jialin Zhang --- net/core/sock_map.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/net/core/sock_map.c b/net/core/sock_map.c index 79528310e6a7..9cdb5b072b3d 100644 --- a/net/core/sock_map.c +++ b/net/core/sock_map.c @@ -286,6 +286,10 @@ static int sock_map_link(struct bpf_map *map, struct sk_psock_progs *progs, if (msg_parser) psock_set_prog(&psock->progs.msg_parser, msg_parser); + if (skb_parser) + psock_set_prog(&psock->progs.skb_parser, skb_parser); + if (skb_verdict) + psock_set_prog(&psock->progs.skb_verdict, skb_verdict); ret = sock_map_init_proto(sk, psock); if (ret < 0) @@ -296,11 +300,8 @@ static int sock_map_link(struct bpf_map *map, struct sk_psock_progs *progs, ret = sk_psock_init_strp(sk, psock); if (ret) goto out_unlock_drop; - psock_set_prog(&psock->progs.skb_verdict, skb_verdict); - psock_set_prog(&psock->progs.skb_parser, skb_parser); sk_psock_start_strp(sk, psock); } else if (!skb_parser && skb_verdict && !psock->parser.enabled) { - psock_set_prog(&psock->progs.skb_verdict, skb_verdict); sk_psock_start_verdict(sk,psock); } write_unlock_bh(&sk->sk_callback_lock); -- GitLab