Re: [PATCH iwl-next v4 1/3] ice: Convert ctrl_pf pointer in struct ice_adapter to RCU

[Przemek Kitszel <przemyslaw.kitszel@intel.com>]

On 8/31/26 18:10, Sergey Temerkhanov wrote:
> Use RCU to ensure the consistent state of the control PF global
> pointer contained in struct ice_adapter. Enforce RCU usage on
> the callers.
> 
> Fix a potential invalid pointer return due a TOCTOU issue

this is rather convincing/self explanatory
but the rule is "Fixes tags are for actual bug fixes, not theoretical
ones". (same comment for other patches).

Would be best if you could describe how to trigger the bug.
Or just drop the tag (since you target next- anyway).

> 
> Fixes: e2193f9f9ec9 ("ice: enable timesync operation on 2xNAC E825 devices")
> Signed-off-by: Sergey Temerkhanov <sergey.temerkhanov@intel.com>
> Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> Tested-by: Frederick Lawler <fred@cloudflare.com>

[...]

> --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> @@ -1,6 +1,7 @@
>   // SPDX-License-Identifier: GPL-2.0
>   /* Copyright (C) 2021, Intel Corporation. */
>   
> +#include <linux/cleanup.h>
>   #include <linux/delay.h>
>   #include <linux/iopoll.h>
>   #include "ice_common.h"
> @@ -335,6 +336,8 @@ void ice_ptp_src_cmd(struct ice_hw *hw, enum ice_ptp_tmr_cmd cmd)
>   	struct ice_pf *pf = container_of(hw, struct ice_pf, hw);
>   	u32 cmd_val = ice_ptp_tmr_cmd_to_src_reg(hw, cmd);
>   
> +	guard(rcu)();
> +
>   	if (!ice_is_primary(hw))
>   		hw = ice_get_primary_hw(pf);
>   
> @@ -353,6 +356,8 @@ static void ice_ptp_exec_tmr_cmd(struct ice_hw *hw)
>   {
>   	struct ice_pf *pf = container_of(hw, struct ice_pf, hw);
>   
> +	guard(rcu)();
> +
>   	if (!ice_is_primary(hw))
>   		hw = ice_get_primary_hw(pf);
>   
> @@ -2004,6 +2009,8 @@ static int ice_read_phy_and_phc_time_eth56g(struct ice_hw *hw, u8 port,
>   		zo = rd32(hw, GLTSYN_SHTIME_0(tmr_idx));
>   		lo = rd32(hw, GLTSYN_SHTIME_L(tmr_idx));
>   	} else {
> +		guard(rcu)();
> +
>   		zo = rd32(ice_get_primary_hw(pf), GLTSYN_SHTIME_0(tmr_idx));
>   		lo = rd32(ice_get_primary_hw(pf), GLTSYN_SHTIME_L(tmr_idx));
>   	}
> @@ -2173,6 +2180,8 @@ int ice_start_phy_timer_eth56g(struct ice_hw *hw, u8 port)
>   		lo = rd32(hw, GLTSYN_INCVAL_L(tmr_idx));
>   		hi = rd32(hw, GLTSYN_INCVAL_H(tmr_idx));
>   	} else {
> +		guard(rcu)();
> +
>   		lo = rd32(ice_get_primary_hw(pf), GLTSYN_INCVAL_L(tmr_idx));
>   		hi = rd32(ice_get_primary_hw(pf), GLTSYN_INCVAL_H(tmr_idx));
>   	}

guard() looks well suited for your use cases

> @@ -215,15 +221,18 @@ int ice_txclk_set_clk(struct ice_pf *pf, enum ice_e825c_ref_clk clk)
>   void ice_txclk_update_and_notify(struct ice_pf *pf)
>   {
>   	struct ice_ptp_port *ptp_port = &pf->ptp.port;
> -	struct ice_pf *ctrl_pf = ice_get_ctrl_pf(pf);
>   	struct dpll_pin *old_pin = NULL;
>   	struct dpll_pin *new_pin = NULL;
> +	struct ice_pf *ctrl_pf;

RCT rule counts whole line length, including assignement.

>   	struct ice_hw *hw = &pf->hw;
>   	enum ice_e825c_ref_clk clk;
>   	bool notify_dpll = false;
>   	int err;
>   	u8 phy;
>   
> +	guard(rwsem_read)(&pf->adapter->ctrl_pf_lock);
> +	ctrl_pf = ice_get_ctrl_pf(pf);
> +
>   	phy = ptp_port->port_num / hw->ptp.ports_per_phy;
>   
>   	/* Hold txclk_notify_rwsem for read across the entire critical