From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: Kurt Kanzenbach <kurt@linutronix.de>,
Vadim Fedorenko <vadim.fedorenko@linux.dev>,
Willem de Bruijn <willemb@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Eric Dumazet <edumazet@google.com>,
"David S. Miller" <davem@davemloft.net>,
"Loktionov, Aleksandr" <aleksandr.loktionov@intel.com>,
"Nguyen, Anthony L" <anthony.l.nguyen@intel.com>,
"Kitszel, Przemyslaw" <przemyslaw.kitszel@intel.com>,
Paul Menzel <pmenzel@molgen.mpg.de>,
"Gomes, Vinicius" <vinicius.gomes@intel.com>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
Richard Cochran <richardcochran@gmail.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Andrew Lunn <andrew+netdev@lunn.ch>,
"intel-wired-lan@lists.osuosl.org"
<intel-wired-lan@lists.osuosl.org>,
"Keller, Jacob E" <jacob.e.keller@intel.com>
Subject: Re: [Intel-wired-lan] [PATCH iwl-next v3] igb: Retrieve Tx timestamp directly from interrupt for i210
Date: Thu, 12 Feb 2026 19:33:33 +0100 [thread overview]
Message-ID: <20260212183333.7xQmtLRY@linutronix.de> (raw)
In-Reply-To: <willemdebruijn.kernel.2759f838d176e@gmail.com>
On 2026-02-11 11:29:43 [-0500], Willem de Bruijn wrote:
> I think we should look at the locking. It is not clear to me that
> sk_callback_lock needs to be held here at all.
>
> From the lock documentation itself its use is more limited.
>
> sk->sk_socket->file is indeed dereferenced elsewhere without holding
> such locks.
>
> sk_capable is another indication.
That skb has a socket reference so that skbs should have a reference on
'sk' so it can't go away while the skb is around.
sk_socket should be assigned once while sk is created and not change.
Also that ->file should be assigned on accept and remain stable.
That assignment happens in sock_graft() under the lock or in
sock_init_data_uid() without it (but it both cases it should be new).
If you close that socket then I think sock_close() will be invoked which
ends in __sock_release() and assigns NULL to ->file. The filep itself
is SLAB_TYPESAFE_BY_RCU (as it comes from alloc_file_pseudo()) so it
safe to access it while in IRQ/ softirq because it can not go away.
It should invoke sock_orphan() which sets ->sk_socket to NULL under the
lock. I don't think that orphan can be called from outside the close
path. And inode (socket) remains around as long as the filep is there.
So based on this, if sk->sk_socket->file is accessed from within a
syscall then the chain up to file should be valid because the fd can not
be closed and so anything in that chain up to file become NULL.
From within the IRQ/ softirq a close on the fd/ socket should not result
in an immediate release. I *think* the skb holds sock holds the socket.
If so that skb would hold the destruction of the socket back and this
would make it safe to access it without the lock.
This is the theory so far.
Sebastian
next prev parent reply other threads:[~2026-02-12 18:33 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-05 7:54 [PATCH iwl-next v3] igb: Retrieve Tx timestamp directly from interrupt for i210 Kurt Kanzenbach
2026-02-05 9:47 ` [Intel-wired-lan] " Loktionov, Aleksandr
2026-02-05 10:03 ` Sebastian Andrzej Siewior
2026-02-05 10:37 ` Loktionov, Aleksandr
2026-02-05 10:52 ` Sebastian Andrzej Siewior
2026-02-05 11:56 ` Vadim Fedorenko
2026-02-05 14:51 ` Sebastian Andrzej Siewior
2026-02-05 16:27 ` Vadim Fedorenko
2026-02-05 16:43 ` Sebastian Andrzej Siewior
2026-02-05 16:48 ` Vadim Fedorenko
2026-02-05 21:41 ` Willem de Bruijn
2026-02-06 7:44 ` Sebastian Andrzej Siewior
2026-02-06 10:12 ` Vadim Fedorenko
2026-02-08 16:25 ` Willem de Bruijn
2026-02-09 9:06 ` Sebastian Andrzej Siewior
2026-02-09 10:43 ` Vadim Fedorenko
2026-02-09 11:48 ` Sebastian Andrzej Siewior
2026-02-09 12:24 ` Vadim Fedorenko
2026-02-09 12:46 ` Willem de Bruijn
2026-02-10 12:12 ` Sebastian Andrzej Siewior
2026-02-10 16:14 ` Willem de Bruijn
2026-02-11 12:08 ` Kurt Kanzenbach
2026-02-11 16:29 ` Willem de Bruijn
2026-02-12 18:33 ` Sebastian Andrzej Siewior [this message]
2026-02-14 23:26 ` Sebastian Andrzej Siewior
2026-02-11 18:54 ` Jakub Kicinski
2026-02-12 16:28 ` Sebastian Andrzej Siewior
2026-02-11 19:29 ` Jacob Keller
2026-02-11 21:44 ` Jakub Kicinski
2026-02-12 16:47 ` Sebastian Andrzej Siewior
2026-02-05 11:58 ` Kurt Kanzenbach
2026-02-05 12:20 ` Loktionov, Aleksandr
2026-02-06 0:05 ` Jacob Keller
2026-02-05 12:12 ` Sebastian Andrzej Siewior
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=20260212183333.7xQmtLRY@linutronix.de \
--to=bigeasy@linutronix.de \
--cc=aleksandr.loktionov@intel.com \
--cc=andrew+netdev@lunn.ch \
--cc=anthony.l.nguyen@intel.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=jacob.e.keller@intel.com \
--cc=kuba@kernel.org \
--cc=kurt@linutronix.de \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=pmenzel@molgen.mpg.de \
--cc=przemyslaw.kitszel@intel.com \
--cc=richardcochran@gmail.com \
--cc=vadim.fedorenko@linux.dev \
--cc=vinicius.gomes@intel.com \
--cc=willemb@google.com \
--cc=willemdebruijn.kernel@gmail.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