bpf, sockmap: Fix psock incorrectly pointing to sk
Subsystems (4)
Jiayuan Chen
maintainerPATCH v2 1/1 bpf, sockmap: Fix psock incorrectly pointing to sk
We observed an issue from the latest selftest: sockmap_redir where sk_psock(psock->sk) != psock in the backlog. The root cause is the special behavior in sockmap_redir - it frequently performs map_update() and map_delete() on the same socket. During map_update(), we create a new psock and during map_delete(), we eventually free the psock via rcu_work in sk_psock_drop(). However, pending workqueues might still exist and not be processed yet. If users immediately perform another map_update(), a new psock will be allocated for the same sk, resulting in two psocks pointing to the same sk.
When the pending workqueue is later triggered, it uses the old psock to access sk for I/O operations, which is incorrect.
Timing Diagram:
cpu0 cpu1
map_update(sk):
sk->psock = psock1
psock1->sk = skmap_delete(sk):
rcu_work_free(psock1)
map_update(sk):
sk->psock = psock2
psock2->sk = sk
workqueue:
wakeup with psock1, but the sk of psock1 doesn't belong to psock1rcu_handler:
clean psock1
free(psock1)
Previously, we used reference counting to address the concurrency issue between backlog and sock_map_close(). This logic remains necessary as it prevents the sk from being freed while processing the backlog. But this patch prevents pending backlogs from using a psock after it has been stopped.
Note: We cannot call cancel_delayed_work_sync() in map_delete() since this might be invoked in BPF context by BPF helper, and the function may sleep.
---V1->V2: Use existing flag instead of adding new one.
https://lore.kernel.org/bpf/[email protected]/
Thanks to Michal Luczaj for providing the sockmap_redir test case, which indeed covers almost all sockmap forwarding paths.
--- net/core/skmsg.c | 7 +++++++ 1 file changed, 7 insertions(+)
net/core/skmsg.c +7 -0
@@ -656,6 +656,13 @@ static void sk_psock_backlog(struct work_struct *work)bool ingress;int ret; +/* If sk is quickly removed from the map and then added back, the old +* psock should not be scheduled, because there are now two psocks +* pointing to the same sk. +*/ +if (!sk_psock_test_state (psock,SK_PSOCK_TX_ENABLED )) +return ; +/* Increment the psock refcnt to synchronize with close(fd) path in * sock_map_close(), ensuring we wait for backlog thread completion * before sk_socket freed. If refcnt increment fails, it indicates
Signature · 1 line
2.47.1
Quoted 12 lines · Jiayuan Chen
We observed an issue from the latest selftest:
sockmap_redirwheresk_psock(psock->sk)!= psock in the backlog. The root cause is the special behavior insockmap_redir- it frequently performsmap_update()andmap_delete()on the same socket. Duringmap_update(), we create a new psock and duringmap_delete(), we eventually free the psock viarcu_workinsk_psock_drop(). However, pending workqueues might still exist and not be processed yet. If users immediately perform anothermap_update(), a new psock will be allocated for the same sk, resulting in two psocks pointing to the same sk.When the pending workqueue is later triggered, it uses the old psock to access sk for I/O operations, which is incorrect.
[...]
Quoted 11 lines
Note: We cannot call
cancel_delayed_work_sync()inmap_delete()since this might be invoked in BPF context by BPF helper, and the function may sleep.Fixes 604326b41a6f ("bpf, sockmap: convert to generic sk_msg interface")Signed-off-by Jiayuan Chen <[email protected]>---
V1->V2: Use existing flag instead of adding new one.
https://lore.kernel.org/bpf/[email protected]/
Quoted 6 lines
Thanks to Michal Luczaj for providing the
sockmap_redirtest case, which indeed covers almost all sockmap forwarding paths.--- net/core/skmsg.c | 7 +++++++ 1 file changed, 7 insertions(+)
This patch was applied to bpf/bpf-next.git (net) by Daniel Borkmann <[email protected]>:
Hello:
This patch was applied to bpf/bpf-next.git (net)
by Daniel Borkmann <[email protected]>:
Quoted 11 lines · On Mon, 9 Jun 2025 10:59:08 +0800 you wrote:
We observed an issue from the latest selftest:
sockmap_redirwheresk_psock(psock->sk)!= psock in the backlog. The root cause is the special behavior insockmap_redir- it frequently performsmap_update()andmap_delete()on the same socket. Duringmap_update(), we create a new psock and duringmap_delete(), we eventually free the psock viarcu_workinsk_psock_drop(). However, pending workqueues might still exist and not be processed yet. If users immediately perform anothermap_update(), a new psock will be allocated for the same sk, resulting in two psocks pointing to the same sk.[...]
Here is the summary with links:
- [bpf-next,v2] bpf, sockmap: Fix psock incorrectly pointing to sk
https://git.kernel.org/bpf/bpf-next/c/76be5fae32fe
You are awesome, thank you!
Signature · 2 lines
Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html