Re: [Intel-wired-lan] [PATCH net] iavf: fix PTP use-after-free during reset

[Petr Oros <poros@redhat.com>]

On 1/29/26 22:48, Paul Menzel wrote:
> Dear Petr,
>
>
> Thank you for the patch.
>
> Am 29.01.26 um 10:57 schrieb Petr Oros:
>> Commit 7c01dbfc8a1c5f ("iavf: periodically cache PHC time") introduced a
>> worker to cache PHC time, but failed to stop it during reset or disable.
>>
>> This creates a race condition where `iavf_reset_task()` or
>> `iavf_disable_vf()` free adapter resources (AQ) while the worker is 
>> still
>> running. If the worker triggers `iavf_queue_ptp_cmd()` during 
>> teardown, it
>> accesses freed memory/locks, leading to a crash.
>
> Do you have a stacktrace, and could you add an excerpt, so people 
> hitting this, can more easily find the commit?

I have some stack traces. The problem is that the race window is so wide 
that it sometimes crashes in the PTP subsystem, looking like:

[ 5611.939379] Call Trace:
[ 5611.941831]  <TASK>
[ 5611.943937]  ? show_trace_log_lvl+0x1b0/0x2f0
[ 5611.948295]  ? show_trace_log_lvl+0x1b0/0x2f0
[ 5611.952656]  ? ptp_aux_kworker+0x1d/0x40
[ 5611.956584]  ? __die_body.cold+0x8/0x12
[ 5611.960422]  ? page_fault_oops+0x148/0x160
[ 5611.964520]  ? exc_page_fault+0x73/0x160
[ 5611.968445]  ? asm_exc_page_fault+0x26/0x30
[ 5611.972634]  ? __pfx_ptp_aux_kworker+0x10/0x10
[ 5611.977082]  ? __pfx_ptp_aux_kworker+0x10/0x10
[ 5611.981525]  ptp_aux_kworker+0x1d/0x40
[ 5611.985278]  kthread_worker_fn+0xa0/0x260
[ 5611.989291]  ? __pfx_kthread_worker_fn+0x10/0x10
[ 5611.993911]  kthread+0xfd/0x240
[ 5611.997056]  ? __pfx_kthread+0x10/0x10
[ 5612.000809]  ret_from_fork+0x34/0x50
[ 5612.004386]  ? __pfx_kthread+0x10/0x10
[ 5612.008140]  ret_from_fork_asm+0x1a/0x30
[ 5612.012069]  </TASK>

  and other times in iavf, looking like:

3476.640150] Call Trace:
[ 3476.642597]  <TASK>
[ 3476.644702]  ? show_trace_log_lvl+0x1b0/0x2f0
[ 3476.649062]  ? show_trace_log_lvl+0x1b0/0x2f0
[ 3476.653420]  ? mod_delayed_work_on+0x9f/0xb0
[ 3476.657691]  ? __die_body.cold+0x8/0x12
[ 3476.661530]  ? page_fault_oops+0x148/0x160
[ 3476.665629]  ? exc_page_fault+0x7f/0x150
[ 3476.669556]  ? asm_exc_page_fault+0x26/0x30
[ 3476.673743]  ? __queue_work.part.0+0x44/0x320
[ 3476.678100]  ? __pfx_ptp_aux_kworker+0x10/0x10
[ 3476.682547]  mod_delayed_work_on+0x9f/0xb0
[ 3476.686647]  iavf_send_phc_read+0xb0/0xd0 [iavf]
[ 3476.691283]  iavf_ptp_do_aux_work+0x39/0x50 [iavf]
[ 3476.696083]  ptp_aux_kworker+0x1d/0x40
[ 3476.699835]  kthread_worker_fn+0xa3/0x260
[ 3476.703848]  ? __pfx_kthread_worker_fn+0x10/0x10
[ 3476.708465]  kthread+0xfa/0x240
[ 3476.711613]  ? __pfx_kthread+0x10/0x10
[ 3476.715365]  ret_from_fork+0x34/0x50
[ 3476.718943]  ? __pfx_kthread+0x10/0x10
[ 3476.722695]  ret_from_fork_asm+0x1a/0x30
[ 3476.726622]  </TASK>

, etc.

>
>> Fix this by calling `iavf_ptp_release()` before tearing down the 
>> adapter.
>> This ensures `ptp_clock_unregister()` synchronously cancels the 
>> worker and
>> cleans up the chardev before the backing resources are destroyed.
>>
>> Fixes: 7c01dbfc8a1c5f ("iavf: periodically cache PHC time")
>> Signed-off-by: Petr Oros <poros@redhat.com>
>> ---
>>   drivers/net/ethernet/intel/iavf/iavf_main.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c 
>> b/drivers/net/ethernet/intel/iavf/iavf_main.c
>> index 4b0fc8f354bc90..0dd58ce5a53ab1 100644
>> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
>> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
>> @@ -3025,6 +3025,8 @@ static void iavf_disable_vf(struct iavf_adapter 
>> *adapter)
>>         adapter->flags |= IAVF_FLAG_PF_COMMS_FAILED;
>>   +    iavf_ptp_release(adapter);
>> +
>>       /* We don't use netif_running() because it may be true prior to
>>        * ndo_open() returning, so we can't assume it means all our open
>>        * tasks have finished, since we're not holding the rtnl_lock 
>> here.
>> @@ -3200,6 +3202,8 @@ static void iavf_reset_task(struct work_struct 
>> *work)
>>       iavf_change_state(adapter, __IAVF_RESETTING);
>>       adapter->flags &= ~IAVF_FLAG_RESET_PENDING;
>>   +    iavf_ptp_release(adapter);
>> +
>>       /* free the Tx/Rx rings and descriptors, might be better to just
>>        * re-use them sometime in the future
>>        */
>
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
>
>
> Kind regards,
>
> Paul
>