Re: [PATCH 2/2] drm/xe: revamp hw reserved PAT index checking

[Matthew Auld <matthew.auld@intel.com>]

On 15/10/2026 11:05, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] drm/xe: missing array_index_nospec for pat_index
> --
> 
> commit c33d7dbef37f797548fdacd3eb5da040ca907442
> Author: Matthew Auld <matthew.auld@intel.com>
> 
> drm/xe: revamp hw reserved PAT index checking
> 
> This commit replaces indirect checks of valid PAT indices with a direct
> check of pat.valid. It also adds additional WARN_ON checks to ensure
> coherency modes are valid.
> 
>> diff --git a/drivers/gpu/drm/xe/xe_pat.c b/drivers/gpu/drm/xe/xe_pat.c
>> index 18e26f04169ab..d0dac1ebd4784 100644
>> --- a/drivers/gpu/drm/xe/xe_pat.c
>> +++ b/drivers/gpu/drm/xe/xe_pat.c
> [ ... ]
>>   u16 xe_pat_index_get_coh_mode(struct xe_device *xe, u16 pat_index)
>>   {
>>   	WARN_ON(pat_index >= xe->pat.n_entries);
>> +	WARN_ON(!xe->pat.table[pat_index].coh_mode ||
>> +		xe->pat.table[pat_index].coh_mode > XE_COH_2WAY);
>>   	return xe->pat.table[pat_index].coh_mode;
>>   }
> 
> [Severity: Low]
> Does this code leak information via a Spectre v1 side channel?
> 
> When handling the MADVISE ioctl with DRM_XE_MEM_RANGE_ATTR_PAT, the raw
> user input args->pat_index.val is passed into check_pat_args_are_sane()
> without being clamped by array_index_nospec():
> 
> xe_vm_madvise_ioctl()
>    check_pat_args_are_sane(xe, &madvise_range, args->pat_index.val)
>      xe_pat_index_get_coh_mode(xe, pat_index)
> 
> Under speculative execution, a user-controlled, out-of-bounds pat_index
> could bypass the earlier bounds check in madvise_args_are_sane(). This
> would cause xe_pat_index_get_coh_mode() to read out-of-bounds memory
> when indexing xe->pat.table[pat_index]. The resulting coh_mode is then
> used in branch conditions in check_pat_args_are_sane(), creating a
> potential side channel.
> 
> Could args->pat_index.val be clamped with array_index_nospec() before
> being passed to check_pat_args_are_sane(), or could the already-clamped
> pat_index local variable be reused in xe_vm_madvise_ioctl()?

Fair, but I would view this as more of a pre-existing issue.

>