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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Kernel heap buffer overflow when writing ethtool statistics because `axienet_1g_get_ethtool_stats` writes past the end of the allocated `data` array.
- [High] Seqcount livelock due to softirq reader interrupting process-context writer without disabling bottom halves.
--

--- Patch [6]: [PATCH net-next 6/7] net: xilinx: axienet: Dispatch statistics through axienet_config ops ---
commit cb6776892b93b092d0fe6ad16946db4662c9e7dd
Author: Suraj Gupta <suraj.gupta2@amd.com>

net: xilinx: axienet: Dispatch statistics through axienet_config ops

This commit adds statistics operation function pointers to
struct axienet_config and routes get_stats64, get_ethtool_stats, and
others through them. It factors the existing 1G counter reads into
axienet_1g_*() helpers.

> 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
> @@ -1878,29 +1878,11 @@ static int axienet_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
>  	return phylink_mii_ioctl(lp->phylink, rq, cmd);
>  }
>  
> -static void
> -axienet_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
> +static void axienet_1g_get_stats64(struct axienet_local *lp,
> +				   struct rtnl_link_stats64 *stats)
>  {
> -	struct axienet_local *lp = netdev_priv(dev);
>  	unsigned int start;
>  
> -	netdev_stats_to_stats64(stats, &dev->stats);
> -
> -	do {
> -		start = u64_stats_fetch_begin(&lp->rx_stat_sync);
> -		stats->rx_packets = u64_stats_read(&lp->rx_packets);
> -		stats->rx_bytes = u64_stats_read(&lp->rx_bytes);
> -	} while (u64_stats_fetch_retry(&lp->rx_stat_sync, start));
> -
> -	do {
> -		start = u64_stats_fetch_begin(&lp->tx_stat_sync);
> -		stats->tx_packets = u64_stats_read(&lp->tx_packets);
> -		stats->tx_bytes = u64_stats_read(&lp->tx_bytes);
> -	} while (u64_stats_fetch_retry(&lp->tx_stat_sync, start));
> -
> -	if (!(lp->features & XAE_FEATURE_STATS))
> -		return;
> -
>  	do {
>  		start = read_seqcount_begin(&lp->hw_stats_seqcount);

[Severity: High]
This is a pre-existing issue, but does reading hw_stats_seqcount here create
a livelock risk with softirq?

If a softirq (e.g., NET_RX_SOFTIRQ) preempts axienet_stats_update() which
writes to hw_stats_seqcount from process context (delayed workqueue) without
disabling bottom halves:

axienet_stats_update()
  write_seqcount_begin(&lp->hw_stats_seqcount);

And calls dev_get_stats() triggering axienet_1g_get_stats64(), the reader
will spin indefinitely in read_seqcount_begin() waiting for the sequence to
become even, but the writer can never resume to release the lock.

[ ... ]

> -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 is a pre-existing issue, but can axienet_1g_get_ethtool_stats() write
past the end of the allocated data array?

axienet_1g_get_sset_count() returns ARRAY_SIZE(axienet_ethtool_stats_strings)
which is 9, so the ethtool core only allocates a 9-element array.

However, axienet_1g_get_ethtool_stats() (in the unchanged lines of this loop)
writes to indices 9 and 10:

  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 cause a heap buffer overflow?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723123838.125145-1-suraj.gupta2@amd.com?part=6