Re: [PATCH net-next v9 09/11] onsemi: s2500: Added selftest support to onsemi's S2500 driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ethtool: user-triggerable WARN_ON panic via ethtool_self_test() when selftests disabled
--

commit 78898d3de4f452382c12b216accf732a736db328
Author: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>

onsemi: s2500: Added selftest support to onsemi's S2500 driver
    
Adds selftest support for onsemi S2500 MAC-PHY. Added as a separate
patch for the ease of review.

> diff --git a/drivers/net/ethernet/onsemi/s2500/s2500_ethtool.c b/drivers/net/ethernet/onsemi/s2500/s2500_ethtool.c
> index 062cab9460dcf..12e59e8d44a62 100644
> --- a/drivers/net/ethernet/onsemi/s2500/s2500_ethtool.c
> +++ b/drivers/net/ethernet/onsemi/s2500/s2500_ethtool.c
[ ... ]
> @@ -201,6 +202,8 @@ static int s2500_get_sset_count(struct net_device *ndev, int sset)
>  	switch (sset) {
>  	case ETH_SS_STATS:
>  		return S2500_MAC_STATS_LEN;
> +	case ETH_SS_TEST:
> +		return net_selftest_get_count();

[Severity: Medium]
Will this return 0 when the selftests configuration is disabled?

When selftests are disabled, net_selftest_get_count() returns 0. However,
the ops->self_test pointer remains non-NULL because net_selftest is
instantiated as an empty static inline function.

When an administrator issues ethtool -t to trigger self-tests,
ethtool_self_test() notices ops->self_test is non-NULL. It queries
ops->get_sset_count(), receives a length of exactly 0, and proceeds to hit a
WARN_ON:

net/ethtool/ioctl.c:ethtool_self_test() {
    ...
	test_len = ops->get_sset_count(dev, ETH_SS_TEST);
	if (test_len < 0)
		return test_len;
	WARN_ON(test_len == 0);
    ...
}

If the system is booted with panic_on_warn, this could allow a local user
with net admin capability to intentionally crash the kernel.

Should the driver check the return value of net_selftest_get_count() and
explicitly return -EOPNOTSUPP if it evaluates to 0?

>  	default:
>  		return -EOPNOTSUPP;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-s2500-mac-phy-support-v9-0-dcefe1d0bf0d@onsemi.com?part=9