netdev - Re: [net-next,2/3] net: dsa: yt921x: Return early for failed MIB read

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: <20260205050813.2421675-1-kuba@kernel.org>
Date: Wed,  4 Feb 2026 21:08:13 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: mmyangfl@...il.com
Cc: Jakub Kicinski <kuba@...nel.org>,
	olteanv@...il.com,
	davem@...emloft.net,
	edumazet@...gle.com,
	linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org,
	pabeni@...hat.com,
	andrew@...n.ch
Subject: Re: [net-next,2/3] net: dsa: yt921x: Return early for failed MIB read

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
---
net: dsa: yt921x: Return early for failed MIB read

This patch refactors error handling in yt921x_read_mib() by checking
for read failures earlier and returning immediately, before attempting
to compute statistics from potentially incomplete data.

> net: dsa: yt921x: Return early for failed MIB read
>
> This patch does not change anything effectively, but serves as a
> prerequisite for another patch.

The claim "This patch does not change anything effectively" is not
accurate. The patch changes the behavior when MIB register reads fail.

Before the patch: If yt921x_reg_read() fails partway through the loop,
the function would still compute pp->rx_frames and pp->tx_frames using
the partially-read MIB values before returning the error code. The frame
counts would contain a mix of old values (for registers read before the
error) and new values (for registers read successfully).

After the patch: If yt921x_reg_read() fails, the function returns
immediately without updating pp->rx_frames and pp->tx_frames, leaving
them at their previous values from the last successful read.

This affects how five callers behave when reads fail:
  - yt921x_dsa_get_pause_stats()
  - yt921x_dsa_get_eth_mac_stats()
  - yt921x_dsa_get_rmon_stats()
  - yt921x_dsa_get_eth_ctrl_stats()
  - yt921x_dsa_get_ethtool_stats()

These functions don't check the return value, so they will now report
stale data from the last successful read instead of partially-updated
data from a failed read.

Should the commit message be updated to accurately describe this
behavior change?

> diff --git a/drivers/net/dsa/yt921x.c b/drivers/net/dsa/yt921x.c
> --- a/drivers/net/dsa/yt921x.c
> +++ b/drivers/net/dsa/yt921x.c
> @@ -714,6 +714,11 @@ static int yt921x_read_mib(struct yt921x_priv *priv, int port)
>  		WRITE_ONCE(*valp, val);
>  	}
>
> +	if (res) {
> +		print_port_err(dev, port, res, "read stats for");
> +		return res;
> +	}
> +
>  	pp->rx_frames = mib->rx_64byte + mib->rx_65_127byte +
>  			mib->rx_128_255byte + mib->rx_256_511byte +
>  			mib->rx_512_1023byte + mib->rx_1024_1518byte +
> @@ -723,9 +728,7 @@
>  			mib->tx_512_1023byte + mib->tx_1024_1518byte +
>  			mib->tx_jumbo;
>
> -	if (res)
> -		print_port_err(dev, port, res, "read stats for");
> -	return res;
> +	return 0;
>  }
-- 
pw-bot: cr