Re: [PATCH v4 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:
- [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 <riana.tauro@intel.com>

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