Re: [PATCH bpf-next v1] bpf, sockmap: Fix psock incorrectly pointing to sk

[John Fastabend <john.fastabend@gmail.com>]

On 2025-05-29 02:42:14, Jiayuan Chen wrote:
> May 29, 2025 at 07:46, "John Fastabend" <john.fastabend@gmail.com> wrote:
> 
> 
> 
> > 
> > On 2025-05-24 00:22:19, Jiayuan Chen wrote:
> > 
> > > 
> > > 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.

[...]

> > 
> > Can we add this to sk_psock_stop where we have the TX_ENABLED bit
> > 
> > cleared.
> 
> 
> 
> Thanks, I just add SK_PSOCK_TX_ENABLED checking at the start of sk_psock_backlog().
> Every works fine, and truly no more flag needed !
> 
> diff --git a/net/core/skmsg.c b/net/core/skmsg.c
> index 34c51eb1a14f..83c78379932e 100644
> --- a/net/core/skmsg.c
> +++ b/net/core/skmsg.c
> @@ -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
> 

Thanks. Please submit an official patch so we can get it merged.