Re: [PATCH v3 3/3] drm/xe: Add up-to-date implementation for Wa_14026539277

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

Pre-existing issues:
- [High] Spectre v1 vulnerability due to missing use of mitigated array index in `xe_vm_madvise_ioctl`.
--

--- Patch [3]: [PATCH v3 3/3] drm/xe: Add up-to-date implementation for Wa_14026539277 ---
commit a64f477e6e9a2ceb517c012bf3dbfd5ac3afdd27
Author: Gustavo Sousa <[email protected]>
Subject: drm/xe: Add up-to-date implementation for Wa_14026539277

This commit updates the implementation for Wa_14026539277. It converts
device-cacheable accesses to device-uncached and 1-way-coherent for
driver-internal usage, and rejects 2-way-coherent VM_BIND/MADVISE calls
on the UAPI side.

[ ... ]

> diff --git a/drivers/gpu/drm/xe/xe_vm_madvise.c b/drivers/gpu/drm/xe/xe_vm_madvise.c
> index 0474768a38aaa..265273294756c 100644
> --- a/drivers/gpu/drm/xe/xe_vm_madvise.c
> +++ b/drivers/gpu/drm/xe/xe_vm_madvise.c
> @@ -500,6 +500,9 @@ static bool check_pat_args_are_sane(struct xe_device *xe,
>  	u16 coh_mode = xe_pat_index_get_coh_mode(xe, pat_index);
>  	int i;
>  
> +	if (XE_IOCTL_DBG(xe, xe_pat_wa_14026539277_reserved(xe, pat_index)))
> +		return false;
> +

[Severity: High]
This is a pre-existing issue, but does this addition exacerbate a Spectre v1
vulnerability by adding another speculative out-of-bounds access?

In xe_vm_madvise_ioctl(), array_index_nospec() is used to mitigate the index,
but its result is discarded and the raw unmitigated args->pat_index.val is
passed to check_pat_args_are_sane():

drivers/gpu/drm/xe/xe_vm_madvise.c:xe_vm_madvise_ioctl() {
    ...
		pat_index = array_index_nospec(args->pat_index.val, xe->pat.n_entries);
		coh_mode = xe_pat_index_get_coh_mode(xe, pat_index);
    ...
	if (args->type == DRM_XE_MEM_RANGE_ATTR_PAT) {
		if (!check_pat_args_are_sane(xe, &madvise_range,
					     args->pat_index.val)) {
    ...
}

This means check_pat_args_are_sane() receives the unmitigated index. It then
passes it to xe_pat_index_get_coh_mode() and now to
xe_pat_wa_14026539277_reserved(), both of which will access the
xe->pat.table array.

Since the ioctl is accessible to userspace, an attacker could provide a
maliciously crafted args->pat_index.val exceeding xe->pat.n_entries and
trigger speculative out-of-bounds array reads. Should the caller be updated
to pass the mitigated pat_index?

>  	/*
>  	 * Using coh_none with CPU cached buffers is not allowed on iGPU.
>  	 * On iGPU the GPU shares the LLC with the CPU, so with coh_none

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