[PATCH net v3] net: dsa: motorcomm: Prevent rx_bytes/tx_bytes underflow

[PATCH net v3] net: dsa: motorcomm: Prevent rx_bytes/tx_bytes underflow

[David Yang]

There is no direct rx_bytes/tx_bytes counter excluding the FCS; both
have to be derived as good_bytes - ETH_FCS_LEN * frames. Three hazards
can pair a frame count with a mismatched byte count and make the
subtraction underflow, reporting rx_bytes or tx_bytes as about 2^64:

- a register read fails midway and the half-refreshed buffer is
  published as is, pairing fresh frame counters with stale byte
  counters;
- get_stats64() on 64-bit kernels reads the published counters without
  any sequencing across fields (u64_stats_sync is a no-op there), so a
  writer update can slip between the frame and byte reads;
- the MIB bank cannot be read as a whole; counters keep counting while
  the read loop walks the table.

Handle this in the writer: do not publish a half-read buffer, keep the
last complete sample on read failure, do the arithmetic on the complete
sample under priv->reg_lock, and publish the derived byte counters only
when check_sub_overflow() succeeds, so that get_stats64() copies plain
published values.

Found by LLM.

Fixes: 186623f4aa72 ("net: dsa: yt921x: Add support for Motorcomm YT921x")
Signed-off-by: David Yang <mmyangfl@gmail.com>
---
v3:
 - prevent underflow anyway
v2: https://lore.kernel.org/r/20261103134705.867613-1-mmyangfl@gmail.com
 - more issues found by sashiko
v1: https://lore.kernel.org/r/20261102101334.3525485-1-mmyangfl@gmail.com
 drivers/net/dsa/motorcomm/mib.c | 25 +++++++++++++++++--------
 drivers/net/dsa/motorcomm/mib.h |  2 ++
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/net/dsa/motorcomm/mib.c b/drivers/net/dsa/motorcomm/mib.c
index 6b170fc6e187..09b3e6b98bdd 100644
--- a/drivers/net/dsa/motorcomm/mib.c
+++ b/drivers/net/dsa/motorcomm/mib.c
@@ -95,6 +95,8 @@ static int yt921x_mib_read(struct yt921x_priv *priv, int port)
 	unsigned long flags;
 	u64 rx_frames;
 	u64 tx_frames;
+	u64 rx_bytes;
+	u64 tx_bytes;
 	int res = 0;
 
 	mib = &pm->stats;
@@ -127,6 +129,12 @@ static int yt921x_mib_read(struct yt921x_priv *priv, int port)
 		buf[i] = val;
 	}
 
+	if (res) {
+		dev_err(dev, "Failed to %s port %d: %i\n", "read stats for",
+			port, res);
+		return res;
+	}
+
 	flags = u64_stats_update_begin_irqsave(&pm->syncp);
 	for (size_t i = 0; i < ARRAY_SIZE(yt921x_mib_descs); i++)
 		u64_stats_set(&((u64_stats_t *)mib)[i], buf[i]);
@@ -146,12 +154,15 @@ static int yt921x_mib_read(struct yt921x_priv *priv, int port)
 		    u64_stats_read(&mib->tx_jumbo);
 	u64_stats_set(&pm->rx_frames, rx_frames);
 	u64_stats_set(&pm->tx_frames, tx_frames);
+	if (!check_sub_overflow(u64_stats_read(&mib->rx_good_bytes),
+				ETH_FCS_LEN * rx_frames, &rx_bytes))
+		u64_stats_set(&pm->rx_bytes, rx_bytes);
+	if (!check_sub_overflow(u64_stats_read(&mib->tx_good_bytes),
+				ETH_FCS_LEN * tx_frames, &tx_bytes))
+		u64_stats_set(&pm->tx_bytes, tx_bytes);
 	u64_stats_update_end_irqrestore(&pm->syncp, flags);
 
-	if (res)
-		dev_err(dev, "Failed to %s port %d: %i\n", "read stats for",
-			port, res);
-	return res;
+	return 0;
 }
 
 void yt921x_mib_poll(struct work_struct *work)
@@ -391,10 +402,8 @@ yt921x_dsa_get_stats64(struct dsa_switch *ds, int port,
 
 		stats->rx_packets = u64_stats_read(&pm->rx_frames);
 		stats->tx_packets = u64_stats_read(&pm->tx_frames);
-		stats->rx_bytes = u64_stats_read(&mib->rx_good_bytes) -
-				  ETH_FCS_LEN * stats->rx_packets;
-		stats->tx_bytes = u64_stats_read(&mib->tx_good_bytes) -
-				  ETH_FCS_LEN * stats->tx_packets;
+		stats->rx_bytes = u64_stats_read(&pm->rx_bytes);
+		stats->tx_bytes = u64_stats_read(&pm->tx_bytes);
 		stats->rx_dropped = u64_stats_read(&mib->rx_dropped);
 		/* stats->tx_dropped */
 		stats->multicast = u64_stats_read(&mib->rx_multicast);
diff --git a/drivers/net/dsa/motorcomm/mib.h b/drivers/net/dsa/motorcomm/mib.h
index 9c24a52d46cc..2e2b832cb245 100644
--- a/drivers/net/dsa/motorcomm/mib.h
+++ b/drivers/net/dsa/motorcomm/mib.h
@@ -136,6 +136,8 @@ struct yt921x_mib {
 	struct yt921x_mib_stats stats;
 	u64_stats_t rx_frames;
 	u64_stats_t tx_frames;
+	u64_stats_t rx_bytes;
+	u64_stats_t tx_bytes;
 	/* protected by priv->reg_lock */
 	u64 data[YT921X_MIB_STAT_NUM];
 };
-- 
2.53.0

Re: [PATCH net v3] net: dsa: motorcomm: Prevent rx_bytes/tx_bytes underflow

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net v3] net: dsa: motorcomm: Prevent rx_bytes/tx_bytes underflow

[David Yang]

On Sat, Nov 7, 2026 at 10:00 AM <netdev-bot+sinfo@kernel.org> wrote:
>
> Hi!
>
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
>
>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by code inspection). If it was triggered please include
>    the symptoms, like the stack trace or error messages.
>
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
>
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

code inspection only

Re: [PATCH net v3] net: dsa: motorcomm: Prevent rx_bytes/tx_bytes underflow

[Andrew Lunn]

> +	if (res) {
> +		dev_err(dev, "Failed to %s port %d: %i\n", "read stats for",
> +			port, res);

Please don't break strings like that. The reason we allow lines with
strings to be longer than 80 is because somebody is likely to grep the
error message in the code to find out why it is being printed.

grep -r "Failed to read stats for" is not going to match.

    Andrew

---
pw-bot: cr