Re: [PATCH] drm/i915/gt: Prevent uninitialized pointer reads

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Mon, Dec 30, 2024 at 02:24:45PM +0000, Krzysztof Karas wrote:
> Hi Apoorva and Andi,
> 
> > > Initialize rq to NULL to prevent uninitialized pointer reads.
> > 
> > where is it happening?
> > 
> > Andi
> 
> There are two instances that I see:
> 1) err = fn(..., &rq); <- this probably will set the rq (at least
> looking at the functions that are used to set "fn", when calling
> "clear()"), but from the clear()'s execution perspective there are
> no guarantees that this value will be initialized.
> 
> 2) If an "err" is set and detected before "rq" is initialized, then
> there is a path that the execution might take that leads to reading
> uninitialized "rq":
> if all calls to i915_gem_object_pin_map() would result in -ENXIO,
> then the loop would exit with a "continue" and err = -ENXIO. This then
> triggers "if (err)" after the loop and takes the second "if", which
> starts by checking the value of "rq", which at this point would still
> be uninitialized.
> 
> I think this initialization removes these corner cases, so it is worth
> introducing.

To be really honest, I'm not entirely convinced of the corner cases,
but just because this function is huge and this pointer is used in
the very end, far from any of the path setting it, I believe it is
already a good reason for introducing this protection.

Also, the function can change in the future and this could be
forgotten.

So, I just pushed the patch to drm-intel-gt-next.

Thanks for the patch and review,
Rodrigo.

> 
> Krzysztof
>