[PATCH AUTOSEL 6.19-5.10] myri10ge: avoid uninitialized variable use

[Sasha Levin <sashal@kernel.org>]

From: Arnd Bergmann <arnd@arndb.de>

[ Upstream commit fd24173439c033ffb3c2a2628fcbc9cb65e62bdb ]

While compile testing on less common architectures, I noticed that gcc-10 on
s390 finds a bug that all other configurations seem to miss:

drivers/net/ethernet/myricom/myri10ge/myri10ge.c: In function 'myri10ge_set_multicast_list':
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:391:25: error: 'cmd.data0' is used uninitialized in this function [-Werror=uninitialized]
  391 |  buf->data0 = htonl(data->data0);
      |                         ^~
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:392:25: error: '*((void *)&cmd+4)' is used uninitialized in this function [-Werror=uninitialized]
  392 |  buf->data1 = htonl(data->data1);
      |                         ^~
drivers/net/ethernet/myricom/myri10ge/myri10ge.c: In function 'myri10ge_allocate_rings':
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:392:13: error: 'cmd.data1' is used uninitialized in this function [-Werror=uninitialized]
  392 |  buf->data1 = htonl(data->data1);
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1939:22: note: 'cmd.data1' was declared here
 1939 |  struct myri10ge_cmd cmd;
      |                      ^~~
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:393:13: error: 'cmd.data2' is used uninitialized in this function [-Werror=uninitialized]
  393 |  buf->data2 = htonl(data->data2);
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1939:22: note: 'cmd.data2' was declared here
 1939 |  struct myri10ge_cmd cmd;

It would be nice to understand how to make other compilers catch this as
well, but for the moment I'll just shut up the warning by fixing the
undefined behavior in this driver.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Link: https://patch.msgid.link/20260205162935.2126442-1-arnd@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

### Understanding the Bug

The analysis reveals this is a **real, meaningful bug**, not just a
compiler warning cleanup:

1. **`struct myri10ge_cmd`** has three `u32` fields: `data0`, `data1`,
   `data2`
2. **`myri10ge_send_cmd()`** unconditionally copies ALL THREE fields to
   hardware via:
  ```c
  buf->data0 = htonl(data->data0);
  buf->data1 = htonl(data->data1);
  buf->data2 = htonl(data->data2);
  ```
  Then sends them to the NIC via `myri10ge_pio_copy()`.

3. In many call sites, the `cmd` struct is declared on the stack but
   **not all fields are initialized** before being passed to
   `myri10ge_send_cmd()`. This means **random stack garbage is sent to
   hardware**.

### Bug Impact

- **Undefined behavior**: Uninitialized stack variables being used is
  undefined behavior in C
- **Hardware commands with garbage data**: The NIC firmware receives
  commands with random data in unused fields, which could cause
  unpredictable behavior depending on firmware implementation
- **Potential data corruption or connectivity issues**: If the firmware
  interprets garbage values in unexpected ways
- **Build failure**: On gcc-10 on s390 with `-Werror`, this prevents
  compilation entirely

### Scope of Changes

The fix is purely additive — it adds zero-initialization of `cmd.data0`,
`cmd.data1`, `cmd.data2` at each call site where they were previously
uninitialized. The changes span **one file** with simple, repetitive
initialization statements. There's also a minor cleanup moving the `cmd`
variable declaration to a narrower scope in `myri10ge_close()`.

Functions fixed:
- `myri10ge_get_firmware_capabilities()`
- `myri10ge_update_mac_address()`
- `myri10ge_change_pause()`
- `myri10ge_change_promisc()`
- `myri10ge_allocate_rings()`
- `myri10ge_get_txrx()`
- `myri10ge_open()`
- `myri10ge_close()`
- `myri10ge_set_multicast_list()`

### Stable Kernel Criteria Assessment

