Re: [PATCH v4] drm/xe/uapi: Reject coh_none PAT index for CPU cached memory in madvise

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

On 03/02/2026 15:48, Jia Yao wrote:
> Add validation in xe_vm_madvise_ioctl() to reject PAT indices with
> XE_COH_NONE coherency mode when applied to CPU cached memory.
> 
> Using coh_none with CPU cached buffers is a security issue. When the
> kernel clears pages before reallocation, the clear operation stays in
> CPU cache (dirty). GPU with coh_none can bypass CPU caches and read
> stale sensitive data directly from DRAM, potentially leaking data from
> previously freed pages of other processes.
> 
> This aligns with the existing validation in vm_bind path
> (xe_vm_bind_ioctl_validate_bo).
> 
> v2(Matthew brost)
> - Add fixes
> - Move one debug print to better place
> 
> v3(Matthew Auld)
> - Should be drm/xe/uapi
> - More Cc
> 
> v4(Shuicheng Lin)
> - Fix kmem leak issues by the way
> 
> Fixes: ada7486c5668 ("drm/xe: Implement madvise ioctl for xe")
> Cc: stable@vger.kernel.org # v6.18
> Cc: Shuicheng Lin <shuicheng.lin@intel.com>
> Cc: Mathew Alwin <alwin.mathew@intel.com>
> Cc: Michal Mrozek <michal.mrozek@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Signed-off-by: Jia Yao <jia.yao@intel.com>

Unless I'm blind, it looks like there is some missing validation on the 
pat_index coming from userspace also, where we can trigger OOB kernel 
read when calling get_coh_mode(), if malicious user gives you a bogus 
too large index. I think we need to fix that also, maybe as a seperate 
patch in this series or just send as seperate fix and get it landed ASAP?

> ---
>   drivers/gpu/drm/xe/xe_vm_madvise.c | 55 +++++++++++++++++++++++++++---
>   1 file changed, 50 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_vm_madvise.c b/drivers/gpu/drm/xe/xe_vm_madvise.c
> index add9a6ca2390..bf41fe75a336 100644
> --- a/drivers/gpu/drm/xe/xe_vm_madvise.c
> +++ b/drivers/gpu/drm/xe/xe_vm_madvise.c
> @@ -74,7 +74,7 @@ static int get_vmas(struct xe_vm *vm, struct xe_vmas_in_madvise_range *madvise_r
>   		}
>   
>   		madvise_range->vmas[madvise_range->num_vmas] = vma;
> -		(madvise_range->num_vmas)++;
> +		madvise_range->num_vmas++;
>   	}
>   
>   	if (!madvise_range->num_vmas)
> @@ -352,6 +352,43 @@ static void xe_madvise_details_fini(struct xe_madvise_details *details)
>   	drm_pagemap_put(details->dpagemap);
>   }
>   
> +static bool check_pat_args_are_sane(struct xe_device *xe,
> +				    struct xe_vmas_in_madvise_range *madvise_range,
> +				    u16 pat_index)
> +{
> +	u16 coh_mode = xe_pat_index_get_coh_mode(xe, pat_index);
> +	int i;
> +
> +	/*
> +	 * Using coh_none with CPU cached buffers is not allowed.
> +	 * Otherwise CPU page clearing can be bypassed, which is a
> +	 * security issue. GPU can directly access system memory and
> +	 * bypass CPU caches, potentially reading stale sensitive data
> +	 * from previously freed pages.
> +	 */
> +	if (coh_mode != XE_COH_NONE)
> +		return true;
> +
> +	for (i = 0; i < madvise_range->num_vmas; i++) {
> +		struct xe_vma *vma = madvise_range->vmas[i];
> +		struct xe_bo *bo = xe_vma_bo(vma);
> +
> +		if (bo) {
> +			/* BO with WB caching + COH_NONE is not allowed */
> +			if (XE_IOCTL_DBG(xe, bo->cpu_caching == DRM_XE_GEM_CPU_CACHING_WB))
> +				return false;
> +			/* Imported dma-buf without caching info, assume cached */
> +			if (XE_IOCTL_DBG(xe, !bo->cpu_caching))
> +				return false;
> +		} else if (XE_IOCTL_DBG(xe, xe_vma_is_cpu_addr_mirror(vma) ||
> +					    xe_vma_is_userptr(vma)))
> +			/* System memory (userptr/SVM) is always CPU cached */
> +			return false;
> +	}
> +
> +	return true;
> +}
> +
>   static bool check_bo_args_are_sane(struct xe_vm *vm, struct xe_vma **vmas,
>   				   int num_vmas, u32 atomic_val)
>   {
> @@ -388,12 +425,12 @@ static bool check_bo_args_are_sane(struct xe_vm *vm, struct xe_vma **vmas,
>   	return true;
>   }
>   /**
> - * xe_vm_madvise_ioctl - Handle MADVise ioctl for a VM
> + * xe_vm_madvise_ioctl - Handle madvise ioctl for a VM
>    * @dev: DRM device pointer
>    * @data: Pointer to ioctl data (drm_xe_madvise*)
>    * @file: DRM file pointer
>    *
> - * Handles the MADVISE ioctl to provide memory advice for vma's within
> + * Handles the madvise ioctl to provide memory advice for vma's within
>    * input range.
>    *
>    * Return: 0 on success or a negative error code on failure.
> @@ -442,13 +479,21 @@ int xe_vm_madvise_ioctl(struct drm_device *dev, void *data, struct drm_file *fil
>   	if (err || !madvise_range.num_vmas)
>   		goto madv_fini;
>   
> +	if (args->type == DRM_XE_MEM_RANGE_ATTR_PAT) {
> +		if (!check_pat_args_are_sane(xe, &madvise_range,
> +					     args->pat_index.val)) {
> +			err = -EINVAL;
> +			goto free_vmas;
> +		}
> +	}
> +
>   	if (madvise_range.has_bo_vmas) {
>   		if (args->type == DRM_XE_MEM_RANGE_ATTR_ATOMIC) {
>   			if (!check_bo_args_are_sane(vm, madvise_range.vmas,
>   						    madvise_range.num_vmas,
>   						    args->atomic.val)) {
>   				err = -EINVAL;
> -				goto madv_fini;
> +				goto free_vmas;
>   			}
>   		}
>   
> @@ -485,8 +530,8 @@ int xe_vm_madvise_ioctl(struct drm_device *dev, void *data, struct drm_file *fil
>   err_fini:
>   	if (madvise_range.has_bo_vmas)
>   		drm_exec_fini(&exec);
> +free_vmas:
>   	kfree(madvise_range.vmas);
> -	madvise_range.vmas = NULL;
>   madv_fini:
>   	xe_madvise_details_fini(&details);
>   unlock_vm: