netdev - Re: [net-next,v23,5/6] eea: introduce ethtool support

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] [day] [month] [year] [list]

Message-ID: <20260130040215.708811-1-kuba@kernel.org>
Date: Thu, 29 Jan 2026 20:02:15 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: xuanzhuo@...ux.alibaba.com
Cc: Jakub Kicinski <kuba@...nel.org>,
	edumazet@...gle.com,
	lulie@...ux.alibaba.com,
	andrew@...n.ch,
	dust.li@...ux.alibaba.com,
	andrew+netdev@...n.ch,
	guwen@...ux.alibaba.com,
	lorenzo@...nel.org,
	lukas.bulwahn@...hat.com,
	davem@...emloft.net,
	dong100@...se.com,
	vadim.fedorenko@...ux.dev,
	pabeni@...hat.com,
	netdev@...r.kernel.org,
	hkallweit1@...il.com
Subject: Re: [net-next,v23,5/6] eea: introduce ethtool support

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
eea: introduce ethtool support

This commit adds ethtool support for the Alibaba Elastic Ethernet Adapter
(EEA) driver, including statistics retrieval, ring parameter configuration,
and channel management.

> diff --git a/drivers/net/ethernet/alibaba/eea/eea_ethtool.c b/drivers/net/ethernet/alibaba/eea/eea_ethtool.c
> new file mode 100644
> index 000000000000..b903ae368473
> --- /dev/null
> +++ b/drivers/net/ethernet/alibaba/eea/eea_ethtool.c

[ ... ]

> +static void eea_get_ethtool_stats(struct net_device *netdev,
> +				  struct ethtool_stats *stats, u64 *data)
> +{
> +	struct eea_net *enet = netdev_priv(netdev);
> +	u32 i, idx = 0;
> +
> +	for (i = 0; i < enet->cfg.rx_ring_num; i++) {
> +		struct eea_net_rx *rx = enet->rx[i];

Can this dereference of enet->rx[i] cause a NULL pointer crash when the
device is not running?

Looking at the driver lifecycle, enet->rx is NULL after probe (before the
device is opened) and after close. However, enet->cfg.rx_ring_num is set
to a non-zero value during probe via eea_update_cfg(). This means if a
user runs "ethtool -S ethX" on a device that has not been brought up,
the loop will execute with enet->rx being NULL.

The follow-up commit 3b731fea0a60 ("eea: introduce callback for
ndo_get_stats64") adds an "if (enet->rx)" check before a similar loop in
eea_stats(), suggesting awareness of this issue. Should a similar check
be added here?

> +
> +		eea_stats_fill_for_q(&rx->stats.syncp, EEA_RX_STATS_LEN,
> +				     eea_rx_stats_desc, data, idx);
> +
> +		idx += EEA_RX_STATS_LEN;
> +	}
> +
> +	for (i = 0; i < enet->cfg.tx_ring_num; i++) {
> +		struct eea_net_tx *tx = &enet->tx[i];

The same question applies here for enet->tx.

> +
> +		eea_stats_fill_for_q(&tx->stats.syncp, EEA_TX_STATS_LEN,
> +				     eea_tx_stats_desc, data, idx);
> +
> +		idx += EEA_TX_STATS_LEN;
> +	}
> +}

[ ... ]