[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <5db7d6b4-5aa4-409d-a21b-51ed8c56ccb7@redhat.com>
Date: Thu, 29 Jan 2026 23:30:48 +0100
From: Petr Oros <poros@...hat.com>
To: Paul Menzel <pmenzel@...gen.mpg.de>
Cc: netdev@...r.kernel.org, ivecera@...hat.com,
Przemek Kitszel <przemyslaw.kitszel@...el.com>,
Richard Cochran <richardcochran@...il.com>,
Eric Dumazet <edumazet@...gle.com>, linux-kernel@...r.kernel.org,
Andrew Lunn <andrew+netdev@...n.ch>, Tony Nguyen
<anthony.l.nguyen@...el.com>, Simon Horman <horms@...nel.org>,
Mateusz Polchlopek <mateusz.polchlopek@...el.com>,
Jacob Keller <jacob.e.keller@...el.com>, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, "David S. Miller" <davem@...emloft.net>,
intel-wired-lan@...ts.osuosl.org
Subject: Re: [Intel-wired-lan] [PATCH net] iavf: fix PTP use-after-free during
reset
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@...hat.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@...gen.mpg.de>
>
>
> Kind regards,
>
> Paul
>
Powered by blists - more mailing lists