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.