Re: [RFC PATCH net-next 9/9] net: phylink: report no link modes while a late PHY is missing

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Uninitialized stack memory read in `macb_taprio_setup_replace`.
--

commit 0643f275f79b61c4578a88309083d8ddef0461ba
Author: Aleksei Sviridkin <f@lex.la>

net: phylink: report no link modes while a late PHY is missing

This commit modifies phylink to report no link modes and reject settings 
while a slow-to-probe PHY is still pending. This prevents the port from 
using the MAC's capabilities for a link that cannot come up yet.

> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 62e6059a18da3..857d38ed45a6a 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
[ ... ]
> @@ -3106,6 +3112,17 @@ int phylink_ethtool_ksettings_get(struct phylink *pl,
>  	else
>  		kset->base.port = pl->link_port;
>  
> +	/* Until the PHY arrives the port can do nothing, so report no link
> +	 * modes at all rather than the MAC's own capabilities, which is what
> +	 * an empty SFP cage reports.
> +	 */
> +	if (phylink_slow_phy_pending(pl)) {
> +		linkmode_zero(kset->link_modes.supported);
> +		kset->base.speed = SPEED_UNKNOWN;
> +		kset->base.duplex = DUPLEX_UNKNOWN;
> +		return 0;
> +	}
> +

[Severity: High]
This is a pre-existing issue, but is it possible that kset contains
uninitialized stack memory when passed to phylink_ethtool_ksettings_get()?

For example, macb_taprio_setup_replace() in
drivers/net/ethernet/cadence/macb_main.c declares struct
ethtool_link_ksettings kset without initializing it:

drivers/net/ethernet/cadence/macb_main.c:macb_taprio_setup_replace() {
    struct ethtool_link_ksettings kset;
    ...
    err = phylink_ethtool_ksettings_get(bp->phylink, &kset);
    ...
}

If a user space process configures a TAPRIO qdisc on a MACB interface that
operates without an attached PHY, this function will call
phylink_get_ksettings(), which then reads the uninitialized stack memory in
phylink_merge_link_mode():

drivers/net/phy/phylink.c:phylink_merge_link_mode() {
    linkmode_and(dst, dst, mask);
    linkmode_or(dst, dst, b);
}

Additionally, kset->base.rate_matching is read to conditionally set
kset->base.speed:

drivers/net/phy/phylink.c:phylink_get_ksettings() {
    if (kset->base.rate_matching == RATE_MATCH_NONE) {
        kset->base.speed = state->speed;
}

This uninitialized speed is later returned and used for hardware interval
calculations. Since this can be reached by untrusted input via the tc
netlink interface if user namespaces are enabled, could this result in
incorrect hardware QoS limits and trigger KMSAN use-of-uninitialized-value
warnings?

>  	linkmode_copy(kset->link_modes.supported, pl->supported);
>  
>  	switch (pl->act_link_an_mode) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260829052546.1152446-1-f@lex.la?part=9