Re: [PATCH iwl-net 01/12] ice: use reference counting and RCU for PTP port access

[Jacob Keller <jacob.e.keller@intel.com>]

On 8/21/2026 5:13 PM, Jacob Keller wrote:
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h b/drivers/net/ethernet/intel/ice/ice_ptp.h
> index c4b0da7ce20e..da2003ba3bb0 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.h
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.h
> @@ -673,20 +675,33 @@ static void ice_ptp_process_tx_tstamp(struct ice_ptp_tx *tx)
>  	pf->ptp.tx_hwtstamp_good += tstamp_good;
>  }
>  
> +static void ice_ptp_release_port_rcu(struct kref *ref)
> +{
> +	wake_up_var(ref);
> +}
> +
>  static void ice_ptp_tx_tstamp_owner(struct ice_pf *pf)
>  {
>  	struct ice_ptp_port *port;
>  
> -	mutex_lock(&pf->adapter->ports.lock);
> -	list_for_each_entry(port, &pf->adapter->ports.ports, list_node) {
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(port, &pf->adapter->ports.list, list_node) {
>  		struct ice_ptp_tx *tx = &port->tx;
>  
> -		if (!tx || !tx->init)
> +		if (!tx->init)
>  			continue;
>  
> +		if (!kref_get_unless_zero(&port->ref))
> +			continue;
> +
> +		rcu_read_unlock();
> +
>  		ice_ptp_process_tx_tstamp(tx);
> +
> +		kref_put(&port->ref, ice_ptp_release_port_rcu);
> +		rcu_read_lock();
>  	}
> -	mutex_unlock(&pf->adapter->ports.lock);
> +	rcu_read_unlock();
>  }
>  

Sashiko says:

> Can this sequence lead to a use-after-free when advancing the iterator?
> The list_for_each_entry_rcu() macro evaluates port->list_node.next at the end
> of each iteration to find the next element.
> If kref_put() drops the last reference here, it wakes up the teardown thread
> in ice_ptp_cleanup_pf().
> Since the iterator thread previously dropped the RCU read lock via
> rcu_read_unlock(), could the following race happen?
> Iterator thread:
>     kref_put(&port->ref, ice_ptp_release_port_rcu);
>     // Last ref dropped, wakes up ice_ptp_cleanup_pf()
> Teardown thread (ice_ptp_cleanup_pf):
>     wakes up from wait_var_event_timeout()
>     synchronize_rcu(); // Completes without waiting for iterator thread
>     // Port memory is freed
> Iterator thread:
>     rcu_read_lock();
>     // list_for_each_entry_rcu evaluates port->list_node.next on freed port
> Is it necessary to use a safe iterator or pre-fetch the next pointer before
> dropping the reference, to prevent reading from the freed port?
> This same traversal pattern dropping the lock and reference appears to also
> be present in ice_ptp_flush_all_tx_tracker(), ice_ptp_restart_all_phy(),
> and ice_ptp_prepare_rebuild_sec().

I am not sure there is a problem here. I need to investigate carefully.

I think there is a problem here because the original logic was built
around xarray iterators which may not work exactly the same as the list
iterator. We currently remove the item from the list before releasing
the final reference.

I believe that worked for the xarray but doesn't work for this list
based approach. I think the best solution here is to follow the guidance
from Documentation/core-api/kref.rst under the kref + RCU section.

I will fix this in a v2, along with any other issues pointed out by sashiko