Re: [PATCH net 1/2] net: macb: zero the link settings taprio reads back

[Théo Lebrun]

Hello Aleksei,

On Wed Sep 2, 2026 at 10:05 AM CEST, Aleksei Sviridkin wrote:
> phylink_ethtool_ksettings_get() fills only what the current link mode
> provides. With no PHY attached it writes port and supported; on a fixed
> or in-band link it also writes speed and duplex, but only if
> base.rate_matching already reads RATE_MATCH_NONE, a field it never
> writes itself and so takes from the caller. The ethtool core zeroes the
> structure on every path into the op, so its callers never notice; the
> taprio path passes a stack variable and reads a speed nobody wrote.
>
> Fixes: 89934dbf169e ("net: macb: Add TAPRIO traffic scheduling support")
> Assisted-by: LLM
> Signed-off-by: Aleksei Sviridkin <f@xxxxxx>

Agreed on the fix.

Could we improve the commit message? Point how straightforwardly the
case where uninitialised memory might be used, which is the main thing
we care about. See how the following is easier to parse out (for humans):

   phylink_ethtool_ksettings_get(bp->phylink, &kset) gets called with
   uninitialised kset but kset is not only an out-param. In some cases
   kset->base.rate_matching might be read.

   ... explain the cases in question ...

> ---
>  drivers/net/ethernet/cadence/macb_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 76ee4f506033..a43855db1e45 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -4297,7 +4297,7 @@ static int macb_taprio_setup_replace(struct net_device *netdev,
>  	struct macb_queue_enst_config *enst_queue;
>  	struct tc_taprio_sched_entry *entry;
>  	struct macb *bp = netdev_priv(netdev);
> -	struct ethtool_link_ksettings kset;
> +	struct ethtool_link_ksettings kset = {};
>  	struct macb_queue *queue;
>  	u32 queue_mask;
>  	u8 queue_id;

Let's try to respect reverse xmas tree as we grow a variable.
I know it wasn't perfect before.

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com