netdev - Re: [PATCH iwl-net 1/2] ice: fix NULL access of tx->in_use in ice_ptp_ts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a59d0242-b89e-4789-8269-43ee3534b843@intel.com>
Date: Thu, 7 Aug 2025 10:56:09 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Przemek Kitszel <przemyslaw.kitszel@...el.com>, Intel Wired LAN
	<intel-wired-lan@...ts.osuosl.org>, <netdev@...r.kernel.org>
Subject: Re: [PATCH iwl-net 1/2] ice: fix NULL access of tx->in_use in
 ice_ptp_ts_irq



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@...el.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;
> 



Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (237 bytes)