| Message ID | 1389843105-20741-1-git-send-email-sachin.kamat@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 16 January 2014 10:51, Inki Dae <inki.dae@samsung.com> wrote: > > >> -----Original Message----- >> From: Sachin Kamat [mailto:sachin.kamat@linaro.org] >> Sent: Thursday, January 16, 2014 12:32 PM >> To: dri-devel@lists.freedesktop.org >> Cc: inki.dae@samsung.com; jy0922.shim@samsung.com; sw0312.kim@samsung.com; >> sachin.kamat@linaro.org; patches@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@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. >
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; } @@ -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; }
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@linaro.org> --- drivers/gpu/drm/exynos/exynos_drm_drv.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)