* [PATCH iwl-net 0/2] ice: fix NULL access of tx->in_use
@ 2025-08-07 17:35 Jacob Keller
2025-08-07 17:35 ` [PATCH iwl-net 1/2] ice: fix NULL access of tx->in_use in ice_ptp_ts_irq Jacob Keller
2025-08-07 17:35 ` [PATCH iwl-net 2/2] ice: fix NULL access of tx->in_use in ice_ll_ts_intr Jacob Keller
0 siblings, 2 replies; 12+ messages in thread
From: Jacob Keller @ 2025-08-07 17:35 UTC (permalink / raw)
To: Przemek Kitszel, Intel Wired LAN, netdev; +Cc: Jacob Keller
Recent versions of the E810 firmware have support for a "low latency"
interface for accessing Tx timestamps without interacting over the AdminQ.
Unfortunately, a couple of the functions involved with this logic don't
properly check if the Tx timestamp tracker is initialized. This can lead to
a use-after-free or NULL pointer access violation:
[245977.278756] BUG: kernel NULL pointer dereference, address: 0000000000000000
[245977.278774] RIP: 0010:_find_first_bit+0x19/0x40
[245977.278796] Call Trace:
[245977.278809] ? ice_misc_intr+0x364/0x380 [ice]
This happens because the reset flow in the driver re-initializes the
tracker. If a Tx timestamp interrupt occurs concurrently with a reset, the
ineractions race and the crash can occur.
I split the fixes to two separate patches because the extra interrupt for
low latency came in a different kernel. Hopefully this aids with
backporting the fix.
This was originally reported on an older kernel prior to f9472aaabd1f
("ice: Process TSYN IRQ in a separate function"), which has a similar bug,
but at a different code point. I plan to look into if any stable kernels
need a separate fix after this.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
Jacob Keller (2):
ice: fix NULL access of tx->in_use in ice_ptp_ts_irq
ice: fix NULL access of tx->in_use in ice_ll_ts_intr
drivers/net/ethernet/intel/ice/ice_main.c | 12 +++++++-----
drivers/net/ethernet/intel/ice/ice_ptp.c | 13 ++++++++-----
2 files changed, 15 insertions(+), 10 deletions(-)
---
base-commit: d942fe13f72bec92f6c689fbd74c5ec38228c16a
change-id: 20250807-jk-ice-fix-tx-tstamp-race-5fe8484670a4
Best regards,
--
Jacob Keller <jacob.e.keller@intel.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH iwl-net 1/2] ice: fix NULL access of tx->in_use in ice_ptp_ts_irq
2025-08-07 17:35 [PATCH iwl-net 0/2] ice: fix NULL access of tx->in_use Jacob Keller
@ 2025-08-07 17:35 ` Jacob Keller
2025-08-07 17:56 ` Jacob Keller
` (4 more replies)
2025-08-07 17:35 ` [PATCH iwl-net 2/2] ice: fix NULL access of tx->in_use in ice_ll_ts_intr Jacob Keller
1 sibling, 5 replies; 12+ messages in thread
From: Jacob Keller @ 2025-08-07 17:35 UTC (permalink / raw)
To: Przemek Kitszel, Intel Wired LAN, netdev; +Cc: Jacob Keller
The E810 device has support for a "low latency" firmware interface to
access and read the Tx timestamps. This interface does not use the standard
Tx timestamp logic, due to the latency overhead of proxying sideband
command requests over the firmware AdminQ.
The logic still makes use of the Tx timestamp tracking structure,
ice_ptp_tx, as it uses the same "ready" bitmap to track which Tx
timestamps.
Unfortunately, the ice_ptp_ts_irq() function does not check if the tracker
is initialized before its first access. This results in NULL dereference or
use-after-free bugs similar to the following:
[245977.278756] BUG: kernel NULL pointer dereference, address: 0000000000000000
[245977.278774] RIP: 0010:_find_first_bit+0x19/0x40
[245977.278796] Call Trace:
[245977.278809] ? ice_misc_intr+0x364/0x380 [ice]
This can occur if a Tx timestamp interrupt races with the driver reset
logic.
Fix this by only checking the in_use bitmap (and other fields) if the
tracker is marked as initialized. The reset flow will clear the init field
under lock before it tears the tracker down, thus preventing any
use-after-free or NULL access.
Fixes: f9472aaabd1f ("ice: Process TSYN IRQ in a separate function")
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/ethernet/intel/ice/ice_ptp.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index e358eb1d719f..fb0f6365a6d6 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -2701,16 +2701,19 @@ irqreturn_t ice_ptp_ts_irq(struct ice_pf *pf)
*/
if (hw->dev_caps.ts_dev_info.ts_ll_int_read) {
struct ice_ptp_tx *tx = &pf->ptp.port.tx;
- u8 idx;
+ u8 idx, last;
if (!ice_pf_state_is_nominal(pf))
return IRQ_HANDLED;
spin_lock(&tx->lock);
- idx = find_next_bit_wrap(tx->in_use, tx->len,
- tx->last_ll_ts_idx_read + 1);
- if (idx != tx->len)
- ice_ptp_req_tx_single_tstamp(tx, idx);
+ if (tx->init) {
+ last = tx->last_ll_ts_idx_read + 1;
+ idx = find_next_bit_wrap(tx->in_use, tx->len,
+ last);
+ if (idx != tx->len)
+ ice_ptp_req_tx_single_tstamp(tx, idx);
+ }
spin_unlock(&tx->lock);
return IRQ_HANDLED;
--
2.50.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH iwl-net 2/2] ice: fix NULL access of tx->in_use in ice_ll_ts_intr
2025-08-07 17:35 [PATCH iwl-net 0/2] ice: fix NULL access of tx->in_use Jacob Keller
2025-08-07 17:35 ` [PATCH iwl-net 1/2] ice: fix NULL access of tx->in_use in ice_ptp_ts_irq Jacob Keller
@ 2025-08-07 17:35 ` Jacob Keller
2025-08-07 21:32 ` [Intel-wired-lan] " Paul Menzel
2025-08-29 6:04 ` Rinitha, SX
1 sibling, 2 replies; 12+ messages in thread
From: Jacob Keller @ 2025-08-07 17:35 UTC (permalink / raw)
To: Przemek Kitszel, Intel Wired LAN, netdev; +Cc: Jacob Keller
Recent versions of the E810 firmware have support for an extra interrupt to
handle report of the "low latency" Tx timestamps coming from the
specialized low latency firmware interface. Instead of polling the
registers, software can wait until the low latency interrupt is fired.
This logic makes use of the Tx timestamp tracking structure, ice_ptp_tx, as
it uses the same "ready" bitmap to track which Tx timestamps.
Unfortunately, the ice_ll_ts_intr() function does not check if the
tracker is initialized before its first access. This results in NULL
dereference or use-after-free bugs similar to the issues fixed in the
ice_ptp_ts_irq() function.
Fix this by only checking the in_use bitmap (and other fields) if the
tracker is marked as initialized. The reset flow will clear the init field
under lock before it tears the tracker down, thus preventing any
use-after-free or NULL access.
Fixes: 82e71b226e0e ("ice: Enable SW interrupt from FW for LL TS")
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/ethernet/intel/ice/ice_main.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 8e0b06c1e02b..7b002127e40d 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3176,12 +3176,14 @@ static irqreturn_t ice_ll_ts_intr(int __always_unused irq, void *data)
hw = &pf->hw;
tx = &pf->ptp.port.tx;
spin_lock_irqsave(&tx->lock, flags);
- ice_ptp_complete_tx_single_tstamp(tx);
+ if (tx->init) {
+ ice_ptp_complete_tx_single_tstamp(tx);
- idx = find_next_bit_wrap(tx->in_use, tx->len,
- tx->last_ll_ts_idx_read + 1);
- if (idx != tx->len)
- ice_ptp_req_tx_single_tstamp(tx, idx);
+ idx = find_next_bit_wrap(tx->in_use, tx->len,
+ tx->last_ll_ts_idx_read + 1);
+ if (idx != tx->len)
+ ice_ptp_req_tx_single_tstamp(tx, idx);
+ }
spin_unlock_irqrestore(&tx->lock, flags);
val = GLINT_DYN_CTL_INTENA_M | GLINT_DYN_CTL_CLEARPBA_M |
--
2.50.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH iwl-net 1/2] ice: fix NULL access of tx->in_use in ice_ptp_ts_irq
2025-08-07 17:35 ` [PATCH iwl-net 1/2] ice: fix NULL access of tx->in_use in ice_ptp_ts_irq Jacob Keller
@ 2025-08-07 17:56 ` Jacob Keller
2025-08-07 21:29 ` [Intel-wired-lan] " Paul Menzel
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Jacob Keller @ 2025-08-07 17:56 UTC (permalink / raw)
To: Przemek Kitszel, Intel Wired LAN, netdev
[-- Attachment #1.1: Type: text/plain, Size: 2983 bytes --]
On 8/7/2025 10:35 AM, Jacob Keller wrote:
> The E810 device has support for a "low latency" firmware interface to
> access and read the Tx timestamps. This interface does not use the standard
> Tx timestamp logic, due to the latency overhead of proxying sideband
> command requests over the firmware AdminQ.
>
> The logic still makes use of the Tx timestamp tracking structure,
> ice_ptp_tx, as it uses the same "ready" bitmap to track which Tx
> timestamps.
>
> Unfortunately, the ice_ptp_ts_irq() function does not check if the tracker
> is initialized before its first access. This results in NULL dereference or
> use-after-free bugs similar to the following:
>
> [245977.278756] BUG: kernel NULL pointer dereference, address: 0000000000000000
> [245977.278774] RIP: 0010:_find_first_bit+0x19/0x40
> [245977.278796] Call Trace:
> [245977.278809] ? ice_misc_intr+0x364/0x380 [ice]
>
> This can occur if a Tx timestamp interrupt races with the driver reset
> logic.
>
> Fix this by only checking the in_use bitmap (and other fields) if the
> tracker is marked as initialized. The reset flow will clear the init field
> under lock before it tears the tracker down, thus preventing any
> use-after-free or NULL access.
>
> Fixes: f9472aaabd1f ("ice: Process TSYN IRQ in a separate function")
Prior to f9472aaabd1f this same code was in ice_main.c, which means that
strictly speaking, the bug exists in 82e71b226e0e ("ice: Enable SW
interrupt from FW for LL TS"). This fix only applies to v6.15, but we
will want to fix the stable tree for v6.12, since the original bug has
existed since v6.12
I guess after this merges, I will need to make a stable request with the
targeted fix for the v6.12 kernel separately.
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice_ptp.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
> index e358eb1d719f..fb0f6365a6d6 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
> @@ -2701,16 +2701,19 @@ irqreturn_t ice_ptp_ts_irq(struct ice_pf *pf)
> */
> if (hw->dev_caps.ts_dev_info.ts_ll_int_read) {
> struct ice_ptp_tx *tx = &pf->ptp.port.tx;
> - u8 idx;
> + u8 idx, last;
>
> if (!ice_pf_state_is_nominal(pf))
> return IRQ_HANDLED;
>
> spin_lock(&tx->lock);
> - idx = find_next_bit_wrap(tx->in_use, tx->len,
> - tx->last_ll_ts_idx_read + 1);
> - if (idx != tx->len)
> - ice_ptp_req_tx_single_tstamp(tx, idx);
> + if (tx->init) {
> + last = tx->last_ll_ts_idx_read + 1;
> + idx = find_next_bit_wrap(tx->in_use, tx->len,
> + last);
> + if (idx != tx->len)
> + ice_ptp_req_tx_single_tstamp(tx, idx);
> + }
> spin_unlock(&tx->lock);
>
> return IRQ_HANDLED;
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-wired-lan] [PATCH iwl-net 1/2] ice: fix NULL access of tx->in_use in ice_ptp_ts_irq
2025-08-07 17:35 ` [PATCH iwl-net 1/2] ice: fix NULL access of tx->in_use in ice_ptp_ts_irq Jacob Keller
2025-08-07 17:56 ` Jacob Keller
@ 2025-08-07 21:29 ` Paul Menzel
2025-08-07 23:56 ` Jacob Keller
2025-08-08 6:47 ` Loktionov, Aleksandr
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Paul Menzel @ 2025-08-07 21:29 UTC (permalink / raw)
To: Jacob Keller; +Cc: Przemek Kitszel, intel-wired-lan, netdev
Dear Jacob,
Thank you for the patch.
Am 07.08.25 um 19:35 schrieb Jacob Keller:
> The E810 device has support for a "low latency" firmware interface to
> access and read the Tx timestamps. This interface does not use the standard
> Tx timestamp logic, due to the latency overhead of proxying sideband
> command requests over the firmware AdminQ.
>
> The logic still makes use of the Tx timestamp tracking structure,
> ice_ptp_tx, as it uses the same "ready" bitmap to track which Tx
> timestamps.
>
> Unfortunately, the ice_ptp_ts_irq() function does not check if the tracker
> is initialized before its first access. This results in NULL dereference or
> use-after-free bugs similar to the following:
>
> [245977.278756] BUG: kernel NULL pointer dereference, address: 0000000000000000
> [245977.278774] RIP: 0010:_find_first_bit+0x19/0x40
> [245977.278796] Call Trace:
> [245977.278809] ? ice_misc_intr+0x364/0x380 [ice]
>
> This can occur if a Tx timestamp interrupt races with the driver reset
> logic.
Do you have a reproducer?
> Fix this by only checking the in_use bitmap (and other fields) if the
> tracker is marked as initialized. The reset flow will clear the init field
> under lock before it tears the tracker down, thus preventing any
> use-after-free or NULL access.
Great commit message. Thank you for taking the time to write this down.
> Fixes: f9472aaabd1f ("ice: Process TSYN IRQ in a separate function")
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice_ptp.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
> index e358eb1d719f..fb0f6365a6d6 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
> @@ -2701,16 +2701,19 @@ irqreturn_t ice_ptp_ts_irq(struct ice_pf *pf)
> */
> if (hw->dev_caps.ts_dev_info.ts_ll_int_read) {
> struct ice_ptp_tx *tx = &pf->ptp.port.tx;
> - u8 idx;
> + u8 idx, last;
>
> if (!ice_pf_state_is_nominal(pf))
> return IRQ_HANDLED;
>
> spin_lock(&tx->lock);
> - idx = find_next_bit_wrap(tx->in_use, tx->len,
> - tx->last_ll_ts_idx_read + 1);
> - if (idx != tx->len)
> - ice_ptp_req_tx_single_tstamp(tx, idx);
> + if (tx->init) {
> + last = tx->last_ll_ts_idx_read + 1;
> + idx = find_next_bit_wrap(tx->in_use, tx->len,
> + last);
> + if (idx != tx->len)
> + ice_ptp_req_tx_single_tstamp(tx, idx);
> + }
> spin_unlock(&tx->lock);
>
> return IRQ_HANDLED;
>
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Kind regards,
Paul
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-wired-lan] [PATCH iwl-net 2/2] ice: fix NULL access of tx->in_use in ice_ll_ts_intr
2025-08-07 17:35 ` [PATCH iwl-net 2/2] ice: fix NULL access of tx->in_use in ice_ll_ts_intr Jacob Keller
@ 2025-08-07 21:32 ` Paul Menzel
2025-08-07 23:53 ` Jacob Keller
2025-08-29 6:04 ` Rinitha, SX
1 sibling, 1 reply; 12+ messages in thread
From: Paul Menzel @ 2025-08-07 21:32 UTC (permalink / raw)
To: Jacob Keller, Przemek Kitszel, Intel Wired LAN, netdev
Dear Jacob,
Thank you for the patch.
Am 07.08.25 um 19:35 schrieb Jacob Keller:
> Recent versions of the E810 firmware have support for an extra interrupt to
> handle report of the "low latency" Tx timestamps coming from the
> specialized low latency firmware interface. Instead of polling the
> registers, software can wait until the low latency interrupt is fired.
>
> This logic makes use of the Tx timestamp tracking structure, ice_ptp_tx, as
> it uses the same "ready" bitmap to track which Tx timestamps.
Is the last part “to track which Tx timestamps” complete?
> Unfortunately, the ice_ll_ts_intr() function does not check if the
> tracker is initialized before its first access. This results in NULL
> dereference or use-after-free bugs similar to the issues fixed in the
> ice_ptp_ts_irq() function.
>
> Fix this by only checking the in_use bitmap (and other fields) if the
> tracker is marked as initialized. The reset flow will clear the init field
> under lock before it tears the tracker down, thus preventing any
> use-after-free or NULL access.
>
> Fixes: 82e71b226e0e ("ice: Enable SW interrupt from FW for LL TS")
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice_main.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index 8e0b06c1e02b..7b002127e40d 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -3176,12 +3176,14 @@ static irqreturn_t ice_ll_ts_intr(int __always_unused irq, void *data)
> hw = &pf->hw;
> tx = &pf->ptp.port.tx;
> spin_lock_irqsave(&tx->lock, flags);
> - ice_ptp_complete_tx_single_tstamp(tx);
> + if (tx->init) {
> + ice_ptp_complete_tx_single_tstamp(tx);
>
> - idx = find_next_bit_wrap(tx->in_use, tx->len,
> - tx->last_ll_ts_idx_read + 1);
> - if (idx != tx->len)
> - ice_ptp_req_tx_single_tstamp(tx, idx);
> + idx = find_next_bit_wrap(tx->in_use, tx->len,
> + tx->last_ll_ts_idx_read + 1);
> + if (idx != tx->len)
> + ice_ptp_req_tx_single_tstamp(tx, idx);
> + }
> spin_unlock_irqrestore(&tx->lock, flags);
>
> val = GLINT_DYN_CTL_INTENA_M | GLINT_DYN_CTL_CLEARPBA_M |
>
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Kind regards,
Paul
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-wired-lan] [PATCH iwl-net 2/2] ice: fix NULL access of tx->in_use in ice_ll_ts_intr
2025-08-07 21:32 ` [Intel-wired-lan] " Paul Menzel
@ 2025-08-07 23:53 ` Jacob Keller
0 siblings, 0 replies; 12+ messages in thread
From: Jacob Keller @ 2025-08-07 23:53 UTC (permalink / raw)
To: Paul Menzel, Przemek Kitszel, Intel Wired LAN, netdev
[-- Attachment #1.1: Type: text/plain, Size: 792 bytes --]
On 8/7/2025 2:32 PM, Paul Menzel wrote:
> Dear Jacob,
>
>
> Thank you for the patch.
>
> Am 07.08.25 um 19:35 schrieb Jacob Keller:
>> Recent versions of the E810 firmware have support for an extra interrupt to
>> handle report of the "low latency" Tx timestamps coming from the
>> specialized low latency firmware interface. Instead of polling the
>> registers, software can wait until the low latency interrupt is fired.
>>
>> This logic makes use of the Tx timestamp tracking structure, ice_ptp_tx, as
>> it uses the same "ready" bitmap to track which Tx timestamps.
>
> Is the last part “to track which Tx timestamps” complete?
>
Ah, yes, I think complete is the right word to end this sentence. Will
fix if there is a need for a re-roll.
Thanks,
Jake
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-wired-lan] [PATCH iwl-net 1/2] ice: fix NULL access of tx->in_use in ice_ptp_ts_irq
2025-08-07 21:29 ` [Intel-wired-lan] " Paul Menzel
@ 2025-08-07 23:56 ` Jacob Keller
0 siblings, 0 replies; 12+ messages in thread
From: Jacob Keller @ 2025-08-07 23:56 UTC (permalink / raw)
To: Paul Menzel; +Cc: Przemek Kitszel, intel-wired-lan, netdev
[-- Attachment #1.1: Type: text/plain, Size: 2075 bytes --]
On 8/7/2025 2:29 PM, Paul Menzel wrote:
> Dear Jacob,
>
>
> Thank you for the patch.
>
> Am 07.08.25 um 19:35 schrieb Jacob Keller:
>> The E810 device has support for a "low latency" firmware interface to
>> access and read the Tx timestamps. This interface does not use the standard
>> Tx timestamp logic, due to the latency overhead of proxying sideband
>> command requests over the firmware AdminQ.
>>
>> The logic still makes use of the Tx timestamp tracking structure,
>> ice_ptp_tx, as it uses the same "ready" bitmap to track which Tx
>> timestamps.
>>
>> Unfortunately, the ice_ptp_ts_irq() function does not check if the tracker
>> is initialized before its first access. This results in NULL dereference or
>> use-after-free bugs similar to the following:
>>
>> [245977.278756] BUG: kernel NULL pointer dereference, address: 0000000000000000
>> [245977.278774] RIP: 0010:_find_first_bit+0x19/0x40
>> [245977.278796] Call Trace:
>> [245977.278809] ? ice_misc_intr+0x364/0x380 [ice]
>>
>> This can occur if a Tx timestamp interrupt races with the driver reset
>> logic.
>
> Do you have a reproducer?
>
Not reliably. Pretty much any time you have a Tx timestamp occurring
simultaneously with a device reset could trigger it. I believe this was
reported by a customer along side a firmware update which triggered the
EMP reset logic on one function while another function had active Tx
timestamps going. I wasn't able to reliably reproduce the issue on my
setup yet, but it is quite obvious from inspecting the reported panic
which I included here in minified form.
>> Fix this by only checking the in_use bitmap (and other fields) if the
>> tracker is marked as initialized. The reset flow will clear the init field
>> under lock before it tears the tracker down, thus preventing any
>> use-after-free or NULL access.
>
> Great commit message. Thank you for taking the time to write this down.
>
Thanks,
Jake
>
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
>
>
> Kind regards,
>
> Paul
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [Intel-wired-lan] [PATCH iwl-net 1/2] ice: fix NULL access of tx->in_use in ice_ptp_ts_irq
2025-08-07 17:35 ` [PATCH iwl-net 1/2] ice: fix NULL access of tx->in_use in ice_ptp_ts_irq Jacob Keller
2025-08-07 17:56 ` Jacob Keller
2025-08-07 21:29 ` [Intel-wired-lan] " Paul Menzel
@ 2025-08-08 6:47 ` Loktionov, Aleksandr
2025-08-08 6:48 ` Loktionov, Aleksandr
2025-08-29 6:01 ` Rinitha, SX
4 siblings, 0 replies; 12+ messages in thread
From: Loktionov, Aleksandr @ 2025-08-08 6:47 UTC (permalink / raw)
To: Keller, Jacob E, Kitszel, Przemyslaw, Intel Wired LAN,
netdev@vger.kernel.org
Cc: Keller, Jacob E
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Jacob Keller
> Sent: Thursday, August 7, 2025 7:35 PM
> To: Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Intel Wired
> LAN <intel-wired-lan@lists.osuosl.org>; netdev@vger.kernel.org
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-net 1/2] ice: fix NULL access of
> tx->in_use in ice_ptp_ts_irq
>
> The E810 device has support for a "low latency" firmware interface to
> access and read the Tx timestamps. This interface does not use the
> standard
> Tx timestamp logic, due to the latency overhead of proxying sideband
> command requests over the firmware AdminQ.
>
> The logic still makes use of the Tx timestamp tracking structure,
> ice_ptp_tx, as it uses the same "ready" bitmap to track which Tx
> timestamps.
>
> Unfortunately, the ice_ptp_ts_irq() function does not check if the
> tracker
> is initialized before its first access. This results in NULL
> dereference or
> use-after-free bugs similar to the following:
>
> [245977.278756] BUG: kernel NULL pointer dereference, address:
> 0000000000000000
> [245977.278774] RIP: 0010:_find_first_bit+0x19/0x40
> [245977.278796] Call Trace:
> [245977.278809] ? ice_misc_intr+0x364/0x380 [ice]
>
> This can occur if a Tx timestamp interrupt races with the driver reset
> logic.
>
> Fix this by only checking the in_use bitmap (and other fields) if the
> tracker is marked as initialized. The reset flow will clear the init
> field
> under lock before it tears the tracker down, thus preventing any
> use-after-free or NULL access.
>
> Fixes: f9472aaabd1f ("ice: Process TSYN IRQ in a separate function")
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice_ptp.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c
> b/drivers/net/ethernet/intel/ice/ice_ptp.c
> index e358eb1d719f..fb0f6365a6d6 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
> @@ -2701,16 +2701,19 @@ irqreturn_t ice_ptp_ts_irq(struct ice_pf *pf)
> */
> if (hw->dev_caps.ts_dev_info.ts_ll_int_read) {
> struct ice_ptp_tx *tx = &pf->ptp.port.tx;
> - u8 idx;
> + u8 idx, last;
>
> if (!ice_pf_state_is_nominal(pf))
> return IRQ_HANDLED;
>
> spin_lock(&tx->lock);
> - idx = find_next_bit_wrap(tx->in_use, tx->len,
> - tx->last_ll_ts_idx_read +
> 1);
> - if (idx != tx->len)
> - ice_ptp_req_tx_single_tstamp(tx, idx);
> + if (tx->init) {
> + last = tx->last_ll_ts_idx_read + 1;
> + idx = find_next_bit_wrap(tx->in_use, tx-
> >len,
> + last);
> + if (idx != tx->len)
> + ice_ptp_req_tx_single_tstamp(tx,
> idx);
> + }
> spin_unlock(&tx->lock);
>
> return IRQ_HANDLED;
>
> --
> 2.50.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [Intel-wired-lan] [PATCH iwl-net 1/2] ice: fix NULL access of tx->in_use in ice_ptp_ts_irq
2025-08-07 17:35 ` [PATCH iwl-net 1/2] ice: fix NULL access of tx->in_use in ice_ptp_ts_irq Jacob Keller
` (2 preceding siblings ...)
2025-08-08 6:47 ` Loktionov, Aleksandr
@ 2025-08-08 6:48 ` Loktionov, Aleksandr
2025-08-29 6:01 ` Rinitha, SX
4 siblings, 0 replies; 12+ messages in thread
From: Loktionov, Aleksandr @ 2025-08-08 6:48 UTC (permalink / raw)
To: Keller, Jacob E, Kitszel, Przemyslaw, Intel Wired LAN,
netdev@vger.kernel.org
Cc: Keller, Jacob E
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Jacob Keller
> Sent: Thursday, August 7, 2025 7:35 PM
> To: Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Intel Wired
> LAN <intel-wired-lan@lists.osuosl.org>; netdev@vger.kernel.org
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-net 1/2] ice: fix NULL access of
> tx->in_use in ice_ptp_ts_irq
>
> The E810 device has support for a "low latency" firmware interface to
> access and read the Tx timestamps. This interface does not use the
> standard
> Tx timestamp logic, due to the latency overhead of proxying sideband
> command requests over the firmware AdminQ.
>
> The logic still makes use of the Tx timestamp tracking structure,
> ice_ptp_tx, as it uses the same "ready" bitmap to track which Tx
> timestamps.
>
> Unfortunately, the ice_ptp_ts_irq() function does not check if the
> tracker
> is initialized before its first access. This results in NULL
> dereference or
> use-after-free bugs similar to the following:
>
> [245977.278756] BUG: kernel NULL pointer dereference, address:
> 0000000000000000
> [245977.278774] RIP: 0010:_find_first_bit+0x19/0x40
> [245977.278796] Call Trace:
> [245977.278809] ? ice_misc_intr+0x364/0x380 [ice]
>
> This can occur if a Tx timestamp interrupt races with the driver reset
> logic.
>
> Fix this by only checking the in_use bitmap (and other fields) if the
> tracker is marked as initialized. The reset flow will clear the init
> field
> under lock before it tears the tracker down, thus preventing any
> use-after-free or NULL access.
>
> Fixes: f9472aaabd1f ("ice: Process TSYN IRQ in a separate function")
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice_ptp.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c
> b/drivers/net/ethernet/intel/ice/ice_ptp.c
> index e358eb1d719f..fb0f6365a6d6 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
> @@ -2701,16 +2701,19 @@ irqreturn_t ice_ptp_ts_irq(struct ice_pf *pf)
> */
> if (hw->dev_caps.ts_dev_info.ts_ll_int_read) {
> struct ice_ptp_tx *tx = &pf->ptp.port.tx;
> - u8 idx;
> + u8 idx, last;
>
> if (!ice_pf_state_is_nominal(pf))
> return IRQ_HANDLED;
>
> spin_lock(&tx->lock);
> - idx = find_next_bit_wrap(tx->in_use, tx->len,
> - tx->last_ll_ts_idx_read +
> 1);
> - if (idx != tx->len)
> - ice_ptp_req_tx_single_tstamp(tx, idx);
> + if (tx->init) {
> + last = tx->last_ll_ts_idx_read + 1;
> + idx = find_next_bit_wrap(tx->in_use, tx-
> >len,
> + last);
> + if (idx != tx->len)
> + ice_ptp_req_tx_single_tstamp(tx,
> idx);
> + }
> spin_unlock(&tx->lock);
>
> return IRQ_HANDLED;
>
> --
> 2.50.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [Intel-wired-lan] [PATCH iwl-net 1/2] ice: fix NULL access of tx->in_use in ice_ptp_ts_irq
2025-08-07 17:35 ` [PATCH iwl-net 1/2] ice: fix NULL access of tx->in_use in ice_ptp_ts_irq Jacob Keller
` (3 preceding siblings ...)
2025-08-08 6:48 ` Loktionov, Aleksandr
@ 2025-08-29 6:01 ` Rinitha, SX
4 siblings, 0 replies; 12+ messages in thread
From: Rinitha, SX @ 2025-08-29 6:01 UTC (permalink / raw)
To: Keller, Jacob E, Kitszel, Przemyslaw, Intel Wired LAN,
netdev@vger.kernel.org
Cc: Keller, Jacob E
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Jacob Keller
> Sent: 07 August 2025 23:05
> To: Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Intel Wired LAN <intel-wired-lan@lists.osuosl.org>; netdev@vger.kernel.org
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-net 1/2] ice: fix NULL access of tx->in_use in ice_ptp_ts_irq
>
> The E810 device has support for a "low latency" firmware interface to access and read the Tx timestamps. This interface does not use the standard Tx timestamp logic, due to the latency overhead of proxying sideband command requests over the firmware AdminQ.
>
> The logic still makes use of the Tx timestamp tracking structure, ice_ptp_tx, as it uses the same "ready" bitmap to track which Tx timestamps.
>
> Unfortunately, the ice_ptp_ts_irq() function does not check if the tracker is initialized before its first access. This results in NULL dereference or use-after-free bugs similar to the following:
>
> [245977.278756] BUG: kernel NULL pointer dereference, address: 0000000000000000 [245977.278774] RIP: 0010:_find_first_bit+0x19/0x40 [245977.278796] Call Trace:
[245977.278809] ? ice_misc_intr+0x364/0x380 [ice]
>
> This can occur if a Tx timestamp interrupt races with the driver reset logic.
>
> Fix this by only checking the in_use bitmap (and other fields) if the tracker is marked as initialized. The reset flow will clear the init field under lock before it tears the tracker down, thus preventing any use-after-free or NULL access.
>
> Fixes: f9472aaabd1f ("ice: Process TSYN IRQ in a separate function")
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice_ptp.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
Tested-by: Rinitha S <sx.rinitha@intel.com> (A Contingent worker at Intel)
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [Intel-wired-lan] [PATCH iwl-net 2/2] ice: fix NULL access of tx->in_use in ice_ll_ts_intr
2025-08-07 17:35 ` [PATCH iwl-net 2/2] ice: fix NULL access of tx->in_use in ice_ll_ts_intr Jacob Keller
2025-08-07 21:32 ` [Intel-wired-lan] " Paul Menzel
@ 2025-08-29 6:04 ` Rinitha, SX
1 sibling, 0 replies; 12+ messages in thread
From: Rinitha, SX @ 2025-08-29 6:04 UTC (permalink / raw)
To: Keller, Jacob E, Kitszel, Przemyslaw, Intel Wired LAN,
netdev@vger.kernel.org
Cc: Keller, Jacob E
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Jacob Keller
> Sent: 07 August 2025 23:05
> To: Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Intel Wired LAN <intel-wired-lan@lists.osuosl.org>; netdev@vger.kernel.org
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-net 2/2] ice: fix NULL access of tx->in_use in ice_ll_ts_intr
>
> Recent versions of the E810 firmware have support for an extra interrupt to handle report of the "low latency" Tx timestamps coming from the specialized low latency firmware interface. Instead of polling the registers, software can wait until the low latency interrupt is fired.
>
> This logic makes use of the Tx timestamp tracking structure, ice_ptp_tx, as it uses the same "ready" bitmap to track which Tx timestamps.
>
> Unfortunately, the ice_ll_ts_intr() function does not check if the tracker is initialized before its first access. This results in NULL dereference or use-after-free bugs similar to the issues fixed in the
> ice_ptp_ts_irq() function.
>
> Fix this by only checking the in_use bitmap (and other fields) if the tracker is marked as initialized. The reset flow will clear the init field under lock before it tears the tracker down, thus preventing any use-after-free or NULL access.
>
> Fixes: 82e71b226e0e ("ice: Enable SW interrupt from FW for LL TS")
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice_main.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
Tested-by: Rinitha S <sx.rinitha@intel.com> (A Contingent worker at Intel)
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-08-29 6:04 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-07 17:35 [PATCH iwl-net 0/2] ice: fix NULL access of tx->in_use Jacob Keller
2025-08-07 17:35 ` [PATCH iwl-net 1/2] ice: fix NULL access of tx->in_use in ice_ptp_ts_irq Jacob Keller
2025-08-07 17:56 ` Jacob Keller
2025-08-07 21:29 ` [Intel-wired-lan] " Paul Menzel
2025-08-07 23:56 ` Jacob Keller
2025-08-08 6:47 ` Loktionov, Aleksandr
2025-08-08 6:48 ` Loktionov, Aleksandr
2025-08-29 6:01 ` Rinitha, SX
2025-08-07 17:35 ` [PATCH iwl-net 2/2] ice: fix NULL access of tx->in_use in ice_ll_ts_intr Jacob Keller
2025-08-07 21:32 ` [Intel-wired-lan] " Paul Menzel
2025-08-07 23:53 ` Jacob Keller
2025-08-29 6:04 ` Rinitha, SX
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).