Re: [PATCH net] igb: initialize PTP lock before registering PHC

[Tony Nguyen]

+ 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@xxxxxxxxxxxxxxx
> Assisted-by: Codex:GPT-5
> Signed-off-by: Runyu Xiao <runyu.xiao@xxxxxxxxxx>
> ---
>   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)