From: Tony Nguyen <anthony.l.nguyen@intel.com>
To: Runyu Xiao <runyu.xiao@seu.edu.cn>,
Alessio Igor Bogani <alessio.bogani@elettra.eu>
Cc: <przemyslaw.kitszel@intel.com>, <richardcochran@gmail.com>,
<intel-wired-lan@lists.osuosl.org>, <netdev@vger.kernel.org>,
<linux-kernel@vger.kernel.org>, <jianhao.xu@seu.edu.cn>,
<stable@vger.kernel.org>
Subject: Re: [PATCH net] igb: initialize PTP lock before registering PHC
Date: Wed, 9 Sep 2026 13:48:14 -0700 [thread overview]
Message-ID: <cd12af33-ed01-4eca-88e3-703b59c05453@intel.com> (raw)
In-Reply-To: <20260830154912.2712900-1-runyu.xiao@seu.edu.cn>
+ Alessio
On 8/30/2026 8:49 AM, Runyu Xiao wrote:
> igb_ptp_init() registers the PHC before initializing
> adapter->tmreg_lock. ptp_clock_register() publishes the PHC device,
> so a userspace PTP operation can enter a callback and take the lock
> before it has been initialized.
>
> Initialize tmreg_lock before registering the PHC so all published PTP
> callbacks see a valid lock.
>
> Fixes: b888c510f7b3 ("igb: Avoid starting unnecessary workqueues")
> Cc: stable@vger.kernel.org
> Assisted-by: Codex:GPT-5
> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
> ---
> drivers/net/ethernet/intel/igb/igb_ptp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
> index 638d8242b..2a4ea327a 100644
> --- a/drivers/net/ethernet/intel/igb/igb_ptp.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
> @@ -1378,6 +1378,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
> return;
> }
>
> + spin_lock_init(&adapter->tmreg_lock);
> adapter->ptp_clock = ptp_clock_register(&adapter->ptp_caps,
> &adapter->pdev->dev);
Sashiko says:
This isn't a bug introduced by this patch, but does this expose the PTP
character device to userspace before the timecounter is fully initialized?
If userspace immediately opens the device and issues the PTP_CLOCK_GETTIME
ioctl, callbacks like igb_ptp_gettimex_82576() are invoked. These callbacks
call timecounter_cyc2time() on adapter->tc. Because igb_ptp_reset() has not
yet executed at this point to initialize the timecounter, adapter->tc.cc is
NULL, which can cause a kernel panic due to a NULL pointer dereference.
> if (IS_ERR(adapter->ptp_clock)) {
> @@ -1388,7 +1389,6 @@ void igb_ptp_init(struct igb_adapter *adapter)
> adapter->netdev->name);
> adapter->ptp_flags |= IGB_PTP_ENABLED;
>
> - spin_lock_init(&adapter->tmreg_lock);
> INIT_WORK(&adapter->ptp_tx_work, igb_ptp_tx_work);
and:
This is a pre-existing issue, but does initializing this work struct here
leave a race window for list corruption?
Since register_netdev() is called in igb_probe() before igb_ptp_init()
executes, the network interface is already visible to userspace. If
userspace brings the interface up and transmits a packet with
SO_TIMESTAMPING
requested, igb_xmit_frame_ring() can execute concurrently and call
schedule_work(&adapter->ptp_tx_work).
Because the work struct is uninitialized at that time, enqueuing it can
cause list corruption and a kernel panic. Should INIT_WORK() and other PTP
initializations be performed before register_netdev() in igb_probe()?
Tony:
Even though these report here as pre-existing, it seems these originate
from the Fixes commit. Everything needs to be setup and ready to run
before calling ptp_clock_register() so we can't defer starting these
until after the PTP register. I think we need to back out the fixes
commit and add a proper CONFIG_PTP check and unwind path for register fail.
Thanks,
Tony
>
> if (adapter->ptp_flags & IGB_PTP_OVERFLOW_CHECK)
prev parent reply other threads:[~2026-09-09 20:48 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-30 15:49 Runyu Xiao
2026-09-09 20:48 ` Tony Nguyen [this message]
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=cd12af33-ed01-4eca-88e3-703b59c05453@intel.com \
--to=anthony.l.nguyen@intel.com \
--cc=alessio.bogani@elettra.eu \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=jianhao.xu@seu.edu.cn \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=przemyslaw.kitszel@intel.com \
--cc=richardcochran@gmail.com \
--cc=runyu.xiao@seu.edu.cn \
--cc=stable@vger.kernel.org \
/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
all inboxes | Powered by JetHome®