Re: [PATCH 1/3] drm/xe: Fix out-of-bounds warning in vm_bind_ioctl_check_args

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

On Thu, Mar 14, 2024 at 10:55:42AM +0000, Matthew Auld wrote:
> On 14/03/2024 09:54, Nirmoy Das wrote:
> > Modify how bind_ops is assigned and used within the for loop to
> > accommodate both array and single bind scenarios which should prevent
> > out-of-bounds access warning from static code analysis tool.
> > 
> > Cc: Matthew Auld <matthew.auld@intel.com>
> > Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> 
> As discussed offline, current thinking is to just ignore this one. Looks
> like false positive. Also we still end up doing &(*bind_ops)[i] so probably
> it still complains.
> 

Agree with Matt Auld here.

Matt

> > ---
> >   drivers/gpu/drm/xe/xe_vm.c | 19 ++++++++++---------
> >   1 file changed, 10 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> > index cbb9b8935c90..0b742d8d82a5 100644
> > --- a/drivers/gpu/drm/xe/xe_vm.c
> > +++ b/drivers/gpu/drm/xe/xe_vm.c
> > @@ -2839,15 +2839,16 @@ static int vm_bind_ioctl_check_args(struct xe_device *xe,
> >   	}
> >   	for (i = 0; i < args->num_binds; ++i) {
> > -		u64 range = (*bind_ops)[i].range;
> > -		u64 addr = (*bind_ops)[i].addr;
> > -		u32 op = (*bind_ops)[i].op;
> > -		u32 flags = (*bind_ops)[i].flags;
> > -		u32 obj = (*bind_ops)[i].obj;
> > -		u64 obj_offset = (*bind_ops)[i].obj_offset;
> > -		u32 prefetch_region = (*bind_ops)[i].prefetch_mem_region_instance;
> > +		struct drm_xe_vm_bind_op *cur_ops = &(*bind_ops)[i];
> > +		u64 range = cur_ops->range;
> > +		u64 addr = cur_ops->addr;
> > +		u32 op = cur_ops->op;
> > +		u32 flags = cur_ops->flags;
> > +		u32 obj = cur_ops->obj;
> > +		u64 obj_offset = cur_ops->obj_offset;
> > +		u32 prefetch_region = cur_ops->prefetch_mem_region_instance;
> >   		bool is_null = flags & DRM_XE_VM_BIND_FLAG_NULL;
> > -		u16 pat_index = (*bind_ops)[i].pat_index;
> > +		u16 pat_index = cur_ops->pat_index;
> >   		u16 coh_mode;
> >   		if (XE_IOCTL_DBG(xe, pat_index >= xe->pat.n_entries)) {
> > @@ -2856,7 +2857,7 @@ static int vm_bind_ioctl_check_args(struct xe_device *xe,
> >   		}
> >   		pat_index = array_index_nospec(pat_index, xe->pat.n_entries);
> > -		(*bind_ops)[i].pat_index = pat_index;
> > +		cur_ops->pat_index = pat_index;
> >   		coh_mode = xe_pat_index_get_coh_mode(xe, pat_index);
> >   		if (XE_IOCTL_DBG(xe, !coh_mode)) { /* hw reserved */
> >   			err = -EINVAL;