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 04/02/2026 18:15, Yao, Jia wrote:
> Thanks! May I add your Reviewed-by tag in the next revision?

Yes, with the comments addressed.

> 
>> -----Original Message-----
>> From: Auld, Matthew <matthew.auld@intel.com>
>> Sent: Wednesday, February 4, 2026 7:44 AM
>> To: Yao, Jia <jia.yao@intel.com>; intel-xe@lists.freedesktop.org
>> Cc: Brost, Matthew <matthew.brost@intel.com>; Lin, Shuicheng
>> <shuicheng.lin@intel.com>; Ghimiray, Himal Prasad
>> <himal.prasad.ghimiray@intel.com>; Thomas Hellström
>> <thomas.hellstrom@linux.intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>
>> Subject: Re: [PATCH v2] drm/xe: Add bounds check on pat_index to prevent
>> OOB kernel read in madvise
>>
>> 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;
>