Re: [RFC PATCH 03/12] drm/fabric: implement query netlink operations
| Newsgroups | org.freedesktop.lists.dri-devel |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]> 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/[email protected]?part=3