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

[Sachin Kamat]

On 16 January 2014 10:51, Inki Dae <inki.dae at samsung.com> wrote:
>
>
>> -----Original Message-----
>> From: Sachin Kamat [mailto:sachin.kamat at linaro.org]
>> Sent: Thursday, January 16, 2014 12:32 PM
>> To: dri-devel at lists.freedesktop.org
>> Cc: inki.dae at samsung.com; jy0922.shim at samsung.com; sw0312.kim at samsung.com;
>> sachin.kamat at linaro.org; patches at linaro.org
>> 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 at linaro.org>
>> ---
>>  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?

Actually that was what I was thinking of doing initially. However I
wasn't sure about
the logic of not doing so :). Hence tried to keep it safe. Will update
as below and resend.

>
>         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.
>

-- 
With warm regards,
Sachin