Netdev List
 help / color / mirror / Atom feed
From: "Daniel Zahka" <daniel.zahka@gmail.com>
To: "Paolo Abeni" <pabeni@redhat.com>,
	"Norbert Szetei" <norbert@doyensec.com>, <netdev@vger.kernel.org>
Cc: "Eric Dumazet" <edumazet@google.com>,
	"Kuniyuki Iwashima" <kuniyu@google.com>,
	"Willem de Bruijn" <willemb@google.com>,
	"David S. Miller" <davem@davemloft.net>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Simon Horman" <horms@kernel.org>,
	"Daniel Zahka" <daniel.zahka@gmail.com>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net] net: psp: do not inherit the Rx association on clone
Date: Tue, 01 Sep 2026 08:28:41 -0400	[thread overview]
Message-ID: <DL3ZAJ3NEDKQ.2ZUNLEUPN0JIF@gmail.com> (raw)
In-Reply-To: <924455c8-9f62-491f-ae3c-ea2127f46769@redhat.com>

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


  reply	other threads:[~2026-09-01 12:28 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-29 16:56 [PATCH net] net: psp: do not inherit the Rx association on clone Norbert Szetei
2026-08-29 20:40 ` Daniel Zahka
2026-08-30 18:07   ` Norbert Szetei
2026-09-01 10:01 ` Paolo Abeni
2026-09-01 12:28   ` Daniel Zahka [this message]
2026-09-01 13:10     ` Paolo Abeni
2026-09-01 13:20 ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DL3ZAJ3NEDKQ.2ZUNLEUPN0JIF@gmail.com \
    --to=daniel.zahka@gmail.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=kuniyu@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=norbert@doyensec.com \
    --cc=pabeni@redhat.com \
    --cc=willemb@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox