Re: [PATCH net-next v6 01/12] net: phylink: keep and use MAC supported_interfaces in phylink struct

[Christian Marangi <ansuelsmth@gmail.com>]

On Wed, Jun 10, 2026 at 04:16:20PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Late modification to `config->supported_interfaces` by drivers is silently ignored due to by-value copying in `phylink_create()`, breaking support for devices like MTK GMAC2.
> --
> 
> commit eaea53b185dbdc4c75406c542c97543c4c451c9d
> Author: Christian Marangi <ansuelsmth@gmail.com>
> 
> net: phylink: keep and use MAC supported_interfaces in phylink struct
> 
> This patch copies supported_interfaces from phylink_config into the phylink
> structure to prepare for internal PCS handling. The internal copy is then
> used in place of the config structure's bitmap.
> 
> > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> > index 087ac63f9193d..4d59c0dd78dbf 100644
> > --- a/drivers/net/phy/phylink.c
> > +++ b/drivers/net/phy/phylink.c
> [ ... ]
> > @@ -1868,6 +1873,9 @@ struct phylink *phylink_create(struct phylink_config *config,
> >  	mutex_init(&pl->state_mutex);
> >  	INIT_WORK(&pl->resolve, phylink_resolve);
> >  
> > +	phy_interface_copy(pl->supported_interfaces,
> > +			   config->supported_interfaces);
> > +
> 
> [Severity: High]
> Will this by-value copy break drivers that modify supported_interfaces after
> calling phylink_create()?
> 
> For example, looking at the mtk_eth_soc driver in mtk_add_mac(), the
> supported_interfaces bitmap is modified after phylink_create() returns:
> 
> drivers/net/ethernet/mediatek/mtk_eth_soc.c:mtk_add_mac() {
>         ...
>         mac->phylink = phylink_create(&mac->phylink_config, ...);
>         ...
>         if (MTK_HAS_CAPS(mac->hw->soc->caps, MTK_2P5GPHY) &&
>             id == MTK_GMAC2_ID)
>                 __set_bit(PHY_INTERFACE_MODE_INTERNAL,
>                           mac->phylink_config.supported_interfaces);
>         ...
> }
> 
> Since phylink_create() has already taken a snapshot of the interfaces,
> this late addition of PHY_INTERFACE_MODE_INTERNAL will be dropped. 
> Could this cause phylink_validate() to incorrectly reject the internal PHY
> capability for the MTK GMAC2?
> 
> >  	pl->config = config;
> >  	if (config->type == PHYLINK_NETDEV) {
> >  		pl->netdev = to_net_dev(config->dev);
>

I was with the idea that value of phylink_config should be used only for
phylink_create and then in theory they should not be changed.

Isn't this a bug in MTK code to modify the phylink_config AFTER
phylink_create?

It was never a problem since it's it's INTERNAL interface mode and a PCS is
not needed for it but still I feel this should be fixed and this change
doesn't introduce some kind of regression.
 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260609151212.29469-1-ansuelsmth@gmail.com?part=1

-- 
	Ansuel