[PATCH AUTOSEL 6.12 04/12] net: thunderx: avoid direct MTU assignment after WRITE_ONCE()

[Sasha Levin]

From: Alok Tiwari <alok.a.tiwari at oracle.com>

[ Upstream commit 849704b8b2115647e12436e5076b8e7a4944f21a ]

The current logic in nicvf_change_mtu() writes the new MTU to
netdev->mtu using WRITE_ONCE() before verifying if the hardware
update succeeds. However on hardware update failure, it attempts
to revert to the original MTU using a direct assignment
(netdev->mtu = orig_mtu)
which violates the intended of WRITE_ONCE protection introduced in
commit 1eb2cded45b3 ("net: annotate writes on dev->mtu from
ndo_change_mtu()")

Additionally, WRITE_ONCE(netdev->mtu, new_mtu) is unnecessarily
performed even when the device is not running.

Fix this by:
  Only writing netdev->mtu after successfully updating the hardware.
  Skipping hardware update when the device is down, and setting MTU
  directly. Remove unused variable orig_mtu.

This ensures that all writes to netdev->mtu are consistent with
WRITE_ONCE expectations and avoids unintended state corruption
on failure paths.

Signed-off-by: Alok Tiwari <alok.a.tiwari at oracle.com>
Reviewed-by: Jacob Keller <jacob.e.keller at intel.com>
Link: https://patch.msgid.link/20250706194327.1369390-1-alok.a.tiwari@oracle.com
Signed-off-by: Jakub Kicinski <kuba at kernel.org>
Signed-off-by: Sasha Levin <sashal at kernel.org>
---

LLM Generated explanations, may be completely bogus:

**YES**, this commit should be backported to stable kernel trees.

## Extensive Explanation:

### 1. **This fixes a real bug introduced by incomplete conversion to
WRITE_ONCE()**

The commit message clearly identifies that this fixes a violation of
WRITE_ONCE() protection introduced in commit 1eb2cded45b3. Looking at
the code changes:

```c
// BEFORE - Problematic code:
int orig_mtu = netdev->mtu;
WRITE_ONCE(netdev->mtu, new_mtu);  // Uses WRITE_ONCE

if (!netif_running(netdev))
    return 0;

if (nicvf_update_hw_max_frs(nic, new_mtu)) {
    netdev->mtu = orig_mtu;  // Direct assignment - VIOLATES WRITE_ONCE!
    return -EINVAL;
}
```

This mixing of WRITE_ONCE() and direct assignment breaks the memory
ordering guarantees that WRITE_ONCE() is designed to provide,
potentially causing data races.

### 2. **The fix is minimal and correct**

The fixed code properly ensures all MTU writes use WRITE_ONCE():

```c
// AFTER - Fixed code:
if (netif_running(netdev) && nicvf_update_hw_max_frs(nic, new_mtu))
    return -EINVAL;

WRITE_ONCE(netdev->mtu, new_mtu);  // Only update after HW success
```

The fix:
- Removes the problematic revert path entirely
- Only updates MTU after confirming hardware update succeeds
- Maintains WRITE_ONCE() consistency throughout

### 3. **This affects stable kernels v6.10+**

>From my analysis:
- Commit 1eb2cded45b3 that introduced WRITE_ONCE() annotations was
  merged in v6.10-rc1
- This means all stable kernels from v6.10 onwards have the incomplete
  WRITE_ONCE() conversion
- The thunderx driver's error path was missed in that mass conversion

### 4. **The bug has real potential for causing issues**

While the commit message doesn't explicitly state observed failures, the
inconsistent use of WRITE_ONCE() can cause:
- Data races when MTU is read concurrently without locks (using
  READ_ONCE())
- Potential for seeing stale or partially updated MTU values
- Violation of the memory ordering guarantees the kernel depends on

### 5. **Meets all stable kernel criteria**

According to stable kernel rules, this commit:
- ✓ Fixes a bug (data race/memory ordering issue)
- ✓ Is minimal (12 lines changed)
- ✓ Has been reviewed and accepted upstream
- ✓ Doesn't add new features
- ✓ The fix is obviously correct

### 6. **Historical context from similar commits**

Looking at the reference commits provided:
- Similar commit #1 (1eb2cded45b3) with "Backport Status: NO" was the
  mass conversion that introduced WRITE_ONCE() - this was a large change
  touching 153 files
- However, this current commit is different - it's a targeted bug fix
  for an error path that was missed

The fact that the mass conversion wasn't backported doesn't mean
individual bug fixes for violations shouldn't be backported.

### Recommendation:

This commit should be submitted to stable maintainers for inclusion in
v6.10+ stable branches with appropriate Fixes: tag referencing commit
1eb2cded45b3.

 drivers/net/ethernet/cavium/thunder/nicvf_main.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index aebb9fef3f6eb..1be2dc40a1a63 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -1578,7 +1578,6 @@ int nicvf_open(struct net_device *netdev)
 static int nicvf_change_mtu(struct net_device *netdev, int new_mtu)
 {
 	struct nicvf *nic = netdev_priv(netdev);
-	int orig_mtu = netdev->mtu;
 
 	/* For now just support only the usual MTU sized frames,
 	 * plus some headroom for VLAN, QinQ.
@@ -1589,15 +1588,10 @@ static int nicvf_change_mtu(struct net_device *netdev, int new_mtu)
 		return -EINVAL;
 	}
 
-	WRITE_ONCE(netdev->mtu, new_mtu);
-
-	if (!netif_running(netdev))
-		return 0;
-
-	if (nicvf_update_hw_max_frs(nic, new_mtu)) {
-		netdev->mtu = orig_mtu;
+	if (netif_running(netdev) && nicvf_update_hw_max_frs(nic, new_mtu))
 		return -EINVAL;
-	}
+
+	WRITE_ONCE(netdev->mtu, new_mtu);
 
 	return 0;
 }
-- 
2.39.5