Re: [PATCH v5 1/3] drm/drm_ras: Add drm_ras netlink error event

[sashiko-bot@kernel.org]

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 <riana.tauro@intel.com>
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/20260720082208.2648279-5-riana.tauro@intel.com?part=1