[PATCH 1/2] myri10ge: avoid passing uninitialized data to inline asm

[PATCH 1/2] myri10ge: avoid passing uninitialized data to inline asm

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

Building with gcc-16.0.1 find a bug with the inline asm:

In file included from include/linux/io.h:12,
                 from drivers/net/ethernet/myricom/myri10ge/myri10ge.c:43:
In function '__const_memcpy_toio_aligned64',
    inlined from '__iowrite64_copy' at arch/arm64/include/asm/io.h:249:3,
    inlined from 'myri10ge_dummy_rdma' at drivers/net/ethernet/myricom/myri10ge/myri10ge.c:535:2:
arch/arm64/include/asm/io.h:206:17: error: '*(const u64 *)(&buf[6])' is used uninitialized [-Werror=uninitialized]
  206 |                 asm volatile("str %x0, [%8, #8 * 0]\n"
      |                 ^~~

Set the extra fields to zero before writing them to hardware.

Fixes: 0da34b6dfe55 ("[PATCH] Add Myri-10G Ethernet driver")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/myricom/myri10ge/myri10ge.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
index 7be30a8df268..cea28aa971cc 100644
--- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
+++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
@@ -508,7 +508,7 @@ static int myri10ge_read_mac_addr(struct myri10ge_priv *mgp)
 static void myri10ge_dummy_rdma(struct myri10ge_priv *mgp, int enable)
 {
 	char __iomem *submit;
-	__be32 buf[16] __attribute__ ((__aligned__(8)));
+	__be32 buf[16] __attribute__ ((__aligned__(8))) = {};
 	u32 dma_low, dma_high;
 	int i;
 
-- 
2.39.5

[PATCH 2/2] [v3] myri10ge: avoid uninitialized variable use

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

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>
---
v2: initialize two more instances of these [Simon Horman]
v3: add one more missing instance
---
 .../net/ethernet/myricom/myri10ge/myri10ge.c  | 26 ++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
index cea28aa971cc..b699d50e227e 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,6 +2250,8 @@ 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 *)
@@ -2312,6 +2326,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 +2429,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 +2489,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 +2507,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 +2977,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.39.5

Re: [2/2,v3] myri10ge: avoid uninitialized variable use

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
myri10ge: avoid uninitialized variable use

This patch fixes uninitialized variable warnings found by gcc-10 on s390
by initializing cmd.data0, cmd.data1, and cmd.data2 before calls to
myri10ge_send_cmd() across multiple functions.

> diff --git a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
> --- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
> +++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c

[ ... ]

> @@ -2238,6 +2250,8 @@ 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;
>  	status |= myri10ge_send_cmd(mgp, MXGEFW_CMD_GET_SMALL_RX_OFFSET,
>  				    &cmd, 0);

The initialization of cmd.data1 and cmd.data2 is inside the conditional
block. When slice > 0 and real_num_tx_queues <= 1, does the subsequent
myri10ge_send_cmd() call for MXGEFW_CMD_GET_SMALL_RX_OFFSET use
uninitialized data1/data2 values?

Simon Horman pointed this out in his review of v2:
https://lore.kernel.org/all/20250622162715.GA297140@horms.kernel.org/

He provided a diff showing that the second cmd.data0 = slice assignment
also needs cmd.data1 and cmd.data2 initialization. This feedback does
not appear to have been addressed in v3.
-- 
pw-bot: cr

Re: [2/2,v3] myri10ge: avoid uninitialized variable use

[Arnd Bergmann]

On Thu, Feb 5, 2026, at 05:57, Jakub Kicinski wrote:
>> @@ -2238,6 +2250,8 @@ 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;
>>  	status |= myri10ge_send_cmd(mgp, MXGEFW_CMD_GET_SMALL_RX_OFFSET,
>>  				    &cmd, 0);
>
> The initialization of cmd.data1 and cmd.data2 is inside the conditional
> block. When slice > 0 and real_num_tx_queues <= 1, does the subsequent
> myri10ge_send_cmd() call for MXGEFW_CMD_GET_SMALL_RX_OFFSET use
> uninitialized data1/data2 values?
>
> Simon Horman pointed this out in his review of v2:
> https://lore.kernel.org/all/20250622162715.GA297140@horms.kernel.org/
>
> He provided a diff showing that the second cmd.data0 = slice assignment
> also needs cmd.data1 and cmd.data2 initialization. This feedback does
> not appear to have been addressed in v3.

Indeed, I thought I had addressed this, but somehow ended up
rebasing to an older version of my original patch. Sent v4 now.

     Arnd