Re: [PATCH v2] drm/xe/ras: Fix sysctrl and drm-ras discrepancy

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Out-of-bounds array read leads to undefined behavior and potential compiler elision of the bounds check, causing a kernel panic.
--

commit 94038693bb6905d1ad4adc4ff5a47f0c15f8cf43
Author: Raag Jadav <raag.jadav@intel.com>

drm/xe/ras: Fix sysctrl and drm-ras discrepancy

This commit adds a validity check to prevent redundant error-event logging
when an error is valid for sysctrl but not exposed as a valid uapi through
drm-ras.

> diff --git a/drivers/gpu/drm/xe/xe_drm_ras.c b/drivers/gpu/drm/xe/xe_drm_ras.c
> index 7f3695707611c..5e6d4926d08e9 100644
> --- a/drivers/gpu/drm/xe/xe_drm_ras.c
> +++ b/drivers/gpu/drm/xe/xe_drm_ras.c
[ ... ]
> @@ -239,6 +250,9 @@ void xe_drm_ras_event(struct xe_device *xe, u32 component, u32 severity, u32 val
>  	if (!xe->info.has_drm_ras)
>  		return;
>  
> +	if (!drm_ras_counter_is_valid(severity, component))
> +		return;
> +

[Severity: High]
This is a pre-existing issue, but does evaluating the array access before
this bounds check introduce a risk of compiler elision?

When hardware reports an informational error, the severity can be mapped to
DRM_XE_RAS_ERR_SEV_MAX. If xe_drm_ras_event() is called with
severity == DRM_XE_RAS_ERR_SEV_MAX, an out-of-bounds read occurs during
variable initialization at the top of the function:

void xe_drm_ras_event(struct xe_device *xe, u32 component, u32 severity, ...)
{
        struct xe_drm_ras *ras = &xe->ras;
        struct xe_drm_ras_counter *info = ras->info[severity];

Because out-of-bounds reads are undefined behavior, modern compilers can use
Value Range Propagation to assume severity is always within bounds. Could
this cause the compiler to optimize away the newly added
drm_ras_counter_is_valid() check entirely?

If the check is elided, execution would proceed with info holding a garbage
pointer, potentially leading to a wild pointer dereference when
info[component].name is accessed later.

Should the initialization of info be deferred until after the bounds check?

>  	node = &ras->node[severity];
>  
>  	if (!info || !info[component].name)

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