Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] Multicast hardware error events are restricted to `init_net` despite the netlink family supporting multiple network namespaces (`.netnsok = true`). - [Medium] Multicast group 'error-report' does not require net-admin privileges, leaking restricted error data to unprivileged users. --
commit 6e5a626b76568687a02f046e7898ab55e1095ab9 Author: Riana Tauro <[email protected]> drm/drm_ras: Add drm_ras netlink error event Define a new netlink event error-event and a new multicast group error-report in drm_ras. Each event contains device name, node and error information to identify the error triggering the event. > diff --git a/drivers/gpu/drm/drm_ras.c b/drivers/gpu/drm/drm_ras.c > index d6eab29a1394c..77f912a4d101f 100644 > --- a/drivers/gpu/drm/drm_ras.c > +++ b/drivers/gpu/drm/drm_ras.c [ ... ] > @@ -222,6 +255,60 @@ static int doit_reply_value(struct genl_info *info, u32 > node_id, [ ... ] > +int drm_ras_nl_error_event(struct drm_ras_node *node, u32 error_id, const > char *error_name, > + u32 value, gfp_t flags) > +{ > + struct genl_info info; > + struct sk_buff *msg; > + struct nlattr *hdr; > + int ret; > + > + if (!error_name) > + return -EINVAL; > + > + if (!genl_has_listeners(&drm_ras_nl_family, &init_net, > DRM_RAS_NLGRP_ERROR_REPORT)) [Severity: Medium] Since the generic netlink family explicitly supports multiple network namespaces by setting .netnsok = true, will hardcoding init_net here prevent listeners in non-init namespaces from receiving error events? > + return 0; > + > + genl_info_init_ntf(&info, &drm_ras_nl_family, DRM_RAS_CMD_ERROR_EVENT); > + > + msg = genlmsg_new(NLMSG_GOODSIZE, flags); [ ... ] > + genlmsg_end(msg, hdr); > + genlmsg_multicast(&drm_ras_nl_family, msg, 0, > DRM_RAS_NLGRP_ERROR_REPORT, flags); [Severity: Medium] Similar to the listener check above, does using genlmsg_multicast() route the message exclusively to the init_net namespace? Any containerized user trying to listen to these events may silently fail to receive them if the events are artificially restricted to init_net. > + return 0; > + > +cancel_msg: [ ... ] > diff --git a/drivers/gpu/drm/drm_ras_nl.c b/drivers/gpu/drm/drm_ras_nl.c > index dea1c1b2494e8..9d3123cc9f9ca 100644 > --- a/drivers/gpu/drm/drm_ras_nl.c > +++ b/drivers/gpu/drm/drm_ras_nl.c > @@ -58,6 +58,10 @@ static const struct genl_split_ops drm_ras_nl_ops[] = { > }, > }; > > +static const struct genl_multicast_group drm_ras_nl_mcgrps[] = { > + [DRM_RAS_NLGRP_ERROR_REPORT] = { "error-report", }, [Severity: Medium] Should this multicast group require net-admin privileges? The get and list commands for drm_ras enforce GENL_ADMIN_PERM, but this multicast group definition omits capability flags like GENL_UNS_ADMIN_PERM. Will this allow unprivileged users to subscribe to the group and receive hardware error data that they are otherwise restricted from querying? > +}; > + > struct genl_family drm_ras_nl_family __ro_after_init = { -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
