Re: [PATCH net-next v9 06/12] net: Document PCS subsystem

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] phylink_create() returns an invalid non-error pointer (or NULL) on invalid config->type.
--

--- Patch [6]: [PATCH net-next v9 06/12] net: Document PCS subsystem ---
commit 867ad4be39467be718a5df59cf2a9739b1c43531
Author: Christian Marangi <ansuelsmth@gmail.com>

net: phylink: introduce internal phylink PCS handling

This commit introduces internal handling of PCS for phylink, moving the
selection logic to phylink using the supported_interface value. It updates
phylink_create() to allocate and populate an array of PCS pointers from the
MAC, creating a linked list of available PCS devices. The changes also include
new validation to ensure a selected PCS supports the requested interfaces.

> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 3464e3ec56997..d96941d4c5235 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
[ ... ]
> @@ -1854,13 +1964,17 @@ struct phylink *phylink_create(struct phylink_config *config,
[ ... ]
> +	/* Fill the PCS list with available PCS from phylink config */
> +	ret = phylink_fill_available_pcs(pl, config);
> +	if (ret < 0)
> +		goto free_pl;
[ ... ]
>  	pl->config = config;
>  	if (config->type == PHYLINK_NETDEV) {
>  		pl->netdev = to_net_dev(config->dev);
> @@ -1882,8 +2012,7 @@ struct phylink *phylink_create(struct phylink_config *config,
>  	} else if (config->type == PHYLINK_DEV) {
>  		pl->dev = config->dev;
>  	} else {
> -		kfree(pl);
> -		return ERR_PTR(-EINVAL);
> +		goto unlink_pcs_list;

[Severity: High]
Can this jump to unlink_pcs_list return an invalid pointer instead of an error?

If config->type is invalid, the code jumps to unlink_pcs_list without setting
'ret = -EINVAL'. Because 'ret' still holds the non-negative result from
phylink_fill_available_pcs(), ERR_PTR(ret) will return a valid-looking pointer
(or NULL) instead of an error pointer.

This would bypass the caller's IS_ERR() check and could lead to a crash when
they attempt to use the returned phylink pointer.

>  	}
>  
[ ... ]
> +unlink_pcs_list:
> +	list_for_each_entry(pcs, &pl->pcs_list, list)
> +		pcs->phylink = NULL;
> +free_pl:
> +	kfree(pl);
> +	return ERR_PTR(ret);
>  }
>  EXPORT_SYMBOL_GPL(phylink_create);

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