Re: [PATCH] drm/xe: Fix bounds checking in __xe_bo_placement_for_flags()

[Matthew Brost <matthew.brost@intel.com>]

On Wed, Jan 10, 2024 at 04:21:11PM -0800, Brian Welty wrote:
> Requesting all memory regions on PVC will fill bo->placements up to
> XE_BO_MAX_PLACEMENTS. The subsequent call to try_add_stolen() will trip
> over the bounds checking even though XE_PL_STOLEN is not expected to
> be used in this case.
> 
> This is hit with igt@xe_exec_fault_mode@once-basic-prefetch:
>     xe 0000:8c:00.0: [drm] Assertion `*c < (sizeof(bo->placements) / sizeof((bo->placements)[0]) + ((int)(sizeof(struct { int:(-!!(__builtin_types_compatible_p(typeof((bo->placements)), typeof(&(bo->placements)[0])))); }))))` failed!
>     WARNING: CPU: 30 PID: 6161 at drivers/gpu/drm/xe/xe_bo.c:203 __xe_bo_placement_for_flags+0x218/0x240 [xe]
> 
> Is fixed here by moving the bounds checks closer to where we actually
> write into the bo->placement array.
> 
> Fixes: 8c54ee8a8606 ("drm/xe: Ensure that we don't access the placements array out-of-bounds")
> Signed-off-by: Brian Welty <brian.welty@intel.com>

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> ---
>  drivers/gpu/drm/xe/xe_bo.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 338f9688a2c9..26fe73f58d72 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -125,9 +125,9 @@ static struct xe_mem_region *res_to_mem_region(struct ttm_resource *res)
>  static void try_add_system(struct xe_device *xe, struct xe_bo *bo,
>  			   u32 bo_flags, u32 *c)
>  {
> -	xe_assert(xe, *c < ARRAY_SIZE(bo->placements));
> -
>  	if (bo_flags & XE_BO_CREATE_SYSTEM_BIT) {
> +		xe_assert(xe, *c < ARRAY_SIZE(bo->placements));
> +
>  		bo->placements[*c] = (struct ttm_place) {
>  			.mem_type = XE_PL_TT,
>  		};
> @@ -145,6 +145,8 @@ static void add_vram(struct xe_device *xe, struct xe_bo *bo,
>  	struct xe_mem_region *vram;
>  	u64 io_size;
>  
> +	xe_assert(xe, *c < ARRAY_SIZE(bo->placements));
> +
>  	vram = to_xe_ttm_vram_mgr(ttm_manager_type(&xe->ttm, mem_type))->vram;
>  	xe_assert(xe, vram && vram->usable_size);
>  	io_size = vram->io_size;
> @@ -175,8 +177,6 @@ static void add_vram(struct xe_device *xe, struct xe_bo *bo,
>  static void try_add_vram(struct xe_device *xe, struct xe_bo *bo,
>  			 u32 bo_flags, u32 *c)
>  {
> -	xe_assert(xe, *c < ARRAY_SIZE(bo->placements));
> -
>  	if (bo->props.preferred_gt == XE_GT1) {
>  		if (bo_flags & XE_BO_CREATE_VRAM1_BIT)
>  			add_vram(xe, bo, bo->placements, bo_flags, XE_PL_VRAM1, c);
> @@ -193,9 +193,9 @@ static void try_add_vram(struct xe_device *xe, struct xe_bo *bo,
>  static void try_add_stolen(struct xe_device *xe, struct xe_bo *bo,
>  			   u32 bo_flags, u32 *c)
>  {
> -	xe_assert(xe, *c < ARRAY_SIZE(bo->placements));
> -
>  	if (bo_flags & XE_BO_CREATE_STOLEN_BIT) {
> +		xe_assert(xe, *c < ARRAY_SIZE(bo->placements));
> +
>  		bo->placements[*c] = (struct ttm_place) {
>  			.mem_type = XE_PL_STOLEN,
>  			.flags = bo_flags & (XE_BO_CREATE_PINNED_BIT |
> -- 
> 2.43.0
>