Re: [PATCH net-next v7 12/12] net: airoha: add phylink support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] NULL pointer dereference in `phylink_destroy()` on probe error path.
- [High] Data race on shared GDM4 fragment size registers causes configuration corruption.
--

commit a159dc684c14d93e9de96a620bb343b8bb77e730
Author: Christian Marangi <ansuelsmth@gmail.com>

    net: airoha: add phylink support
    
    Add phylink support for each GDM port. For GDM1 add the internal interface
    mode as the only supported mode. For GDM2/3/4 add the required
    configuration of the PCS to make the external PHY or attached SFP cage
    work.

> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index 5f1a118875fbe..9a42fb991bd72 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
[ ... ]
> @@ -3168,6 +3180,151 @@ bool airoha_is_valid_gdm_dev(struct airoha_eth *eth,
> +static void airoha_mac_link_up(struct phylink_config *config, struct phy_device *phy,
> +			       unsigned int mode, phy_interface_t interface,
> +			       int speed, int duplex, bool tx_pause, bool rx_pause)
> +{
> +	struct airoha_gdm_dev *dev = container_of(config, struct airoha_gdm_dev,
> +						  phylink_config);
> +	struct airoha_gdm_port *port = dev->port;
> +	struct airoha_eth *eth = dev->eth;
> +	u32 frag_size_tx, frag_size_rx;
> +	u32 mask, val;
> +
> +	/* TX/RX frag is configured only for GDM4 */
> +	if (port->id != AIROHA_GDM4_IDX)
> +		return;
[ ... ]
> +	/* Configure TX/RX frag based on speed */
> +	if (dev->nbq == 1) {
> +		mask = GDMA4_SGMII1_TX_FRAG_SIZE_MASK;
> +		val = FIELD_PREP(GDMA4_SGMII1_TX_FRAG_SIZE_MASK,
> +				 frag_size_tx);
> +	}  else {
> +		mask = GDMA4_SGMII0_TX_FRAG_SIZE_MASK;
> +		val = FIELD_PREP(GDMA4_SGMII0_TX_FRAG_SIZE_MASK,
> +				 frag_size_tx);
> +	}
> +	airoha_fe_rmw(eth, REG_GDMA4_TMBI_FRAG, mask, val);

[Severity: High]
Does this lockless read-modify-write on shared GDM4 registers cause data
corruption?

Because the SGMII lanes of the GDM4 port operate as separate phylink instances,
their link states are resolved independently.

If both lanes undergo a link state change simultaneously, phylink's workqueues
can execute airoha_mac_link_up() concurrently on different CPUs.

Without a lock around airoha_fe_rmw(), these concurrent updates to
REG_GDMA4_TMBI_FRAG and REG_GDMA4_RMBI_FRAG could overwrite and corrupt each
other's settings.

> +
> +	if (dev->nbq == 1) {
> +		mask = GDMA4_SGMII1_RX_FRAG_SIZE_MASK;
> +		val = FIELD_PREP(GDMA4_SGMII1_RX_FRAG_SIZE_MASK,
> +				 frag_size_rx);
> +	} else {
> +		mask = GDMA4_SGMII0_RX_FRAG_SIZE_MASK;
> +		val = FIELD_PREP(GDMA4_SGMII0_RX_FRAG_SIZE_MASK,
> +				 frag_size_rx);
> +	}
> +	airoha_fe_rmw(eth, REG_GDMA4_RMBI_FRAG, mask, val);
> +}
[ ... ]
> @@ -3231,7 +3388,7 @@ static int airoha_alloc_gdm_device(struct airoha_eth *eth,
>  	dev->nbq = nbq;
>  	port->devs[index] = dev;
>  
> -	return 0;
> +	return airoha_setup_phylink(netdev);
>  }
[ ... ]
> @@ -3457,6 +3614,7 @@ static int airoha_probe(struct platform_device *pdev)
>  			netdev = netdev_from_priv(dev);
>  			if (netdev->reg_state == NETREG_REGISTERED)
>  				unregister_netdev(netdev);
> +			phylink_destroy(dev->phylink);

[Severity: High]
Could this cause a NULL pointer dereference during a probe failure?

In airoha_alloc_gdm_device(), the port->devs[index] = dev assignment happens
before calling airoha_setup_phylink(). If airoha_setup_phylink() fails,
dev->phylink remains NULL.

The error propagates back up to airoha_probe(), which then unconditionally
calls phylink_destroy() on all populated port->devs slots.

Because phylink_destroy() dereferences its argument without checking if it is
NULL, this will result in a kernel panic.

>  			of_node_put(netdev->dev.of_node);
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615122950.22281-1-ansuelsmth@gmail.com?part=12