[PATCH v2 1/1] drm/xe: Prevent null pointer access in xe_migrate_copy

[Matt Roper]

On Fri, Sep 20, 2024 at 09:58:14AM +0530, Ghimiray, Himal Prasad wrote:
> 
> 
> On 20-09-2024 04:12, Zhanjun Dong wrote:
> > Update lacks source flag to include resource is null case. This will
> > prevent null pointer derefrence in xe_migrate_copy.
> > 
> > Signed-off-by: Zhanjun Dong <zhanjun.dong at intel.com>
> > ---
> >   drivers/gpu/drm/xe/xe_bo.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> > index 5f2f1ec46b57..5e8f60a8d431 100644
> > --- a/drivers/gpu/drm/xe/xe_bo.c
> > +++ b/drivers/gpu/drm/xe/xe_bo.c
> > @@ -682,8 +682,8 @@ static int xe_bo_move(struct ttm_buffer_object *ttm_bo, bool evict,
> >   	tt_has_data = ttm && (ttm_tt_is_populated(ttm) ||
> >   			      (ttm->page_flags & TTM_TT_FLAG_SWAPPED));
> > -	move_lacks_source = handle_system_ccs ? (!bo->ccs_cleared)  :
> > -						(!mem_type_is_vram(old_mem_type) && !tt_has_data);
> > +	move_lacks_source = !old_mem || (handle_system_ccs ? (!bo->ccs_cleared) :
> > +					 (!mem_type_is_vram(old_mem_type) && !tt_has_data));
> 
> 
> Just for curiosity, isn't !old_mem implicitly taken care here ?
> shouldn't ttm be NULL, if resource is NULL ? IIRC, this was what Thomas had
> confirmed during handle_system_ccs implementation.

Drive-by comment:  If this is an invariant, it might still be worth
adding an xe_assert() so that CI can ensure the condition never gets
violated by future code refactors and design changes.


Matt

> 
> Thomas/Matt,
> Can you confirm here ?
> 
> BR
> Himal
> 
> 
> >   	needs_clear = (ttm && ttm->page_flags & TTM_TT_FLAG_ZERO_ALLOC) ||
> >   		(!ttm && ttm_bo->type == ttm_bo_type_device);

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation