Re: [PATCH v5 4/5] drm/xe/drm_ras: Wire up error threshold callbacks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] The driver violates the DRM RAS framework API contract by failing to perform bounds checking on the user-supplied threshold value.

Pre-existing issues:
- [High] Lack of reference counting or RCU read locks in the generic drm_ras core causes a TOCTOU race leading to Use-After-Free of drm_ras_node and xe_device during concurrent device unbind.
--

--- Patch [4]: [PATCH v5 4/5] drm/xe/drm_ras: Wire up error threshold callbacks ---
commit a57e8de665a45c38e3544fb21f325ce074bc995d
Author: Raag Jadav <raag.jadav@intel.com>

drm/xe/drm_ras: Wire up error threshold callbacks

This commit wires up the get/set error threshold support in the xe driver to
the generic drm_ras core. This allows userspace to query and configure
correctable error thresholds via Netlink.

> diff --git a/drivers/gpu/drm/xe/xe_drm_ras.c b/drivers/gpu/drm/xe/xe_drm_ras.c
> --- a/drivers/gpu/drm/xe/xe_drm_ras.c
> +++ b/drivers/gpu/drm/xe/xe_drm_ras.c
> @@ -86,6 +86,38 @@ static int clear_correctable_error_counter(struct drm_ras_node *node, u32 error_
>  	return clear_error_counter(xe, DRM_XE_RAS_ERR_SEV_CORRECTABLE, error_id);
>  }
>  
> +static int query_correctable_error_threshold(struct drm_ras_node *ep, u32 error_id,
> +					     const char **name, u32 *threshold)
> +{
> +	struct xe_device *xe = ep->priv;

[Severity: High]
This is a pre-existing issue, but can this dereference of ep and ep->priv
lead to a use-after-free during concurrent device unbind?

If a Netlink request calls get_node_error_threshold() in the generic drm_ras
core, it retrieves the node via xa_load() but doesn't hold RCU or take a
reference count. Concurrently, if the device is unbound, the drmm action
cleanup_node() could free the drm_ras_node and the xe_device.

When the Netlink thread resumes and calls this callback, could the ep pointer
and the xe_device it points to already be freed?

> +	struct xe_drm_ras *ras = &xe->ras;
> +	struct xe_drm_ras_counter *info = ras->info[DRM_XE_RAS_ERR_SEV_CORRECTABLE];

[ ... ]

> +static int set_correctable_error_threshold(struct drm_ras_node *ep, u32 error_id, u32 threshold)
> +{
> +	struct xe_device *xe = ep->priv;
> +	struct xe_drm_ras *ras = &xe->ras;
> +	struct xe_drm_ras_counter *info = ras->info[DRM_XE_RAS_ERR_SEV_CORRECTABLE];
> +
> +	if (!info || !info[error_id].name)
> +		return -ENOENT;
> +
> +	if (!xe->info.has_sysctrl)
> +		return -EOPNOTSUPP;
> +
> +	return xe_ras_set_threshold(xe, DRM_XE_RAS_ERR_SEV_CORRECTABLE, error_id, threshold);

[Severity: Medium]
Is this missing a bounds check on the user-supplied threshold value?

The DRM RAS documentation in include/drm/drm_ras.h explicitly mandates
that the driver is responsible for error threshold bounds checking in the
set_error_threshold() callback. Here, the threshold value is passed directly
to xe_ras_set_threshold() and the system controller without validation.

Could this lead to speculative hardware misbehavior or interrupt storms if
userspace passes malformed values like 0 or an extremely large integer?

> +}
> +
>  static struct xe_drm_ras_counter *allocate_and_copy_counters(struct xe_device *xe)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722034228.662746-1-raag.jadav@intel.com?part=4