[PATCH i-g-t v5 06/17] lib/intel_batchbuffer: Add helper to get pointer at specified offset

[Zbigniew Kempczyński]

On Fri, Aug 30, 2024 at 11:44:11AM +0200, Manszewski, Christoph wrote:
> Hi Zbigniew,
> 
> On 30.08.2024 11:36, Zbigniew Kempczyński wrote:
> > On Thu, Aug 29, 2024 at 04:45:36PM +0200, Christoph Manszewski wrote:
> > > From: Andrzej Hajda <andrzej.hajda at intel.com>
> > > 
> > > The helper will be used to access data placed in batchbuffer.
> > > 
> > > Signed-off-by: Andrzej Hajda <andrzej.hajda at intel.com>
> > > Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> > > Cc: Dominik Grzegorzek <dominik.grzegorzek at intel.com>
> > > Acked-by: Dominik Grzegorzek <dominik.grzegorzek at intel.com>
> > > ---
> > >   lib/intel_batchbuffer.h | 7 +++++++
> > >   1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/lib/intel_batchbuffer.h b/lib/intel_batchbuffer.h
> > > index cb32206e5..b1670d23e 100644
> > > --- a/lib/intel_batchbuffer.h
> > > +++ b/lib/intel_batchbuffer.h
> > > @@ -353,6 +353,13 @@ static inline uint32_t intel_bb_offset(struct intel_bb *ibb)
> > >   	return (uint32_t) ((uint8_t *) ibb->ptr - (uint8_t *) ibb->batch);
> > >   }
> > > +static inline void *intel_bb_ptr_get(struct intel_bb *ibb, uint32_t offset)
> > > +{
> > > +	igt_assert(offset <= ibb->size);
> > 
> > I think it should be < only, == means we're outside the buffer.
> 
> Now that you say it - it makes sense. To be honest I just took a similar
> approach to intel_bb_ptr_set. But I will change it to '<' for
> intel_bb_ptr_get then.

It seems that intel_bb_ptr_set() is also wrong, it should be < too.

Regarding 'get' - nothing will protect us from using returned pointer
out of bounds leading to memory corruption. But at least we may try
to catch some cases early.

Feel free to fix 'set' as well.

--
Zbigniew

> 
> Thanks,
> Christoph
> > 
> > --
> > Zbigniew
> > 
> > > +
> > > +	return ((uint8_t *) ibb->batch + offset);
> > > +}
> > > +
> > >   static inline void intel_bb_ptr_set(struct intel_bb *ibb, uint32_t offset)
> > >   {
> > >   	ibb->ptr = (void *) ((uint8_t *) ibb->batch + offset);
> > > -- 
> > > 2.34.1
> > >