RE: [PATCH 1/1] drm/exynos: Fix freeing issues in exynos_drm_drv.c

[Inki Dae <inki.dae@xxxxxxxxxxx>]

> -----Original Message-----
> From: Sachin Kamat [mailto:sachin.kamat@xxxxxxxxxx]
> Sent: Thursday, January 16, 2014 12:32 PM
> To: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> Cc: inki.dae@xxxxxxxxxxx; jy0922.shim@xxxxxxxxxxx; sw0312.kim@xxxxxxxxxxx;
> sachin.kamat@xxxxxxxxxx; patches@xxxxxxxxxx
> Subject: [PATCH 1/1] drm/exynos: Fix freeing issues in exynos_drm_drv.c
> 
> Make 'file_priv' NULL upon freeing and add a check before dereferencing to
> avoid the following errors:
> drivers/gpu/drm/exynos/exynos_drm_drv.c:182 exynos_drm_open()
> error: double free of 'file_priv'
> drivers/gpu/drm/exynos/exynos_drm_drv.c:188 exynos_drm_open()
> error: dereferencing freed memory 'file_priv'
> 
> Signed-off-by: Sachin Kamat <sachin.kamat@xxxxxxxxxx>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_drv.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 9d096a0..ee84a7b6 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -173,6 +173,7 @@ static int exynos_drm_open(struct drm_device *dev,
> struct drm_file *file)
>  	ret = exynos_drm_subdrv_open(dev, file);
>  	if (ret) {
>  		kfree(file_priv);
> +		file_priv = NULL;
>  		file->driver_priv = NULL;

Thanks you for patch but it would better to just return error at here.
Actually I missed it. So could you correct and re-post it like below?

  	ret = exynos_drm_subdrv_open(dev, file);
  	if (ret) {
  		kfree(file_priv);
		file->driver_priv = NULL;
		return ret;   <- add this line.
	}
	...
 	if (IS_ERR(anon_filp)) {
		kfree(file_priv);
		file->driver_priv = NULL; <- add this line.
		return PTR_ERR(anon_filp);
	}

Or, you can do more cleanup using "goto" to avoid duplicated codes,
kfree(file_priv) and file->driver_prive = NULL.

Thanks,
Inki Dae

>  	}
> 
> @@ -184,7 +185,8 @@ static int exynos_drm_open(struct drm_device *dev,
> struct drm_file *file)
>  	}
> 
>  	anon_filp->f_mode = FMODE_READ | FMODE_WRITE;
> -	file_priv->anon_filp = anon_filp;
> +	if (file_priv)
> +		file_priv->anon_filp = anon_filp;
> 
>  	return ret;
>  }
> --
> 1.7.9.5

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel