Re: [PATCH net-next v2 6/7] net: xilinx: axienet: Dispatch statistics through axienet_config ops

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] `axienet_1g_get_ethtool_stats` writes hardware statistics out-of-bounds of the `data` array allocated by the ethtool core.
--

--- Patch [6]: [PATCH net-next v2 6/7] net: xilinx: axienet: Dispatch statistics through axienet_config ops ---
commit dd40493ec0a25e2800677d93923a2097ca0c48e3
Author: Suraj Gupta <[email protected]>

net: xilinx: axienet: Dispatch statistics through axienet_config ops

The commit refactors hardware statistic callbacks to use function pointers
in struct axienet_config. This avoids conditional branching for different
MAC types by populating operations like get_ethtool_stats via an
axienet_1g_config structure.

> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> index 1892d87ebaaea..6e4b3a073e733 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c

[ ... ]

> @@ -2360,11 +2369,8 @@ static int axienet_ethtools_nway_reset(struct net_device *dev)
>  	return phylink_ethtool_nway_reset(lp->phylink);
>  }
>  
> -static void axienet_ethtools_get_ethtool_stats(struct net_device *dev,
> -					       struct ethtool_stats *stats,
> -					       u64 *data)
> +static void axienet_1g_get_ethtool_stats(struct axienet_local *lp, u64 *data)
>  {
> -	struct axienet_local *lp = netdev_priv(dev);
>  	unsigned int start;
>  
>  	do {

[Severity: High]
This isn't a bug introduced by this patch, but does this function write
hardware statistics out-of-bounds of the data array allocated by the ethtool
core?

The ethtool core allocates memory based on axienet_1g_get_sset_count, which
returns 9 for the size of axienet_ethtool_stats_strings. However, this
function writes to data[8], data[9], and data[10], skipping data[4] and
data[5]:

axienet_1g_get_ethtool_stats() {
    ...
    data[7] = axienet_stat(lp, STAT_RX_PFC_FRAMES);
    data[8] = axienet_stat(lp, STAT_USER_DEFINED0);
    data[9] = axienet_stat(lp, STAT_USER_DEFINED1);
    data[10] = axienet_stat(lp, STAT_USER_DEFINED2);
    ...
}

Could this lead to a heap buffer overflow and memory corruption when a user
requests statistics with ethtool -S?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6