netdev - Re: [PATCH v2 net-next 3/6] net: dsa: mv88e6xxx: Fix mv88e6352_serdes_get

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20231205175040.mbpepmbtxjkrb4dq@skbuf>
Date: Tue, 5 Dec 2023 19:50:40 +0200
From: Vladimir Oltean <vladimir.oltean@....com>
To: Tobias Waldekranz <tobias@...dekranz.com>
Cc: davem@...emloft.net, kuba@...nel.org, andrew@...n.ch,
	f.fainelli@...il.com, netdev@...r.kernel.org
Subject: Re: [PATCH v2 net-next 3/6] net: dsa: mv88e6xxx: Fix
 mv88e6352_serdes_get_stats error path

On Tue, Dec 05, 2023 at 05:04:15PM +0100, Tobias Waldekranz wrote:
> mv88e6xxx_get_stats, which collects stats from various sources,
> expects all callees to return the number of stats read. If an error
> occurs, 0 should be returned.
> 
> Prevent future mishaps of this kind by updating the return type to
> reflect this contract.
> 
> Signed-off-by: Tobias Waldekranz <tobias@...dekranz.com>
> ---
> diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
> index 3b4b42651fa3..01ea53940786 100644
> --- a/drivers/net/dsa/mv88e6xxx/serdes.c
> +++ b/drivers/net/dsa/mv88e6xxx/serdes.c
> @@ -187,7 +187,7 @@ int mv88e6352_serdes_get_stats(struct mv88e6xxx_chip *chip, int port,
>  
>  	err = mv88e6352_g2_scratch_port_has_serdes(chip, port);
>  	if (err <= 0)
> -		return err;
> +		return 0;

Ok, you're saying we don't care enough about handling the catastrophic
event where an MDIO access error takes place in mv88e6xxx_g2_scratch_read()
to submit this to "stable".

I guess the impact in such a case is that the error (interpreted as negative
count) makes us go back by -EIO (5) entries or whatever into the "data"
array provided to user space, overwriting some previous stats and making
everything after the failed counter minus the error code be reported in
the wrong place relative to its string. I don't think that the error
codes are high enough to overcome the ~60 port stats and cause memory
accesses behind the "data" array.

Anyway, for the patch content:

Reviewed-by: Vladimir Oltean <vladimir.oltean@....com>