Re: [PATCH bpf-next v6] bpf, sockmap: avoid using sk_socket after free when sending

[Martin KaFai Lau <martin.lau@linux.dev>]

On 5/22/25 3:56 PM, Jiayuan Chen wrote:
>>>   @@ -656,6 +656,13 @@ static void sk_psock_backlog(struct work_struct *work)
>>>   bool ingress;
>>>   int ret;
>>>   > + /* 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
>>>   + * sock_map_close() completed with sk_socket potentially already freed.
>>>   + */
>>>   + if (!sk_psock_get(psock->sk))
>>
>> This seems to be the first use case to pass "psock->sk" to "sk_psock_get()".
>>
>> I could have missed the sock_map details here. Considering it is racing with sock_map_close() which should also do a sock_put(sk) [?],
>>
>> could you help to explain what makes it safe to access the psock->sk here?
>>

> 
> Using 'sk_psock_get(psock->sk)' in the workqueue is safe because
> sock_map_close() only reduces the reference count of psock to zero, while
> the actual memory release is fully handled by the RCU callback: sk_psock_destroy().
> 
> In sk_psock_destroy(), we first cancel_delayed_work_sync() to wait for the
> workqueue to complete, and then perform sock_put(psock->sk). This means we

Got it. The sock_put(psock->sk) done after a rcu gp is the part that I was missing.

Applied. Thanks.