Re: [PATCH] drm/i915/selftests: avoid using uninitialized context

[Sebastian Brzezinka <sebastian.brzezinka@intel.com>]

Hi Krzysztof,

On Mon Jan 27, 2025 at 11:46 AM UTC, Krzysztof Karas wrote:
> There is an error path in igt_ppgtt_alloc(), which leads to ww
> object being passed down to i915_gem_ww_ctx_fini() without
> initialization. Correct that by zeroing the struct.
>
> Fixes: 480ae79537b2 ("drm/i915/selftests: Prepare gtt tests for obj->mm.lock removal")
> Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
> ---
>  drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> index 5816d515203a..29b9c75557da 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> @@ -154,7 +154,7 @@ static int igt_ppgtt_alloc(void *arg)
>  {
>  	struct drm_i915_private *dev_priv = arg;
>  	struct i915_ppgtt *ppgtt;
> -	struct i915_gem_ww_ctx ww;
> +	struct i915_gem_ww_ctx ww = {};

I don't thing it's a best idea to just initialize ww here, you still have
incorrect path that try to fini ww before it was initialize.

I would probably do something like this instead.
------------------------------------
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index 5816d515203a..526518bc4dba 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -167,8 +167,10 @@ static int igt_ppgtt_alloc(void *arg)
        if (IS_ERR(ppgtt))
                        return PTR_ERR(ppgtt);

                        -       if (!ppgtt->vm.allocate_va_range)
                        -               goto err_ppgtt_cleanup;
                        +       if (!ppgtt->vm.allocate_va_range) {
                        +               i915_vm_put(&ppgtt->vm);
                        +               return 0;
                        +       }
------------------------------------

-- 
Best regards,
Sebastian