Re: [PATCH 10/10] drm/xe: Taint kernel when debug configfs parameters are set

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

Pre-existing issues:
- [High] An out-of-bounds read occurs in parse_wa_bb_lines when parsing a configfs string that does not end with a newline or whitespace.
--

--- Patch [10]: [PATCH 10/10] drm/xe: Taint kernel when debug configfs parameters are set ---
commit 7e0ffcb5910459cff420b85bbfb7e80c0fa6b9b1
Author: Stuart Summers <[email protected]>

drm/xe: Taint kernel when debug configfs parameters are set

Writing any debug configfs attribute indicates the system is running
in a non-standard configuration that may mask bugs or produce misleading
behaviour. Taint the kernel with TAINT_TEST on each successful write so
bug reports clearly reflect when debug settings were active.

> diff --git a/drivers/gpu/drm/xe/xe_configfs_debug.c b/drivers/gpu/drm/xe/xe_configfs_debug.c
> index 2429d11f27a75..6cd206ec2ec4f 100644
> --- a/drivers/gpu/drm/xe/xe_configfs_debug.c
> +++ b/drivers/gpu/drm/xe/xe_configfs_debug.c
[ ... ]
> @@ -528,6 +533,7 @@ static ssize_t wa_bb_store(struct wa_bb wa_bb[static XE_ENGINE_CLASS_MAX],
>  
>  	if (!count) {
>  		memset(wa_bb, 0, sizeof(tmp_wa_bb));
> +		add_taint(TAINT_TEST, LOCKDEP_STILL_OK);
>  		return len;
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but can this lead to an
out-of-bounds read in parse_wa_bb_lines() when called from wa_bb_store()?

If userspace writes a string to the configfs attribute that does not end
with a trailing newline (e.g., echo -n 'rcs cmd 1'), the inner parsing
logic in parse_wa_bb_lines() processes the tokens and leaves the pointer
at the null terminator:

drivers/gpu/drm/xe/xe_configfs_debug.c:parse_wa_bb_lines() {
    ...
    for (p = lines; *p; p++) {
        ...
        if (str_has_prefix(p, "cmd")) {
            for (p += strlen("cmd"); *p;) {
                ret = parse_hex(p, &val);
                ...
                p += ret;
            }
        }
        ...
    }
}

Because the outer loop unconditionally increments the pointer with p++ at the
end of each iteration, if p is already at the null terminator ('\0') after
the inner parsing, p++ pushes it out-of-bounds. The subsequent outer loop
condition check *p then reads out-of-bounds memory.

If the configfs input string is exactly 4095 bytes long, this read could
cross the PAGE_SIZE boundary of the allocated buffer and trigger a kernel
panic.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=10