Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] The 'error-report' multicast group lacks the admin-perm flag, allowing unprivileged users to listen to sensitive hardware events. - [Medium] The exported API drm_ras_nl_error_event() unconditionally uses GFP_KERNEL for netlink message allocation, making it unsafe to call from atomic context. --
--- Patch [1]: [PATCH v5 1/3] drm/drm_ras: Add drm_ras netlink error event --- commit 50118022fc58ae5076236cbd29a79d06fb22c201 Author: Riana Tauro <[email protected]> drm/drm_ras: Add drm_ras netlink error event This commit defines a new netlink event error-event and a new multicast group error-report in drm_ras to report hardware errors to userspace. It also adds the exported function drm_ras_nl_error_event() to trigger these events from drivers. > diff --git a/Documentation/netlink/specs/drm_ras.yaml > b/Documentation/netlink/specs/drm_ras.yaml > index e113056f8c016..8aed3d4515e5a 100644 > --- a/Documentation/netlink/specs/drm_ras.yaml > +++ b/Documentation/netlink/specs/drm_ras.yaml [ ... ] > +mcast-groups: > + list: > + - > + name: error-report [Severity: High] Should this multicast group include flags: [admin-perm]? The netlink commands for this family explicitly require GENL_ADMIN_PERM, but this multicast group omits the flag. Without it, unprivileged users can open a netlink socket and subscribe to the error-report group, potentially observing sensitive hardware reliability events that could be used in side-channel attacks. > diff --git a/drivers/gpu/drm/drm_ras.c b/drivers/gpu/drm/drm_ras.c > index d6eab29a1394c..0b77e4358ea8f 100644 > --- a/drivers/gpu/drm/drm_ras.c > +++ b/drivers/gpu/drm/drm_ras.c [ ... ] > +int drm_ras_nl_error_event(struct drm_ras_node *node, u32 error_id, const > char *error_name, > + u32 value) > +{ > + struct genl_info info; > + struct sk_buff *msg; > + struct nlattr *hdr; > + int ret; > + > + if (!error_name) > + return -EINVAL; > + > + genl_info_init_ntf(&info, &drm_ras_nl_family, DRM_RAS_CMD_ERROR_EVENT); > + > + msg = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL); [Severity: Medium] Will this function be called from atomic contexts? Hardware RAS errors are frequently detected and reported from interrupt handlers or while holding spinlocks. If a driver calls this API from an atomic context, the unconditional GFP_KERNEL allocation will cause a sleep-in-atomic panic. Would it be safer to accept a gfp_t mask as a parameter, or default to GFP_ATOMIC, to support reporting from these contexts? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
