Re: [PATCH net] net: psp: do not inherit the Rx association on clone
From: Daniel Zahka
Date: Tue Sep 01 2026 - 08:59:34 EST
On Tue Sep 1, 2026 at 6:01 AM EDT, Paolo Abeni wrote:
> On 8/29/26 6:56 PM, Norbert Szetei wrote:
>> sk->psp_assoc sits past sk_dontcopy_end, so sock_copy() copies it into
>> every socket accepted from a listener without taking a reference, while
>> inet_sock_destruct() puts for every inet socket. psp_twsk_init() does
>> refcount_inc() for the timewait socket, so a child closing through
>> TIME_WAIT cancels its own put and leaves the association with one
>> reference and N timewait sockets holding the same pointer. Closing the
>> listener frees it, and the timewait timers then put freed memory.
>>
>> Rejecting the association on a listening socket is not sufficient: a socket
>> can acquire one while established and then be turned back into a listener,
>> because tcp_disconnect() leaves sk->psp_assoc in place.
>
> So rejecting the association on listener, and clearing on disconnect
> would be enough, right?
I think that would solve this problem with sk_clone(), but clearing out
the psp_assoc from the sk anywhere other than the socket destructor
makes me nervous because of the risk of leaking cleartext to the
network, or admitting cleartext the receive queue.
Specifically about tcp_disconnect(), the write queue purge won't save us
from skbs already queued to the device. I suppose if we clear out the
sk_validate_xmit_skb hook we have for psp, those would at least get
dropped first. After that there is still a hazard of the psp_assoc
cleanup deleting a tx key handle, while tx descriptors in the driver
ring are still referencing that. That is something for which we rely
upon psp skbs being socket owned until the driver tx completion path.
All that to say, I think the fix here is the best option.
>
> I think that would be preferable: it's a pity to add safeguard code to
> the datapath due to a syscall (disconnect) used mostly by fuzzers.
>
> /P