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

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

On 03/02/2026 21:06, 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
> 
> Fixes: ada7486c5668 ("drm/xe: Implement madvise ioctl for xe")
> 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>

I think you dropped the stable Cc.

> ---
>   drivers/gpu/drm/xe/xe_vm_madvise.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_vm_madvise.c b/drivers/gpu/drm/xe/xe_vm_madvise.c
> index add9a6ca2390..3564e1442694 100644
> --- a/drivers/gpu/drm/xe/xe_vm_madvise.c
> +++ b/drivers/gpu/drm/xe/xe_vm_madvise.c
> @@ -291,7 +291,12 @@ 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);
> +		if (XE_IOCTL_DBG(xe, args->pat_index.val >= xe->pat.n_entries))
> +			return false;
> +
> +		u16 pat_index = array_index_nospec(args->pat_index.val, xe->pat.n_entries);
> +
> +		u16 coh_mode = xe_pat_index_get_coh_mode(xe, pat_index);

nit: I think the normal coding style is to put the declarations at the 
start of the block. The rest of this file seems to follow that.

Otherwise:
Reviewed-by: Matthew Auld <matthew.auld@intel.com>

>   
>   		if (XE_IOCTL_DBG(xe, !coh_mode))
>   			return false;