Re: [PATCH net-next v14 03/13] net: mdio: realtek-rtl9300: deny C45 over C22 access

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Using `dev_WARN_ONCE` for an unsupported hardware operation triggers kernel stack traces and potential panics, creating a local denial of service vector.
- [Medium] Returning the internal `-ENOTSUPP` error code instead of the standard `-EOPNOTSUPP` breaks error handling and leaks internal kernel error codes to user-space.
--

--- Patch [3]: [PATCH net-next v14 03/13] net: mdio: realtek-rtl9300: deny C45 over C22 access ---
commit d6cd33a68aefd1fa8ff00c7952b513cfe5a1bda8
Author: Markus Stockhausen <[email protected]>

net: mdio: realtek-rtl9300: deny C45 over C22 access

Hardware polling breaks C45 over C22 PHY access. This patch adds checks
to the read and write paths to deny these accesses, return an error, and
give a one-time warning message.

> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c

[ ... ]

> @@ -453,6 +454,10 @@ static int otto_emdio_read_c22(struct mii_bus *bus, int phy_id, int regnum)
>  	int ret, port;
>  	u32 value;
>  
> +	if (dev_WARN_ONCE(&bus->dev, regnum == MII_MMD_CTRL || regnum == MII_MMD_DATA,
> +			  "C45 over C22 read access broken due to polling\n"))

[Severity: High]
Is it intentional to use dev_WARN_ONCE() here instead of dev_warn_once()?

The generic PHY framework routinely uses C45-over-C22 to probe capabilities 
like Energy Efficient Ethernet (EEE) via mmd_phy_indirect(), which triggers
access to MII_MMD_CTRL.

Since standard ethtool commands like '--show-eee' can trigger this path,
using a WARN macro allows local users to trigger a kernel stack dump. If
panic_on_warn is enabled, this could be used to cause a system crash.

> +		return -ENOTSUPP;

[Severity: Medium]
Should this return -EOPNOTSUPP instead?

-ENOTSUPP is an internal NFS error code (524) that isn't recognized by
userspace, which can lead to unrecognized error messages (like
"Unknown error 524") when leaked through netlink or ioctls.

Also, the PHY subsystem typically relies on the standard -EOPNOTSUPP code
to gracefully handle unsupported operations and fall back to alternatives.
Does the same suggestion apply to otto_emdio_write_c22() as well?

> +
>  	port = otto_emdio_phy_to_port(bus, phy_id);
>  	if (port < 0)
>  		return port;

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