* [patch] drm/exynos: potential use after free in exynos_drm_open()
@ 2014-01-21 6:57 Dan Carpenter
2014-01-21 7:12 ` Jingoo Han
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Dan Carpenter @ 2014-01-21 6:57 UTC (permalink / raw)
To: linux-arm-kernel
If exynos_drm_subdrv_open() fails then we re-use "file_priv".
Fixes: 96f5421523df ('drm/exynos: use a new anon file for exynos gem mmaper')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 9d096a0c5f8d..3c845292845a 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -174,6 +174,7 @@ static int exynos_drm_open(struct drm_device *dev, struct drm_file *file)
if (ret) {
kfree(file_priv);
file->driver_priv = NULL;
+ return ret;
}
anon_filp = anon_inode_getfile("exynos_gem", &exynos_drm_gem_fops,
@@ -186,7 +187,7 @@ 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;
- return ret;
+ return 0;
}
static void exynos_drm_preclose(struct drm_device *dev,
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [patch] drm/exynos: potential use after free in exynos_drm_open()
2014-01-21 6:57 [patch] drm/exynos: potential use after free in exynos_drm_open() Dan Carpenter
@ 2014-01-21 7:12 ` Jingoo Han
2014-01-21 7:12 ` Inki Dae
2014-01-21 12:37 ` walter harms
2 siblings, 0 replies; 6+ messages in thread
From: Jingoo Han @ 2014-01-21 7:12 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday, January 21, 2014 3:58 PM, Dan Carpenter wrote:
>
> If exynos_drm_subdrv_open() fails then we re-use "file_priv".
>
> Fixes: 96f5421523df ('drm/exynos: use a new anon file for exynos gem mmaper')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Jingoo Han <jg1.han@samsung.com>
Yes, right.
The freed 'file_priv' should not be re-used.
Best regards,
Jingoo Han
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 9d096a0c5f8d..3c845292845a 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -174,6 +174,7 @@ static int exynos_drm_open(struct drm_device *dev, struct drm_file *file)
> if (ret) {
> kfree(file_priv);
> file->driver_priv = NULL;
> + return ret;
> }
>
> anon_filp = anon_inode_getfile("exynos_gem", &exynos_drm_gem_fops,
> @@ -186,7 +187,7 @@ 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;
>
> - return ret;
> + return 0;
> }
>
> static void exynos_drm_preclose(struct drm_device *dev,
^ permalink raw reply [flat|nested] 6+ messages in thread
* [patch] drm/exynos: potential use after free in exynos_drm_open()
2014-01-21 6:57 [patch] drm/exynos: potential use after free in exynos_drm_open() Dan Carpenter
2014-01-21 7:12 ` Jingoo Han
@ 2014-01-21 7:12 ` Inki Dae
2014-01-21 12:37 ` walter harms
2 siblings, 0 replies; 6+ messages in thread
From: Inki Dae @ 2014-01-21 7:12 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
This issue had already been reported, and fixed by Sachin. And also
that patch has been merged to exynos-drm-next. Please see below link,
http://www.spinics.net/lists/dri-devel/msg51889.html
Thanks,
Inki Dae
2014/1/21 Dan Carpenter <dan.carpenter@oracle.com>:
> If exynos_drm_subdrv_open() fails then we re-use "file_priv".
>
> Fixes: 96f5421523df ('drm/exynos: use a new anon file for exynos gem mmaper')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 9d096a0c5f8d..3c845292845a 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -174,6 +174,7 @@ static int exynos_drm_open(struct drm_device *dev, struct drm_file *file)
> if (ret) {
> kfree(file_priv);
> file->driver_priv = NULL;
> + return ret;
> }
>
> anon_filp = anon_inode_getfile("exynos_gem", &exynos_drm_gem_fops,
> @@ -186,7 +187,7 @@ 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;
>
> - return ret;
> + return 0;
> }
>
> static void exynos_drm_preclose(struct drm_device *dev,
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [patch] drm/exynos: potential use after free in exynos_drm_open()
2014-01-21 6:57 [patch] drm/exynos: potential use after free in exynos_drm_open() Dan Carpenter
2014-01-21 7:12 ` Jingoo Han
2014-01-21 7:12 ` Inki Dae
@ 2014-01-21 12:37 ` walter harms
2014-01-21 12:43 ` walter harms
2 siblings, 1 reply; 6+ messages in thread
From: walter harms @ 2014-01-21 12:37 UTC (permalink / raw)
To: linux-arm-kernel
Am 21.01.2014 07:57, schrieb Dan Carpenter:
> If exynos_drm_subdrv_open() fails then we re-use "file_priv".
>
> Fixes: 96f5421523df ('drm/exynos: use a new anon file for exynos gem mmaper')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 9d096a0c5f8d..3c845292845a 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -174,6 +174,7 @@ static int exynos_drm_open(struct drm_device *dev, struct drm_file *file)
> if (ret) {
> kfree(file_priv);
> file->driver_priv = NULL;
> + return ret;
> }
using
kfree( file->driver_priv );
file->driver_priv = NULL;
would be less confusing to read, and give checkers a better chance to spot mistakes.
(btw: file_priv could be removed from this function completely).
just my 2 cents,
re,
wh
>
> anon_filp = anon_inode_getfile("exynos_gem", &exynos_drm_gem_fops,
> @@ -186,7 +187,7 @@ 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;
>
> - return ret;
> + return 0;
> }
>
> static void exynos_drm_preclose(struct drm_device *dev,
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [patch] drm/exynos: potential use after free in exynos_drm_open()
2014-01-21 12:37 ` walter harms
@ 2014-01-21 12:43 ` walter harms
2014-01-21 13:35 ` Dan Carpenter
0 siblings, 1 reply; 6+ messages in thread
From: walter harms @ 2014-01-21 12:43 UTC (permalink / raw)
To: linux-arm-kernel
i have just noticed: The function already exits
194 static void exynos_drm_postclose(struct drm_device *dev, struct drm_file *file)
195 {
196 if (!file->driver_priv)
197 return;
198
199 kfree(file->driver_priv);
200 file->driver_priv = NULL;
201 }
Am 21.01.2014 13:37, schrieb walter harms:
>
>
> Am 21.01.2014 07:57, schrieb Dan Carpenter:
>> If exynos_drm_subdrv_open() fails then we re-use "file_priv".
>>
>> Fixes: 96f5421523df ('drm/exynos: use a new anon file for exynos gem mmaper')
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> index 9d096a0c5f8d..3c845292845a 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> @@ -174,6 +174,7 @@ static int exynos_drm_open(struct drm_device *dev, struct drm_file *file)
>> if (ret) {
>> kfree(file_priv);
>> file->driver_priv = NULL;
>> + return ret;
>> }
>
> using
> kfree( file->driver_priv );
> file->driver_priv = NULL;
>
> would be less confusing to read, and give checkers a better chance to spot mistakes.
> (btw: file_priv could be removed from this function completely).
>
> just my 2 cents,
> re,
> wh
>
>>
>> anon_filp = anon_inode_getfile("exynos_gem", &exynos_drm_gem_fops,
>> @@ -186,7 +187,7 @@ 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;
>>
>> - return ret;
>> + return 0;
>> }
>>
>> static void exynos_drm_preclose(struct drm_device *dev,
>> --
>> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [patch] drm/exynos: potential use after free in exynos_drm_open()
2014-01-21 12:43 ` walter harms
@ 2014-01-21 13:35 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2014-01-21 13:35 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jan 21, 2014 at 01:43:55PM +0100, walter harms wrote:
>
> i have just noticed: The function already exits
>
> 194 static void exynos_drm_postclose(struct drm_device *dev, struct drm_file *file)
> 195 {
> 196 if (!file->driver_priv)
> 197 return;
> 198
> 199 kfree(file->driver_priv);
> 200 file->driver_priv = NULL;
> 201 }
The function is different in the current code. I glanced through
drm_open_helper() and I don't see that file->driver_priv to NULL is
needed anyway...
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-01-21 13:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-21 6:57 [patch] drm/exynos: potential use after free in exynos_drm_open() Dan Carpenter
2014-01-21 7:12 ` Jingoo Han
2014-01-21 7:12 ` Inki Dae
2014-01-21 12:37 ` walter harms
2014-01-21 12:43 ` walter harms
2014-01-21 13:35 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).