Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Missing bounds check on the hardware-provided `component` value before 
using it in the `BIT(component)` macro, leading to undefined behavior (shift 
out of bounds) and failure of the deduplication logic.
--

--- Patch [2]: [PATCH v5 2/3] drm/xe/xe_ras: Report correctable error events to 
userspace ---
commit 5a086f0455803475a13c848290347fe3571d79fa
Author: Riana Tauro <[email protected]>

drm/xe/xe_ras: Report correctable error events to userspace

When an interrupt indicates the error counter has crossed its threshold,
this commit reads the current counter value and delivers a drm_ras error
event to userspace. It also attempts to send the error event only once
per component.

> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
> index ed609912fda14..cf85075d62667 100644
> --- a/drivers/gpu/drm/xe/xe_ras.c
> +++ b/drivers/gpu/drm/xe/xe_ras.c
[ ... ]
> @@ -241,8 +293,10 @@ void xe_ras_counter_threshold_crossed(struct xe_device 
> *xe,
>       struct xe_ras_threshold_crossed *pending = (void *)&response->data;
>       struct xe_ras_error_class *errors = pending->counters;
>       u32 id, ncounters = pending->ncounters;
> +     u8 sent = 0;
>  
>       BUILD_BUG_ON(sizeof(response->data) < sizeof(*pending));
> +     BUILD_BUG_ON(BITS_PER_TYPE(sent) < XE_RAS_COMP_MAX);
>       xe_device_assert_mem_access(xe);
>  
>       if (!ncounters || ncounters > XE_RAS_NUM_COUNTERS)
> @@ -256,8 +310,21 @@ void xe_ras_counter_threshold_crossed(struct xe_device 
> *xe,
>               severity = errors[id].common.severity;
>               component = errors[id].common.component;
>  
> +             if (severity != XE_RAS_SEV_CORRECTABLE) {
> +                     xe_warn(xe, "sysctrl: unexpected severity %s (%u)\n", 
> sev_to_str(severity),
> +                             severity);
> +                     continue;
> +             }
> +
>               xe_warn(xe, "[RAS]: %s %s detected\n",
>                       comp_to_str(component), sev_to_str(severity));
> +
> +             /* Send event once per component */
> +             if (sent & BIT(component))
> +                     continue;
> +             sent |= BIT(component);

[Severity: High]
Since component comes directly from the firmware response in
errors[id].common.component, could this cause a shift out of bounds or
truncation if the firmware sends an unexpected component ID?

If component >= 8, BIT(component) evaluates to 256 or larger. When this is
bitwise-ORed into the u8 sent mask, it truncates to 0 and the deduplication
logic fails. If component >= 64, this will trigger a shift out of bounds.

Should there be a bounds check against XE_RAS_COMP_MAX before using the
component value in the BIT() macro?

> +
> +             ras_send_error_event(xe, severity, component);
>       }
>  }

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=2

Reply via email to