| Criterion | Assessment |
|-----------|-----------|
| Obviously correct? | Yes — initializing variables to zero before use
is trivially correct |
| Fixes a real bug? | Yes — undefined behavior (uninitialized data sent
to hardware) and build failure on s390 |
| Small and contained? | Yes — single file, simple initialization
additions |
| No new features? | Correct — purely a bug fix |
| Risk of regression? | Very low — adding zero-initialization cannot
break correct code |

### Risk vs Benefit

- **Risk**: Extremely low. Zero-initializing struct fields before
  passing them to a function is the safest possible change. The only
  conceivable risk is if the hardware somehow relied on receiving
  specific non-zero garbage values, which would be absurd.
- **Benefit**: Fixes undefined behavior, prevents hardware from
  receiving garbage command data, and fixes a build failure on s390 with
  gcc-10.

### Concerns

- The myri10ge driver is for Myricom 10-Gigabit Ethernet adapters, which
  are older hardware (Myricom was acquired by CSPI years ago). However,
  users with this hardware in stable kernels still benefit from correct
  behavior.
- No dependency on other commits — this is fully self-contained.
- Note the use of commas instead of semicolons in some initializations
  (e.g., `cmd.data0 = 0,` instead of `cmd.data0 = 0;`) — this is
  syntactically valid C (comma operator) but unusual. It won't cause
  issues.

### Conclusion

This commit fixes real undefined behavior where uninitialized stack data
is sent to network hardware. It's a textbook example of a safe,
contained bug fix: zero-initializing variables before use in a single
driver file. The fix prevents both runtime undefined behavior and a
build failure on certain architectures. The risk of regression is
negligible.

**YES**

 .../net/ethernet/myricom/myri10ge/myri10ge.c  | 28 ++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
index 7be30a8df2685..2f0cdbd4e2ac9 100644
--- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
+++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
@@ -688,6 +688,9 @@ static int myri10ge_get_firmware_capabilities(struct myri10ge_priv *mgp)
 
 	/* probe for IPv6 TSO support */
 	mgp->features = NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_TSO;
