Re: [PATCH v2 08/11] drm/xe/ras: Report device memory errors using SIGID

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `xe_log_err` family of macros passes `ERR_PTR(err)` to the logging framework, which crashes when used with hardware component tags due to an unchecked cast.
--

commit 3a41849b5bfff0854c8490dfdd5d178d31641b09
Author: Badal Nilawar <badal.nilawar@intel.com>

drm/xe/ras: Report device memory errors using SIGID

This commit updates the device memory error handler to use the
xe_log_comp_info() helper instead of xe_info(). This allows reporting
these errors with the DEVICE_MEMORY hardware component tag.

> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
> index b4cdb5ec64915..172653be1b829 100644
> --- a/drivers/gpu/drm/xe/xe_ras.c
> +++ b/drivers/gpu/drm/xe/xe_ras.c
> @@ -409,14 +409,17 @@ static u8 handle_device_memory_errors(struct xe_device *xe, struct xe_ras_error_
>  	 */
>  	switch (info->category) {
>  	case XE_RAS_MEMORY_POISON:
> -		xe_info(xe, "[RAS]: Poison error detected\n");
> +		xe_log_comp_info(xe, DEVICE_MEMORY, &arr->counter, sizeof(arr->counter),
> +				 "Poison error detected\n");

[Severity: Medium]
Does introducing the DEVICE_MEMORY hardware tag expose a latent API trap in
the logging framework?

While this specific call passes a valid pointer (&arr->counter), the logging
macro family is documented to allow passing ERR_PTR(err) as the data
parameter.

If a developer later calls xe_log_err(any, DEVICE_MEMORY, err, ...), it
passes ERR_PTR(err) as the data parameter. This gets processed in
log_emit_cper():

drivers/gpu/drm/xe/xe_log.c:log_emit_cper() {
    ...
    if (is_hw_sigid(sigid))
        xe_emit_hardware_error_cper(pdev, cper_sev, sigid,
                                    (struct xe_ras_error_class *)data);
    ...
}

This blindly casts the data parameter to struct xe_ras_error_class *.
Then xe_emit_hardware_error_cper() checks if it is non-NULL but fails to check
for an error pointer:

drivers/gpu/drm/xe/xe_ras.c:xe_emit_hardware_error_cper() {
    ...
    if (counter && !ras_counter_is_valid(xe, counter))
    ...
}

This leads to an invalid pointer dereference when ras_counter_is_valid()
accesses the severity field:

drivers/gpu/drm/xe/xe_ras.c:ras_counter_is_valid() {
    ...
    u8 severity = counter->common.severity;
    ...
}

Should an IS_ERR() check be added in log_emit_cper() or
xe_emit_hardware_error_cper() before dereferencing the pointer to prevent a
potential kernel crash?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825175916.1103841-13-badal.nilawar@intel.com?part=8