Re: [RFC PATCH 01/12] drm/fabric: add core object model and provider API

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm_fabric_endpoint_find_by_dev_name() skips ambiguity checking when busname is provided.
--

--- Patch [1]: [RFC PATCH 01/12] drm/fabric: add core object model and provider API ---
commit a52787ad8ee51af45c9fca91640883affafd66ca
Author: Ilia Levi <ilia.levi@intel.com>

drm/fabric: add core object model and provider API

This commit introduces a common DRM object model for interconnect fabrics,
endpoints, ports, and peers. It provides a registry and a provider API to
manage object identities, relationships, and lifetimes under a central lock.

> diff --git a/drivers/gpu/drm/fabric/drm_fabric.c b/drivers/gpu/drm/fabric/drm_fabric.c
> new file mode 100644
> index 0000000000000..8769d7bdcde1f
> --- /dev/null
> +++ b/drivers/gpu/drm/fabric/drm_fabric.c

[ ... ]

> +/*
> + * Returns NULL if no endpoint matches, or ERR_PTR(-EINVAL) if @devname is
> + * ambiguous across buses and @busname does not disambiguate it.
> + */
> +struct drm_fabric_endpoint *
> +drm_fabric_endpoint_find_by_dev_name(const char *devname, const char *busname)
> +{
> +	struct drm_fabric_endpoint *match = NULL;
> +	struct drm_fabric_endpoint *ep;
> +	unsigned long idx;
> +
> +	lockdep_assert_held(&drm_fabric_lock);
> +	xa_for_each(&drm_fabric_ep_xa, idx, ep) {
> +		if (strcmp(dev_name(ep->parent), devname))
> +			continue;
> +
> +		if (busname) {
> +			if (strcmp(dev_bus_name(ep->parent), busname))
> +				continue;
> +			return ep;

[Severity: Medium]
Does this early return bypass the ambiguity check if a single parent device
registers multiple endpoints? 

When a busname is provided, this path returns the very first matching endpoint
it finds. If a device matching both the devname and busname has registered
multiple endpoints, this silently resolves to the first one instead of
rejecting the ambiguous lookup with ERR_PTR(-EINVAL). 

The code block just below this handles the ambiguity correctly when busname
is not provided. Should the busname path also continue checking the rest of
the xarray to ensure only one match exists?

> +		}
> +
> +		if (match)
> +			return ERR_PTR(-EINVAL);
> +
> +		match = ep;
> +	}
> +
> +	return match;
> +}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1787552412.git.ksinyuk@kernel.org?part=1