Re: [PATCH v14 02/12] net: phylink: introduce internal phylink PCS handling
From: sashiko-bot@kernel.org
Date: 2026-08-14 08:36:39
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] phylink_create() unconditionally expands the MAC's supported interfaces with all PCS-supported interfaces, enabling unsupported MAC hardware modes. - [Medium] phylink_validate_mac_and_pcs() fails to reject interfaces that require a dedicated PCS when no matching PCS is found. --
--- Patch [2]: [PATCH v14 02/12] net: phylink: introduce internal phylink PCS handling ---commit 6086d011ffb72e143c6102bee959c780b432bd7a Author: Christian Marangi [off-list ref] net: phylink: introduce internal phylink PCS handling This commit introduces internal handling of PCS for phylink by allowing MACs to provide a list of available PCS via phylink_config. It parses this array and links the PCS list to the phylink instance. phylink validation and configuration functions are updated to use this PCS list instead of relying exclusively on .mac_select_pcs.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index e1e9f07f51bc6..6204e3bba16d7 100644 --- a/drivers/net/phy/phylink.c +++ b/drivers/net/phy/phylink.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -530,9 +550,24 @@ static int phylink_validate_mac_and_pcs(struct phylink *pl, pcs = pl->mac_ops->mac_select_pcs(pl->config, state->interface); if (IS_ERR(pcs)) return PTR_ERR(pcs); + + pcs_found = !!pcs; + /* + * Find a PCS in available PCS list for the requested interface. + * + * Skip searching if the MAC doesn't require a dedicated PCS for + * the requested interface. + */ + } else if (test_bit(state->interface, pl->config->pcs_interfaces)) { + list_for_each_entry(pcs, &pl->pcs_list, list) { + if (!phylink_validate_pcs_interface(pcs, state->interface)) { + pcs_found = true; + break; + } + } } - if (pcs) { + if (pcs_found) {
[Severity: Medium] Should there be an explicit error return here if a dedicated PCS is required but none is found? If pcs_found remains false after the loop in phylink_validate_mac_and_pcs(), the code skips the validation block without returning -EINVAL. This allows the unsupported mode to pass MAC validation, which later fails during phylink_major_config(). [ ... ]
quoted hunk ↗ jump to hunk
@@ -1908,16 +2028,31 @@ struct phylink *phylink_create(struct phylink_config *config, __set_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state); timer_setup(&pl->link_poll, phylink_fixed_poll, 0); + /* Fill the PCS list with available PCS from phylink config */ + ret = phylink_fill_available_pcs(pl, config); + if (ret < 0) + goto free_pl; + + /* Link available PCS to phylink */ + list_for_each_entry(pcs, &pl->pcs_list, list) + pcs->phylink = pl; + phy_interface_copy(pl->supported_interfaces, pl->config->supported_interfaces); + /* Update supported interfaces */ + list_for_each_entry(pcs, &pl->pcs_list, list) + phy_interface_or(pl->supported_interfaces, + pl->supported_interfaces, + pcs->supported_interfaces); +
[Severity: High]
Does this bitwise OR operation inadvertently expand the system's supported
interfaces beyond the MAC's physical capabilities?
By injecting all PCS-supported interfaces into pl->supported_interfaces,
generic PCS modules might expose high-speed modes that the attached MAC
does not actually support. Could this lead to the MAC driver receiving
configuration requests for unhandled interface types?
A similar capability expansion also appears to happen dynamically in
pcs_provider_notify() when a PCS provider is added:
/* Refresh supported interfaces */
phy_interface_copy(pl->supported_interfaces,
pl->config->supported_interfaces);
list_for_each_entry(pcs, &pl->pcs_list, list)
phy_interface_or(pl->supported_interfaces,
pl->supported_interfaces,
pcs->supported_interfaces);
Is this intended?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260813083536.970196-1-ansuelsmth@gmail.com?part=2