Re: [PATCH v6 2/5] drm/ras: Introduce error threshold

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The drm_ras_node pointer returned by xa_load() is used without RCU protection or locking, leading to a Use-After-Free if the node is concurrently unregistered.
--

--- Patch [2]: [PATCH v6 2/5] drm/ras: Introduce error threshold ---
commit 4fde8387621a92c01fa74b45df16200c6e02bd2a
Author: Raag Jadav <raag.jadav@intel.com>

drm/ras: Introduce error threshold

Add get-error-threshold and set-error-threshold command support which
allows querying/setting error threshold of the counter. Threshold in RAS
context means the number of errors the hardware is expected to accumulate
before it raises them to software.

> diff --git a/drivers/gpu/drm/drm_ras.c b/drivers/gpu/drm/drm_ras.c
> --- a/drivers/gpu/drm/drm_ras.c
> +++ b/drivers/gpu/drm/drm_ras.c
[ ... ]
> +static int get_node_error_threshold(u32 node_id, u32 error_id, const char **name, u32 *threshold)
> +{
> +	struct drm_ras_node *node;
> +
> +	node = xa_load(&drm_ras_xa, node_id);
> +	if (!node)
> +		return -ENOENT;
> +
> +	if (!node->query_error_threshold)
> +		return -EOPNOTSUPP;
> +
> +	if (error_id < node->error_counter_range.first || error_id > node->error_counter_range.last)
> +		return -EINVAL;
> +
> +	return node->query_error_threshold(node, error_id, name, threshold);

[Severity: High]
This is a pre-existing issue, but can this cause a use-after-free?

The drm_ras_node pointer returned by xa_load() is used without RCU protection 
or locking. If the node is concurrently unregistered and freed, for example 
during driver unbind, the node memory could be freed before we invoke 
node->query_error_threshold(). This would result in a use-after-free on the 
node pointer.

> +}
> +
> +static int set_node_error_threshold(u32 node_id, u32 error_id, u32 threshold)
> +{
> +	struct drm_ras_node *node;
> +
> +	node = xa_load(&drm_ras_xa, node_id);
> +	if (!node)
> +		return -ENOENT;
> +
> +	if (!node->set_error_threshold)
> +		return -EOPNOTSUPP;
> +
> +	if (error_id < node->error_counter_range.first || error_id > node->error_counter_range.last)
> +		return -EINVAL;
> +
> +	return node->set_error_threshold(node, error_id, threshold);

[Severity: High]
This is a pre-existing issue, but does this suffer from the same concurrent 
unregistration race? 

If the node pointer from xa_load() is freed before we call 
node->set_error_threshold(), this dereference could also access freed memory.

> +}

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