+	cmd.data0 = 0,
+	cmd.data1 = 0,
+	cmd.data2 = 0,
 	status = myri10ge_send_cmd(mgp, MXGEFW_CMD_GET_MAX_TSO6_HDR_SIZE,
 				   &cmd, 0);
 	if (status == 0) {
@@ -806,6 +809,7 @@ static int myri10ge_update_mac_address(struct myri10ge_priv *mgp,
 		     | (addr[2] << 8) | addr[3]);
 
 	cmd.data1 = ((addr[4] << 8) | (addr[5]));
+	cmd.data2 = 0;
 
 	status = myri10ge_send_cmd(mgp, MXGEFW_SET_MAC_ADDRESS, &cmd, 0);
 	return status;
@@ -817,6 +821,9 @@ static int myri10ge_change_pause(struct myri10ge_priv *mgp, int pause)
 	int status, ctl;
 
 	ctl = pause ? MXGEFW_ENABLE_FLOW_CONTROL : MXGEFW_DISABLE_FLOW_CONTROL;
+	cmd.data0 = 0,
+	cmd.data1 = 0,
+	cmd.data2 = 0,
 	status = myri10ge_send_cmd(mgp, ctl, &cmd, 0);
 
 	if (status) {
@@ -834,6 +841,9 @@ myri10ge_change_promisc(struct myri10ge_priv *mgp, int promisc, int atomic)
 	int status, ctl;
 
 	ctl = promisc ? MXGEFW_ENABLE_PROMISC : MXGEFW_DISABLE_PROMISC;
+	cmd.data0 = 0;
+	cmd.data1 = 0;
+	cmd.data2 = 0;
 	status = myri10ge_send_cmd(mgp, ctl, &cmd, atomic);
 	if (status)
 		netdev_err(mgp->dev, "Failed to set promisc mode\n");
@@ -1946,6 +1956,8 @@ static int myri10ge_allocate_rings(struct myri10ge_slice_state *ss)
 	/* get ring sizes */
 	slice = ss - mgp->ss;
 	cmd.data0 = slice;
+	cmd.data1 = 0;
+	cmd.data2 = 0;
 	status = myri10ge_send_cmd(mgp, MXGEFW_CMD_GET_SEND_RING_SIZE, &cmd, 0);
 	tx_ring_size = cmd.data0;
 	cmd.data0 = slice;
@@ -2238,12 +2250,16 @@ static int myri10ge_get_txrx(struct myri10ge_priv *mgp, int slice)
 	status = 0;
 	if (slice == 0 || (mgp->dev->real_num_tx_queues > 1)) {
 		cmd.data0 = slice;
+		cmd.data1 = 0;
+		cmd.data2 = 0;
 		status = myri10ge_send_cmd(mgp, MXGEFW_CMD_GET_SEND_OFFSET,
 					   &cmd, 0);
 		ss->tx.lanai = (struct mcp_kreq_ether_send __iomem *)
 		    (mgp->sram + cmd.data0);
 	}
 	cmd.data0 = slice;
+	cmd.data1 = 0;
+	cmd.data2 = 0;
 	status |= myri10ge_send_cmd(mgp, MXGEFW_CMD_GET_SMALL_RX_OFFSET,
 				    &cmd, 0);
 	ss->rx_small.lanai = (struct mcp_kreq_ether_recv __iomem *)
@@ -2312,6 +2328,7 @@ static int myri10ge_open(struct net_device *dev)
 	if (mgp->num_slices > 1) {
 		cmd.data0 = mgp->num_slices;
 		cmd.data1 = MXGEFW_SLICE_INTR_MODE_ONE_PER_SLICE;
+		cmd.data2 = 0;
 		if (mgp->dev->real_num_tx_queues > 1)
 			cmd.data1 |= MXGEFW_SLICE_ENABLE_MULTIPLE_TX_QUEUES;
 		status = myri10ge_send_cmd(mgp, MXGEFW_CMD_ENABLE_RSS_QUEUES,
@@ -2414,6 +2431,8 @@ static int myri10ge_open(struct net_device *dev)
 
 	/* now give firmware buffers sizes, and MTU */
 	cmd.data0 = dev->mtu + ETH_HLEN + VLAN_HLEN;
+	cmd.data1 = 0;
+	cmd.data2 = 0;
 	status = myri10ge_send_cmd(mgp, MXGEFW_CMD_SET_MTU, &cmd, 0);
 	cmd.data0 = mgp->small_bytes;
 	status |=
@@ -2472,7 +2491,6 @@ static int myri10ge_open(struct net_device *dev)
 static int myri10ge_close(struct net_device *dev)
 {
 	struct myri10ge_priv *mgp = netdev_priv(dev);
-	struct myri10ge_cmd cmd;
 	int status, old_down_cnt;
 	int i;
 
@@ -2491,8 +2509,13 @@ static int myri10ge_close(struct net_device *dev)
 
 	netif_tx_stop_all_queues(dev);
 	if (mgp->rebooted == 0) {
+		struct myri10ge_cmd cmd;
+
 		old_down_cnt = mgp->down_cnt;
 		mb();
+		cmd.data0 = 0;
+		cmd.data1 = 0;
+		cmd.data2 = 0;
 		status =
 		    myri10ge_send_cmd(mgp, MXGEFW_CMD_ETHERNET_DOWN, &cmd, 0);
 		if (status)
@@ -2956,6 +2979,9 @@ static void myri10ge_set_multicast_list(struct net_device *dev)
 
 	/* Disable multicast filtering */
 
+	cmd.data0 = 0;
+	cmd.data1 = 0;
+	cmd.data2 = 0;
 	err = myri10ge_send_cmd(mgp, MXGEFW_ENABLE_ALLMULTI, &cmd, 1);
 	if (err != 0) {
 		netdev_err(dev, "Failed MXGEFW_ENABLE_ALLMULTI, error status: %d\n",
-- 
2.51.0