Re: [PATCH iwl-net v2 3/5] iavf: prevent VSI corruption when ring params changed during reset

[Simon Horman <horms@kernel.org>]

On Tue, Sep 15, 2026 at 02:55:49PM +0200, Aleksandr Loktionov wrote:
> From: Sylwester Dziedziuch <sylwesterx.dziedziuch@intel.com>
> 
> Changing ring parameters via ethtool triggers a VF reset and queue
> reconfiguration. If ethtool is called again before the first reset
> completes, the second reset races with uninitialised queue state and
> can corrupt the VSI resource tree on the PF side.
> 
> Return -EAGAIN from iavf_set_ringparam() when the adapter is already
> resetting or its queues are disabled.
> 
> Fixes: fbb7ddfef253 ("i40evf: core ethtool functionality")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sylwester Dziedziuch <sylwesterx.dziedziuch@intel.com>
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> ---
>  drivers/net/ethernet/intel/iavf/iavf_ethtool.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> index 1cd1f3f..3909131 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> @@ -495,6 +495,11 @@ static int iavf_set_ringparam(struct net_device *netdev,
>  	if ((ring->rx_mini_pending) || (ring->rx_jumbo_pending))
>  		return -EINVAL;
>  
> +	if (adapter->state == __IAVF_RESETTING ||
> +	    (adapter->state == __IAVF_RUNNING &&
> +	     adapter->flags & IAVF_FLAG_QUEUES_DISABLED))
> +		return -EAGAIN;
> +
>  	if (ring->tx_pending > IAVF_MAX_TXD ||
>  	    ring->tx_pending < IAVF_MIN_TXD ||
>  	    ring->rx_pending > IAVF_MAX_RXD ||

I believe I reported this for v1 as well, but AI-generated review reports
a TOCTOU issue here. And I believe a similar, pre-existing, problem also
exists in iavf_set_channels.

Text from
https://sashiko.dev/#/patchset/20260915125551.3976068-1-aleksandr.loktionov%40intel.com
follows:

Could this state check introduce a time-of-check to time-of-use (TOCTOU) race
against the background reset task?

The background iavf_reset_task() relies on netdev_lock() to serialize, but
ethtool callbacks for drivers like iavf (which do not set request_ops_lock =
true) are executed under rtnl_lock() without holding netdev_lock().

Because they do not share a common lock, the background task can acquire
netdev_lock(), change the state to __IAVF_RESETTING, and tear down queues
immediately after iavf_set_ringparam() evaluates this condition to false.

Will this lock mismatch also cause a lockdep splat later in the same function?

iavf_set_ringparam() proceeds to call iavf_reset_step() directly:
drivers/net/ethernet/intel/iavf/iavf_ethtool.c:iavf_set_ringparam() {
...
	if (netif_running(netdev)) {
		adapter->flags |= IAVF_FLAG_RESET_NEEDED;
		iavf_reset_step(adapter);
	}
...
}

Looking at iavf_reset_step(), it asserts that netdev_lock() is held:

drivers/net/ethernet/intel/iavf/iavf_main.c:iavf_reset_step() {
...
	netdev_assert_locked(netdev);
...
}

Executing iavf_reset_step() from the ethtool callback without holding
netdev_lock() would trigger this lockdep assertion, and allow it to
concurrently step on any executing reset task.