netdev - Re: [PATCH net v2] net: atlantic: fix overwritten return value in Aquantia driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250729132808.GD1877762@horms.kernel.org>
Date: Tue, 29 Jul 2025 14:28:08 +0100
From: Simon Horman <horms@...nel.org>
To: Andrew Lunn <andrew@...n.ch>
Cc: Tian <27392025k@...il.com>, irusskikh@...vell.com,
	netdev@...r.kernel.org, andrew+netdev@...n.ch, davem@...emloft.net,
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH net v2] net: atlantic: fix overwritten return value in
 Aquantia driver

On Tue, Jul 29, 2025 at 03:39:01AM +0200, Andrew Lunn wrote:
> > This patch uses `err |=` instead of assignment, so that any earlier error
> > is preserved even if later calls succeed. This is safe because all involved
> > functions return standard negative error codes (e.g. -EIO, -ETIMEDOUT, etc).
> > OR-ing such values does not convert them into success, and preserves the
> > indication that an error occurred.
> 
> 22 | 110 = 132 = ERFKILL
> 
> How easy will it be for somebody to debug that?

Hi Andrew and Tian,

I agree that this does not seem to be a good approach.

Looking over the code, it seems that we can just bail
out if an error occurs. Returning error codes as they arise.

Does this seem reasonable to you?

diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
index 7e88d7234b14..6c490f61ce5c 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
@@ -492,6 +492,9 @@ static int hw_atl_utils_init_ucp(struct aq_hw_s *self,
 					self, self->mbox_addr,
 					self->mbox_addr != 0U,
 					1000U, 10000U);
+	if (err)
+		return err;
+
 	err = readx_poll_timeout_atomic(aq_fw1x_rpc_get, self,
 					self->rpc_addr,
 					self->rpc_addr != 0U,
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
index 4d4cfbc91e19..48f7c863c04b 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
@@ -101,11 +101,15 @@ static int aq_fw2x_init(struct aq_hw_s *self)
 					self, self->mbox_addr,
 					self->mbox_addr != 0U,
 					1000U, 10000U);
+	if (err)
+		return err;
 
 	err = readx_poll_timeout_atomic(aq_fw2x_rpc_get,
 					self, self->rpc_addr,
 					self->rpc_addr != 0U,
 					1000U, 100000U);
+	if (err)
+		return err;
 
 	err = aq_fw2x_settings_get(self, &self->settings_addr);