Re: [PATCH] drm/radeon: fix error handling in radeon_driver_open_kms

[Alex Deucher <alexdeucher@gmail.com>]

On Mon, Jan 17, 2022 at 4:38 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> The return value was never initialized so the cleanup code executed when
> it isn't even necessary.
>
> Just add proper error handling.
>
> Fixes: 2ad5d8fca195 ("drm/radeon/radeon_kms: Fix a NULL pointer dereference in radeon_driver_open_kms()")
> Signed-off-by: Christian König <christian.koenig@amd.com>

If you kept the same labels, the patch would be smaller, but either way,

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/radeon/radeon_kms.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
> index e2488559cc9f..11ad210919c8 100644
> --- a/drivers/gpu/drm/radeon/radeon_kms.c
> +++ b/drivers/gpu/drm/radeon/radeon_kms.c
> @@ -666,18 +666,18 @@ int radeon_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
>                 fpriv = kzalloc(sizeof(*fpriv), GFP_KERNEL);
>                 if (unlikely(!fpriv)) {
>                         r = -ENOMEM;
> -                       goto out_suspend;
> +                       goto err_suspend;
>                 }
>
>                 if (rdev->accel_working) {
>                         vm = &fpriv->vm;
>                         r = radeon_vm_init(rdev, vm);
>                         if (r)
> -                               goto out_fpriv;
> +                               goto err_fpriv;
>
>                         r = radeon_bo_reserve(rdev->ring_tmp_bo.bo, false);
>                         if (r)
> -                               goto out_vm_fini;
> +                               goto err_vm_fini;
>
>                         /* map the ib pool buffer read only into
>                          * virtual address space */
> @@ -685,7 +685,7 @@ int radeon_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
>                                                         rdev->ring_tmp_bo.bo);
>                         if (!vm->ib_bo_va) {
>                                 r = -ENOMEM;
> -                               goto out_vm_fini;
> +                               goto err_vm_fini;
>                         }
>
>                         r = radeon_vm_bo_set_addr(rdev, vm->ib_bo_va,
> @@ -693,19 +693,21 @@ int radeon_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
>                                                   RADEON_VM_PAGE_READABLE |
>                                                   RADEON_VM_PAGE_SNOOPED);
>                         if (r)
> -                               goto out_vm_fini;
> +                               goto err_vm_fini;
>                 }
>                 file_priv->driver_priv = fpriv;
>         }
>
> -       if (!r)
> -               goto out_suspend;
> +       pm_runtime_mark_last_busy(dev->dev);
> +       pm_runtime_put_autosuspend(dev->dev);
> +       return 0;
>
> -out_vm_fini:
> +err_vm_fini:
>         radeon_vm_fini(rdev, vm);
> -out_fpriv:
> +err_fpriv:
>         kfree(fpriv);
> -out_suspend:
> +
> +err_suspend:
>         pm_runtime_mark_last_busy(dev->dev);
>         pm_runtime_put_autosuspend(dev->dev);
>         return r;
> --
> 2.25.1
>