Re: [PATCH net-next 2/3] of: property: link PHY package suppliers to member PHYs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] A malformed devicetree (e.g., placing a `remote-endpoint` property on a node without parents) causes `of_graph_get_port_parent()` to return NULL. This NULL pointer is passed as `con_np` to `of_link_to_phandle()`, which forwards it unmodified to `fwnode_link_add()` and eventually `__fwnode_link_add()`, leading to a kernel panic when dereferenced.
--

--- Patch [2]: [PATCH net-next 2/3] of: property: link PHY package suppliers to member PHYs ---
commit 4b95d666e97ce94700e97701480015a01fd0a0f5
Author: James Hilliard <james.hilliard1@gmail.com>

of: property: link PHY package suppliers to member PHYs

This commit links Ethernet PHY package suppliers directly to their
member PHYs. This prevents proxy links on the MDIO bus from persisting
indefinitely and blocking suppliers from receiving sync_state callbacks.

> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index 72cf12907de03..f29ba818ecbc1 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
[ ... ]
> @@ -1272,6 +1272,20 @@ static void of_link_to_phandle(struct device_node *con_np,
>  		tmp_np = of_get_next_parent(tmp_np);
>  	}
>  
> +	/*
> +	 * An Ethernet PHY package node describes resources shared by its member
> +	 * PHYs, but is not populated as a struct device. Link every enabled
> +	 * member PHY to those suppliers so fw_devlink can use the real consumer
> +	 * devices instead of leaving a proxy link on the MDIO bus indefinitely.
> +	 */
> +	if (of_node_name_eq(con_np, "ethernet-phy-package")) {
> +		for_each_available_child_of_node_scoped(con_np, child)
> +			fwnode_link_add(of_fwnode_handle(child),
> +					of_fwnode_handle(sup_np), flags);
> +
> +		return;
> +	}
> +
>  	fwnode_link_add(of_fwnode_handle(con_np), of_fwnode_handle(sup_np), flags);

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for con_np to
be NULL here?

If a malformed devicetree contains a remote-endpoint property on a node
without parents, of_graph_get_port_parent() can return NULL. When fw_devlink
parses this via of_link_property(), it passes this NULL pointer as con_np to
of_link_to_phandle().

The NULL con_np is then forwarded unmodified to fwnode_link_add(), which
eventually reaches __fwnode_link_add() and dereferences the NULL consumer
pointer without validation:

drivers/base/core.c:__fwnode_link_add() {
    ...
    list_add(&link->c_hook, &con->suppliers);
    ...
}

Should there be a check to ensure con_np is valid before attempting to
create the link?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814-submit-phy-package-fwdevlink-v1-v1-0-2319844f057a@gmail.com?part=2