Re: [PATCH v3] drm/xe: Add bounds check on pat_index to prevent OOB kernel read in madvise

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

On 05/02/2026 16:15, Jia Yao wrote:
> When user provides a bogus pat_index value through the madvise IOCTL, the
> xe_pat_index_get_coh_mode() function performs an array access without
> validating bounds. This allows a malicious user to trigger an out-of-bounds
> kernel read from the xe->pat.table array.
> 
> The vulnerability exists because the validation in madvise_args_are_sane()
> directly calls xe_pat_index_get_coh_mode(xe, args->pat_index.val) without
> first checking if pat_index is within [0, xe->pat.n_entries).
> 
> Although xe_pat_index_get_coh_mode() has a WARN_ON to catch this in debug
> builds, it still performs the unsafe array access in production kernels.
> 
> v2(Matthew Auld)
> - Using array_index_nospec() to mitigate spectre attacks when the value
> is used
> 
> v3(Matthew Auld)
> - Put the declarations at the start of the block
> 
> Fixes: ada7486c5668 ("drm/xe: Implement madvise ioctl for xe")
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> Cc: <stable@vger.kernel.org> # v6.18+
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Shuicheng Lin <shuicheng.lin@intel.com>
> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Signed-off-by: Jia Yao <jia.yao@intel.com>
> ---
>   drivers/gpu/drm/xe/xe_vm_madvise.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_vm_madvise.c b/drivers/gpu/drm/xe/xe_vm_madvise.c
> index add9a6ca2390..091e450b781c 100644
> --- a/drivers/gpu/drm/xe/xe_vm_madvise.c
> +++ b/drivers/gpu/drm/xe/xe_vm_madvise.c
> @@ -246,6 +246,10 @@ static int xe_vm_invalidate_madvise_range(struct xe_vm *vm, u64 start, u64 end)
>   
>   static bool madvise_args_are_sane(struct xe_device *xe, const struct drm_xe_madvise *args)
>   {
> +	s32 fd;
> +	u16 pat_index;
> +	u16 coh_mode;
> +
>   	if (XE_IOCTL_DBG(xe, !args))
>   		return false;
>   
> @@ -261,7 +265,7 @@ static bool madvise_args_are_sane(struct xe_device *xe, const struct drm_xe_madv
>   	switch (args->type) {
>   	case DRM_XE_MEM_RANGE_ATTR_PREFERRED_LOC:
>   	{
> -		s32 fd = (s32)args->preferred_mem_loc.devmem_fd;

Nit: This was fine. The {} is also a block.

> +		fd = (s32)args->preferred_mem_loc.devmem_fd;
>   
>   		if (XE_IOCTL_DBG(xe, fd < DRM_XE_PREFERRED_LOC_DEFAULT_SYSTEM))
>   			return false;
> @@ -291,8 +295,11 @@ static bool madvise_args_are_sane(struct xe_device *xe, const struct drm_xe_madv
>   		break;
>   	case DRM_XE_MEM_RANGE_ATTR_PAT:
>   	{
> -		u16 coh_mode = xe_pat_index_get_coh_mode(xe, args->pat_index.val);

Same here, we could just add pat_index here. Anyway, this can be fixed 
up when merging, no need to resend.

> +		if (XE_IOCTL_DBG(xe, args->pat_index.val >= xe->pat.n_entries))
> +			return false;
>   
> +		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 (XE_IOCTL_DBG(xe, !coh_mode))
>   			return false;
>