Re: [RFC PATCH 03/12] drm/fabric: implement query netlink operations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Ambiguity check bypass in endpoint resolution when busname is provided.
--

--- Patch [3]: [RFC PATCH 03/12] drm/fabric: implement query netlink operations ---
commit ec7b295b3219dced2d6c1d65d7829fa5aa0283c3
Author: Konstantin Sinyuk <ksinyuk@kernel.org>
Subject: drm/fabric: implement query netlink operations

This commit registers the drm-fabric Generic Netlink family and connects
read-only query commands to the core object model. It supports single-object
lookup and multipart dumps for fabrics, endpoints, and ports.

> diff --git a/drivers/gpu/drm/fabric/drm_fabric_netlink.c b/drivers/gpu/drm/fabric/drm_fabric_netlink.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/gpu/drm/fabric/drm_fabric_netlink.c

[ ... ]

> +/* Identified by core-assigned id, or by bus + dev name. */
> +static struct drm_fabric_endpoint *
> +drm_fabric_resolve_endpoint(struct genl_info *info)
> +{
> +	struct nlattr **attrs = info->attrs;
> +	const char *devname = NULL;
> +	const char *busname = NULL;
> +	struct drm_fabric_endpoint *ep;
> +
> +	lockdep_assert_held(&drm_fabric_lock);
> +
> +	if (attrs[DRM_FABRIC_A_DEV_NAME])
> +		devname = nla_data(attrs[DRM_FABRIC_A_DEV_NAME]);
> +	if (attrs[DRM_FABRIC_A_BUS_NAME])
> +		busname = nla_data(attrs[DRM_FABRIC_A_BUS_NAME]);

[ ... ]

> +	if (!devname)
> +		return ERR_PTR(-EINVAL);
> +
> +	ep = drm_fabric_endpoint_find_by_dev_name(devname, busname);

[Severity: High]
Does this lookup bypass the ambiguity check when both devname and busname are
provided?

Looking at drm_fabric_endpoint_find_by_dev_name() in drm_fabric.c:

drm_fabric_endpoint_find_by_dev_name() {
    ...
    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;
        }
    ...
}

When busname is matched, the loop immediately returns the first matched
endpoint instead of assigning it to the match variable and continuing the
loop. If multiple endpoints are registered by the same physical device
(sharing the exact same devname and busname), could this cause userspace
queries to silently succeed and operate on the wrong endpoint instead of
returning an ambiguity error?

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