Re: [PATCH iwl-net v2 3/5] iavf: prevent VSI corruption when ring params changed during reset
flat view
From: Simon Horman <horms@kernel.org>
Date: 2026-09-18 15:12:50
Also in:
intel-wired-lan
On Tue, Sep 15, 2026 at 02:55:49PM +0200, Aleksandr Loktionov wrote:
quoted hunk ↗ jump to hunk
From: Sylwester Dziedziuch <redacted> 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 <redacted> Signed-off-by: Aleksandr Loktionov <redacted> --- 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.