Re: [PATCH net 02/13] i40e: Cleanup PTP pins on probe failure

[Jacob Keller <jacob.e.keller@intel.com>]

On 5/4/2026 10:14 PM, Jacob Keller wrote:
> From: Matt Vollrath <tactii@gmail.com>
> 
> PTP pin structs are allocated early in probe, but never cleaned up.
> 
> Fix this by calling i40e_ptp_free_pins in the error path.
> 
> To support this, i40e_ptp_free_pins is added to the header and
> pin_config is correctly nullified after being freed.
> 
> This has been an issue since i40e_ptp_alloc_pins was introduced.
> 
> Fixes: 1050713026a08 ("i40e: add support for PTP external synchronization clock")
> Reported-by: Kohei Enju <kohei@enjuk.jp>
> Cc: stable@vger.kernel.org
> Signed-off-by: Matt Vollrath <tactii@gmail.com>
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Reviewed-by: Kohei Enju <kohei@enjuk.jp>
> Tested-by: Sunitha Mekala <sunithax.d.mekala@intel.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e.h      | 1 +
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 1 +
>  drivers/net/ethernet/intel/i40e/i40e_ptp.c  | 3 ++-
>  3 files changed, 4 insertions(+), 1 deletion(-)
> 
Sashiko points out some issues which are related but pre-existing this fix:

> This isn't a bug in this patch, but looking at i40e_probe(), the return
> value of i40e_ptp_alloc_pins() is ignored earlier in the probe sequence.
> If the memory allocation fails, pf->ptp_pins remains NULL, but the probe
> routine continues successfully.
> Later, if a user attempts to configure the PTP pins via the PTP clock ioctl
> (such as PTP_EXTTS_REQUEST), i40e_ptp_enable_pin() unconditionally dereferences
> the pointer:
> drivers/net/ethernet/intel/i40e/i40e_ptp.c:i40e_ptp_enable_pin() {
>     ...
> 	pins.sdp3_2 = pf->ptp_pins->sdp3_2;
>     ...
> }
> Could this lead to a NULL pointer dereference panic triggered by userspace?


Probably we just need to add an error handling of the pin allocation
since failure to allocate memory is likely to hit a lot of other places
as well. This is also a pre-existing issue.

> This isn't a bug in this patch, but while looking at PTP cleanup, it appears
> the pf->ptp_extts0_work work item is never canceled during device removal.
> In i40e_remove(), i40e_ptp_stop() masks the PTP event interrupts, but it
> doesn't call cancel_work_sync(&pf->ptp_extts0_work) to flush any already-queued
> work.
> i40e_remove() later cancels service_task but completely omits ptp_extts0_work.
> If an interrupt triggers and schedules the work immediately before the teardown
> path masks the interrupt, will the work item execute after the pf structure is
> completely freed, resulting in a use-after-free?

This is also a pre-existing issue in the PTP teardown that was reported
on patch 1/13 as well: we need to cancel the extts0_work item.