linux-kernel - Re: [PATCH net-next v3] dsa: lan9303: Add 3 ethtool stats

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: <20221128210515.kqvdshlh3phmdpxx@skbuf>
Date:   Mon, 28 Nov 2022 23:05:15 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     Jerry Ray <jerry.ray@...rochip.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v3] dsa: lan9303: Add 3 ethtool stats

On Mon, Nov 28, 2022 at 02:55:21PM -0600, Jerry Ray wrote:
>  static void lan9303_get_ethtool_stats(struct dsa_switch *ds, int port,
>  				      uint64_t *data)
>  {
>  	struct lan9303 *chip = ds->priv;
> -	unsigned int u;
> +	unsigned int i, u;
>  
>  	for (u = 0; u < ARRAY_SIZE(lan9303_mib); u++) {
>  		u32 reg;
>  		int ret;
>  
> -		ret = lan9303_read_switch_port(
> -			chip, port, lan9303_mib[u].offset, &reg);
> -
> +		/* Read Port-based MIB stats. */
> +		ret = lan9303_read_switch_port(chip, port,
> +					       lan9303_mib[u].offset,
> +					       &reg);

Speaking of unrelated changes...

>  		if (ret)
>  			dev_warn(chip->dev, "Reading status port %d reg %u failed\n",
>  				 port, lan9303_mib[u].offset);

...If lan9303_read_switch_port() fails, should we copy kernel stack
uninitialized memory (reg) to user space?

>  		data[u] = reg;
>  	}
> +	for (i = 0; i < ARRAY_SIZE(lan9303_switch_mib); i++) {
> +		u32 reg;
> +		int ret;
> +
> +		/* Read Switch stats indexed by port. */
> +		ret = lan9303_read_switch_reg(chip,
> +					      (lan9303_switch_mib[i].offset +
> +					       port), &reg);
> +		if (ret)
> +			dev_warn(chip->dev, "Reading status port %d reg %u failed\n",
> +				 port, lan9303_switch_mib[i].offset);

Because the same, existing pattern is also used for new code here.

> +		data[i + u] = reg;
> +	}
>  }
>  
>  static int lan9303_get_sset_count(struct dsa_switch *ds, int port, int sset)
> @@ -1017,7 +1061,7 @@ static int lan9303_get_sset_count(struct dsa_switch *ds, int port, int sset)
>  	if (sset != ETH_SS_STATS)
>  		return 0;
>  
> -	return ARRAY_SIZE(lan9303_mib);
> +	return ARRAY_SIZE(lan9303_mib) + ARRAY_SIZE(lan9303_switch_mib);
>  }
>  
>  static int lan9303_phy_read(struct dsa_switch *ds, int phy, int regnum)
> -- 
> 2.17.1
>