* [PATCH v2] drm/vgem: fix use-after-free when drm_gem_handle_create() fails
[not found] <20190226213053.GC218103@gmail.com>
@ 2019-02-26 21:44 ` Eric Biggers
2019-02-27 23:52 ` Laura Abbott
2019-03-04 23:24 ` Rodrigo Siqueira
2019-02-26 22:08 ` [PATCH] drm/vkms: " Eric Biggers
1 sibling, 2 replies; 10+ messages in thread
From: Eric Biggers @ 2019-02-26 21:44 UTC (permalink / raw)
To: dri-devel
Cc: Chris Wilson, syzkaller-bugs, linux-kernel,
syzbot+e73f2fb5ed5a5df36d33, Laura Abbott, Daniel Vetter, stable
From: Eric Biggers <ebiggers@google.com>
If drm_gem_handle_create() fails in vgem_gem_create(), then the
drm_vgem_gem_object is freed twice: once when the reference is dropped
by drm_gem_object_put_unlocked(), and again by __vgem_gem_destroy().
This was hit by syzkaller using fault injection.
Fix it by skipping the second free.
Reported-by: syzbot+e73f2fb5ed5a5df36d33@syzkaller.appspotmail.com
Fixes: af33a9190d02 ("drm/vgem: Enable dmabuf import interfaces")
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Laura Abbott <labbott@redhat.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
drivers/gpu/drm/vgem/vgem_drv.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index 5930facd6d2d8..11a8f99ba18c5 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -191,13 +191,9 @@ static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
ret = drm_gem_handle_create(file, &obj->base, handle);
drm_gem_object_put_unlocked(&obj->base);
if (ret)
- goto err;
+ return ERR_PTR(ret);
return &obj->base;
-
-err:
- __vgem_gem_destroy(obj);
- return ERR_PTR(ret);
}
static int vgem_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
--
2.21.0.rc2.261.ga7da99ff1b-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] drm/vkms: fix use-after-free when drm_gem_handle_create() fails
[not found] <20190226213053.GC218103@gmail.com>
2019-02-26 21:44 ` [PATCH v2] drm/vgem: fix use-after-free when drm_gem_handle_create() fails Eric Biggers
@ 2019-02-26 22:08 ` Eric Biggers
2019-02-26 22:14 ` Chris Wilson
2019-02-27 23:12 ` Rodrigo Siqueira
1 sibling, 2 replies; 10+ messages in thread
From: Eric Biggers @ 2019-02-26 22:08 UTC (permalink / raw)
To: dri-devel
Cc: syzkaller-bugs, linux-kernel, Rodrigo Siqueira, Haneen Mohammed,
Daniel Vetter, Chris Wilson, stable
From: Eric Biggers <ebiggers@google.com>
If drm_gem_handle_create() fails in vkms_gem_create(), then the
vkms_gem_object is freed twice: once when the reference is dropped by
drm_gem_object_put_unlocked(), and again by the extra calls to
drm_gem_object_release() and kfree().
Fix it by skipping the second release and free.
This bug was originally found in the vgem driver by syzkaller using
fault injection, but I noticed it's also present in the vkms driver.
Fixes: 559e50fd34d1 ("drm/vkms: Add dumb operations")
Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
drivers/gpu/drm/vkms/vkms_gem.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c
index 138b0bb325cf9..69048e73377dc 100644
--- a/drivers/gpu/drm/vkms/vkms_gem.c
+++ b/drivers/gpu/drm/vkms/vkms_gem.c
@@ -111,11 +111,8 @@ struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
ret = drm_gem_handle_create(file, &obj->gem, handle);
drm_gem_object_put_unlocked(&obj->gem);
- if (ret) {
- drm_gem_object_release(&obj->gem);
- kfree(obj);
+ if (ret)
return ERR_PTR(ret);
- }
return &obj->gem;
}
--
2.21.0.rc2.261.ga7da99ff1b-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/vkms: fix use-after-free when drm_gem_handle_create() fails
2019-02-26 22:08 ` [PATCH] drm/vkms: " Eric Biggers
@ 2019-02-26 22:14 ` Chris Wilson
2019-02-27 23:12 ` Rodrigo Siqueira
1 sibling, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2019-02-26 22:14 UTC (permalink / raw)
To: Eric Biggers, dri-devel
Cc: syzkaller-bugs, linux-kernel, Rodrigo Siqueira, Haneen Mohammed,
Daniel Vetter, stable
Quoting Eric Biggers (2019-02-26 22:08:58)
> From: Eric Biggers <ebiggers@google.com>
>
> If drm_gem_handle_create() fails in vkms_gem_create(), then the
> vkms_gem_object is freed twice: once when the reference is dropped by
> drm_gem_object_put_unlocked(), and again by the extra calls to
> drm_gem_object_release() and kfree().
>
> Fix it by skipping the second release and free.
>
> This bug was originally found in the vgem driver by syzkaller using
> fault injection, but I noticed it's also present in the vkms driver.
>
> Fixes: 559e50fd34d1 ("drm/vkms: Add dumb operations")
> Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/vkms: fix use-after-free when drm_gem_handle_create() fails
2019-02-26 22:08 ` [PATCH] drm/vkms: " Eric Biggers
2019-02-26 22:14 ` Chris Wilson
@ 2019-02-27 23:12 ` Rodrigo Siqueira
2019-02-28 6:41 ` Dmitry Vyukov
1 sibling, 1 reply; 10+ messages in thread
From: Rodrigo Siqueira @ 2019-02-27 23:12 UTC (permalink / raw)
To: Eric Biggers
Cc: dri-devel, syzkaller-bugs, linux-kernel, Haneen Mohammed,
Daniel Vetter, Chris Wilson, stable
[-- Attachment #1: Type: text/plain, Size: 1920 bytes --]
On 02/26, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> If drm_gem_handle_create() fails in vkms_gem_create(), then the
> vkms_gem_object is freed twice: once when the reference is dropped by
> drm_gem_object_put_unlocked(), and again by the extra calls to
> drm_gem_object_release() and kfree().
>
> Fix it by skipping the second release and free.
>
> This bug was originally found in the vgem driver by syzkaller using
> fault injection, but I noticed it's also present in the vkms driver.
>
> Fixes: 559e50fd34d1 ("drm/vkms: Add dumb operations")
> Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> drivers/gpu/drm/vkms/vkms_gem.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c
> index 138b0bb325cf9..69048e73377dc 100644
> --- a/drivers/gpu/drm/vkms/vkms_gem.c
> +++ b/drivers/gpu/drm/vkms/vkms_gem.c
> @@ -111,11 +111,8 @@ struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
>
> ret = drm_gem_handle_create(file, &obj->gem, handle);
> drm_gem_object_put_unlocked(&obj->gem);
> - if (ret) {
> - drm_gem_object_release(&obj->gem);
> - kfree(obj);
> + if (ret)
> return ERR_PTR(ret);
> - }
>
> return &obj->gem;
> }
> --
> 2.21.0.rc2.261.ga7da99ff1b-goog
>
Hi,
Thanks for your patch! :)
The patch looks good for me. I also tested it under the IGT tests on my
local VM and everything was fine.
Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
--
Rodrigo Siqueira
https://siqueira.tech
Graduate Student
Department of Computer Science
University of São Paulo
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] drm/vgem: fix use-after-free when drm_gem_handle_create() fails
2019-02-26 21:44 ` [PATCH v2] drm/vgem: fix use-after-free when drm_gem_handle_create() fails Eric Biggers
@ 2019-02-27 23:52 ` Laura Abbott
2019-03-04 23:24 ` Rodrigo Siqueira
1 sibling, 0 replies; 10+ messages in thread
From: Laura Abbott @ 2019-02-27 23:52 UTC (permalink / raw)
To: Eric Biggers, dri-devel
Cc: Chris Wilson, syzkaller-bugs, linux-kernel,
syzbot+e73f2fb5ed5a5df36d33, Daniel Vetter, stable
On 2/26/19 1:44 PM, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> If drm_gem_handle_create() fails in vgem_gem_create(), then the
> drm_vgem_gem_object is freed twice: once when the reference is dropped
> by drm_gem_object_put_unlocked(), and again by __vgem_gem_destroy().
>
> This was hit by syzkaller using fault injection.
>
> Fix it by skipping the second free.
>
> Reported-by: syzbot+e73f2fb5ed5a5df36d33@syzkaller.appspotmail.com
> Fixes: af33a9190d02 ("drm/vgem: Enable dmabuf import interfaces")
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Laura Abbott <labbott@redhat.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> drivers/gpu/drm/vgem/vgem_drv.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
> index 5930facd6d2d8..11a8f99ba18c5 100644
> --- a/drivers/gpu/drm/vgem/vgem_drv.c
> +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> @@ -191,13 +191,9 @@ static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
> ret = drm_gem_handle_create(file, &obj->base, handle);
> drm_gem_object_put_unlocked(&obj->base);
> if (ret)
> - goto err;
> + return ERR_PTR(ret);
>
> return &obj->base;
> -
> -err:
> - __vgem_gem_destroy(obj);
> - return ERR_PTR(ret);
> }
>
> static int vgem_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
>
Acked-by: Laura Abbott <labbott@redhat.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/vkms: fix use-after-free when drm_gem_handle_create() fails
2019-02-27 23:12 ` Rodrigo Siqueira
@ 2019-02-28 6:41 ` Dmitry Vyukov
2019-03-04 23:23 ` Rodrigo Siqueira
0 siblings, 1 reply; 10+ messages in thread
From: Dmitry Vyukov @ 2019-02-28 6:41 UTC (permalink / raw)
To: Rodrigo Siqueira
Cc: Eric Biggers, DRI, syzkaller-bugs, LKML, Haneen Mohammed,
Daniel Vetter, Chris Wilson, stable
On Thu, Feb 28, 2019 at 12:12 AM Rodrigo Siqueira
<rodrigosiqueiramelo@gmail.com> wrote:
>
> On 02/26, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> >
> > If drm_gem_handle_create() fails in vkms_gem_create(), then the
> > vkms_gem_object is freed twice: once when the reference is dropped by
> > drm_gem_object_put_unlocked(), and again by the extra calls to
> > drm_gem_object_release() and kfree().
> >
> > Fix it by skipping the second release and free.
> >
> > This bug was originally found in the vgem driver by syzkaller using
> > fault injection, but I noticed it's also present in the vkms driver.
> >
> > Fixes: 559e50fd34d1 ("drm/vkms: Add dumb operations")
> > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > ---
> > drivers/gpu/drm/vkms/vkms_gem.c | 5 +----
> > 1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c
> > index 138b0bb325cf9..69048e73377dc 100644
> > --- a/drivers/gpu/drm/vkms/vkms_gem.c
> > +++ b/drivers/gpu/drm/vkms/vkms_gem.c
> > @@ -111,11 +111,8 @@ struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
> >
> > ret = drm_gem_handle_create(file, &obj->gem, handle);
> > drm_gem_object_put_unlocked(&obj->gem);
> > - if (ret) {
> > - drm_gem_object_release(&obj->gem);
> > - kfree(obj);
> > + if (ret)
> > return ERR_PTR(ret);
> > - }
> >
> > return &obj->gem;
> > }
> > --
> > 2.21.0.rc2.261.ga7da99ff1b-goog
> >
>
> Hi,
>
> Thanks for your patch! :)
>
> The patch looks good for me. I also tested it under the IGT tests on my
> local VM and everything was fine.
Hi Rodrigo,
What are IGT tests? How can I run them?
>
> Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
>
> --
> Rodrigo Siqueira
> https://siqueira.tech
> Graduate Student
> Department of Computer Science
> University of São Paulo
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20190227231202.tycdbcqtk5ylwp4k%40smtp.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/vkms: fix use-after-free when drm_gem_handle_create() fails
2019-02-28 6:41 ` Dmitry Vyukov
@ 2019-03-04 23:23 ` Rodrigo Siqueira
2019-03-05 14:25 ` Dmitry Vyukov
0 siblings, 1 reply; 10+ messages in thread
From: Rodrigo Siqueira @ 2019-03-04 23:23 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: Eric Biggers, DRI, syzkaller-bugs, LKML, Haneen Mohammed,
Daniel Vetter, Chris Wilson, stable
[-- Attachment #1: Type: text/plain, Size: 3470 bytes --]
On 02/28, Dmitry Vyukov wrote:
> On Thu, Feb 28, 2019 at 12:12 AM Rodrigo Siqueira
> <rodrigosiqueiramelo@gmail.com> wrote:
> >
> > On 02/26, Eric Biggers wrote:
> > > From: Eric Biggers <ebiggers@google.com>
> > >
> > > If drm_gem_handle_create() fails in vkms_gem_create(), then the
> > > vkms_gem_object is freed twice: once when the reference is dropped by
> > > drm_gem_object_put_unlocked(), and again by the extra calls to
> > > drm_gem_object_release() and kfree().
> > >
> > > Fix it by skipping the second release and free.
> > >
> > > This bug was originally found in the vgem driver by syzkaller using
> > > fault injection, but I noticed it's also present in the vkms driver.
> > >
> > > Fixes: 559e50fd34d1 ("drm/vkms: Add dumb operations")
> > > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > > Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > > ---
> > > drivers/gpu/drm/vkms/vkms_gem.c | 5 +----
> > > 1 file changed, 1 insertion(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c
> > > index 138b0bb325cf9..69048e73377dc 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_gem.c
> > > +++ b/drivers/gpu/drm/vkms/vkms_gem.c
> > > @@ -111,11 +111,8 @@ struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
> > >
> > > ret = drm_gem_handle_create(file, &obj->gem, handle);
> > > drm_gem_object_put_unlocked(&obj->gem);
> > > - if (ret) {
> > > - drm_gem_object_release(&obj->gem);
> > > - kfree(obj);
> > > + if (ret)
> > > return ERR_PTR(ret);
> > > - }
> > >
> > > return &obj->gem;
> > > }
> > > --
> > > 2.21.0.rc2.261.ga7da99ff1b-goog
> > >
> >
> > Hi,
> >
> > Thanks for your patch! :)
> >
> > The patch looks good for me. I also tested it under the IGT tests on my
> > local VM and everything was fine.
Hi,
Patch applied to drm-misc-fixes.
> Hi Rodrigo,
>
> What are IGT tests? How can I run them?
Hi Dmitry,
IGT is a test suite focused on DRM drivers.
You can clone the project using the link below:
https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
In the README, you will find the software dependencies. After you
install all the required package, just use:
mkdir build && meson build && cd build && ninja
Finally, if you want to test VKMS, I recommend you to do it inside a VM.
Best Regards
Rodrigo Siqueira
> >
> > Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> >
> > --
> > Rodrigo Siqueira
> > https://siqueira.tech
> > Graduate Student
> > Department of Computer Science
> > University of São Paulo
> >
> > --
> > You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20190227231202.tycdbcqtk5ylwp4k%40smtp.gmail.com.
> > For more options, visit https://groups.google.com/d/optout.
--
Rodrigo Siqueira
https://siqueira.tech
Graduate Student
Department of Computer Science
University of São Paulo
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] drm/vgem: fix use-after-free when drm_gem_handle_create() fails
2019-02-26 21:44 ` [PATCH v2] drm/vgem: fix use-after-free when drm_gem_handle_create() fails Eric Biggers
2019-02-27 23:52 ` Laura Abbott
@ 2019-03-04 23:24 ` Rodrigo Siqueira
1 sibling, 0 replies; 10+ messages in thread
From: Rodrigo Siqueira @ 2019-03-04 23:24 UTC (permalink / raw)
To: Eric Biggers
Cc: dri-devel, Daniel Vetter, syzkaller-bugs, linux-kernel, stable,
syzbot+e73f2fb5ed5a5df36d33
[-- Attachment #1: Type: text/plain, Size: 1909 bytes --]
On 02/26, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> If drm_gem_handle_create() fails in vgem_gem_create(), then the
> drm_vgem_gem_object is freed twice: once when the reference is dropped
> by drm_gem_object_put_unlocked(), and again by __vgem_gem_destroy().
>
> This was hit by syzkaller using fault injection.
>
> Fix it by skipping the second free.
>
> Reported-by: syzbot+e73f2fb5ed5a5df36d33@syzkaller.appspotmail.com
> Fixes: af33a9190d02 ("drm/vgem: Enable dmabuf import interfaces")
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Laura Abbott <labbott@redhat.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> drivers/gpu/drm/vgem/vgem_drv.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
> index 5930facd6d2d8..11a8f99ba18c5 100644
> --- a/drivers/gpu/drm/vgem/vgem_drv.c
> +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> @@ -191,13 +191,9 @@ static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
> ret = drm_gem_handle_create(file, &obj->base, handle);
> drm_gem_object_put_unlocked(&obj->base);
> if (ret)
> - goto err;
> + return ERR_PTR(ret);
>
> return &obj->base;
> -
> -err:
> - __vgem_gem_destroy(obj);
> - return ERR_PTR(ret);
> }
>
> static int vgem_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
> --
> 2.21.0.rc2.261.ga7da99ff1b-goog
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Applied to drm-misc-fixes.
Thanks
--
Rodrigo Siqueira
https://siqueira.tech
Graduate Student
Department of Computer Science
University of São Paulo
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/vkms: fix use-after-free when drm_gem_handle_create() fails
2019-03-04 23:23 ` Rodrigo Siqueira
@ 2019-03-05 14:25 ` Dmitry Vyukov
2019-03-10 15:36 ` Rodrigo Siqueira
0 siblings, 1 reply; 10+ messages in thread
From: Dmitry Vyukov @ 2019-03-05 14:25 UTC (permalink / raw)
To: Rodrigo Siqueira
Cc: Eric Biggers, DRI, syzkaller-bugs, LKML, Haneen Mohammed,
Daniel Vetter, Chris Wilson, stable
On Tue, Mar 5, 2019 at 12:23 AM Rodrigo Siqueira
<rodrigosiqueiramelo@gmail.com> wrote:
>
> On 02/28, Dmitry Vyukov wrote:
> > On Thu, Feb 28, 2019 at 12:12 AM Rodrigo Siqueira
> > <rodrigosiqueiramelo@gmail.com> wrote:
> > >
> > > On 02/26, Eric Biggers wrote:
> > > > From: Eric Biggers <ebiggers@google.com>
> > > >
> > > > If drm_gem_handle_create() fails in vkms_gem_create(), then the
> > > > vkms_gem_object is freed twice: once when the reference is dropped by
> > > > drm_gem_object_put_unlocked(), and again by the extra calls to
> > > > drm_gem_object_release() and kfree().
> > > >
> > > > Fix it by skipping the second release and free.
> > > >
> > > > This bug was originally found in the vgem driver by syzkaller using
> > > > fault injection, but I noticed it's also present in the vkms driver.
> > > >
> > > > Fixes: 559e50fd34d1 ("drm/vkms: Add dumb operations")
> > > > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > > > Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
> > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > > > ---
> > > > drivers/gpu/drm/vkms/vkms_gem.c | 5 +----
> > > > 1 file changed, 1 insertion(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c
> > > > index 138b0bb325cf9..69048e73377dc 100644
> > > > --- a/drivers/gpu/drm/vkms/vkms_gem.c
> > > > +++ b/drivers/gpu/drm/vkms/vkms_gem.c
> > > > @@ -111,11 +111,8 @@ struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
> > > >
> > > > ret = drm_gem_handle_create(file, &obj->gem, handle);
> > > > drm_gem_object_put_unlocked(&obj->gem);
> > > > - if (ret) {
> > > > - drm_gem_object_release(&obj->gem);
> > > > - kfree(obj);
> > > > + if (ret)
> > > > return ERR_PTR(ret);
> > > > - }
> > > >
> > > > return &obj->gem;
> > > > }
> > > > --
> > > > 2.21.0.rc2.261.ga7da99ff1b-goog
> > > >
> > >
> > > Hi,
> > >
> > > Thanks for your patch! :)
> > >
> > > The patch looks good for me. I also tested it under the IGT tests on my
> > > local VM and everything was fine.
>
> Hi,
>
> Patch applied to drm-misc-fixes.
>
> > Hi Rodrigo,
> >
> > What are IGT tests? How can I run them?
>
> Hi Dmitry,
>
> IGT is a test suite focused on DRM drivers.
>
> You can clone the project using the link below:
>
> https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
>
> In the README, you will find the software dependencies. After you
> install all the required package, just use:
>
> mkdir build && meson build && cd build && ninja
Hi Rodrigo,
Thanks for the info, but this did not work for me.
I installed all recommended packages (including libdw-dev), but then got:
igt-gpu-tools$ mkdir -p build && meson build && cd build && ninja
The Meson build system
Version: 0.46.1
Source dir: /src/igt-gpu-tools
Build dir: /src/igt-gpu-tools/build
Build type: native build
Project name: igt-gpu-tools
Native C compiler: ccache cc (gcc 7.3.0 "cc (Debian 7.3.0-5) 7.3.0")
Build machine cpu family: x86_64
Build machine cpu: x86_64
Compiler for C supports arguments -Wbad-function-cast: YES
Compiler for C supports arguments -Wdeclaration-after-statement: YES
Compiler for C supports arguments -Wformat=2: YES
Compiler for C supports arguments -Wimplicit-fallthrough=0: YES
Compiler for C supports arguments -Wlogical-op: YES
Compiler for C supports arguments -Wmissing-declarations: YES
Compiler for C supports arguments -Wmissing-format-attribute: YES
Compiler for C supports arguments -Wmissing-noreturn: YES
Compiler for C supports arguments -Wmissing-prototypes: YES
Compiler for C supports arguments -Wnested-externs: YES
Compiler for C supports arguments -Wold-style-definition: YES
Compiler for C supports arguments -Wpointer-arith: YES
Compiler for C supports arguments -Wredundant-decls: YES
Compiler for C supports arguments -Wshadow: YES
Compiler for C supports arguments -Wstrict-prototypes: YES
Compiler for C supports arguments -Wuninitialized: YES
Compiler for C supports arguments -Wunused: YES
Compiler for C supports arguments -Wno-clobbered -Wclobbered: YES
Compiler for C supports arguments -Wno-maybe-uninitialized
-Wmaybe-uninitialized: YES
Compiler for C supports arguments -Wno-missing-field-initializers
-Wmissing-field-initializers: YES
Compiler for C supports arguments -Wno-pointer-arith -Wpointer-arith: YES
Compiler for C supports arguments -Wno-sign-compare -Wsign-compare: YES
Compiler for C supports arguments -Wno-type-limits -Wtype-limits: YES
Compiler for C supports arguments -Wno-unused-parameter -Wunused-parameter: YES
Compiler for C supports arguments -Wno-unused-result -Wunused-result: YES
Compiler for C supports arguments -Werror=address: YES
Compiler for C supports arguments -Werror=array-bounds: YES
Compiler for C supports arguments -Werror=implicit: YES
Compiler for C supports arguments -Werror=init-self: YES
Compiler for C supports arguments -Werror=int-to-pointer-cast: YES
Compiler for C supports arguments -Werror=main: YES
Compiler for C supports arguments -Werror=missing-braces: YES
Compiler for C supports arguments -Werror=nonnull: YES
Compiler for C supports arguments -Werror=pointer-to-int-cast: YES
Compiler for C supports arguments -Werror=return-type: YES
Compiler for C supports arguments -Werror=sequence-point: YES
Compiler for C supports arguments -Werror=trigraphs: YES
Compiler for C supports arguments -Werror=write-strings: YES
Found pkg-config: /usr/bin/pkg-config (0.29)
Native dependency libdrm found: YES 2.4.91
Native dependency libdrm_intel found: YES 2.4.91
Native dependency libdrm_nouveau found: YES 2.4.91
Native dependency libdrm_amdgpu found: YES 2.4.91
Native dependency pciaccess found: YES 0.13.4
Native dependency libkmod found: YES 24
Native dependency libprocps found: YES 3.3.15
Native dependency libunwind found: YES 1.21
meson.build:151:0: ERROR: Could not generate cargs for libdw:
A full log can be found at /src/igt-gpu-tools/build/meson-logs/meson-log.txt
and meson-log.txt ends with:
Compiler for C supports arguments -Werror=write-strings: YES
Found pkg-config: /usr/bin/pkg-config (0.29)
Determining dependency 'libdrm' with pkg-config executable '/usr/bin/pkg-config'
Native dependency libdrm found: YES 2.4.91
Determining dependency 'libdrm_intel' with pkg-config executable
'/usr/bin/pkg-config'
Native dependency libdrm_intel found: YES 2.4.91
Determining dependency 'libdrm_nouveau' with pkg-config executable
'/usr/bin/pkg-config'
Native dependency libdrm_nouveau found: YES 2.4.91
Determining dependency 'libdrm_amdgpu' with pkg-config executable
'/usr/bin/pkg-config'
Native dependency libdrm_amdgpu found: YES 2.4.91
Determining dependency 'pciaccess' with pkg-config executable
'/usr/bin/pkg-config'
Native dependency pciaccess found: YES 0.13.4
Determining dependency 'libkmod' with pkg-config executable
'/usr/bin/pkg-config'
Native dependency libkmod found: YES 24
Determining dependency 'libprocps' with pkg-config executable
'/usr/bin/pkg-config'
Native dependency libprocps found: YES 3.3.15
Determining dependency 'libunwind' with pkg-config executable
'/usr/bin/pkg-config'
Native dependency libunwind found: YES 1.21
Determining dependency 'libdw' with pkg-config executable '/usr/bin/pkg-config'
meson.build:151:0: ERROR: Could not generate cargs for libdw:
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/vkms: fix use-after-free when drm_gem_handle_create() fails
2019-03-05 14:25 ` Dmitry Vyukov
@ 2019-03-10 15:36 ` Rodrigo Siqueira
0 siblings, 0 replies; 10+ messages in thread
From: Rodrigo Siqueira @ 2019-03-10 15:36 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: Eric Biggers, DRI, syzkaller-bugs, LKML, Haneen Mohammed,
Daniel Vetter, Chris Wilson, stable
[-- Attachment #1: Type: text/plain, Size: 8357 bytes --]
On 03/05, Dmitry Vyukov wrote:
> On Tue, Mar 5, 2019 at 12:23 AM Rodrigo Siqueira
> <rodrigosiqueiramelo@gmail.com> wrote:
> >
> > On 02/28, Dmitry Vyukov wrote:
> > > On Thu, Feb 28, 2019 at 12:12 AM Rodrigo Siqueira
> > > <rodrigosiqueiramelo@gmail.com> wrote:
> > > >
> > > > On 02/26, Eric Biggers wrote:
> > > > > From: Eric Biggers <ebiggers@google.com>
> > > > >
> > > > > If drm_gem_handle_create() fails in vkms_gem_create(), then the
> > > > > vkms_gem_object is freed twice: once when the reference is dropped by
> > > > > drm_gem_object_put_unlocked(), and again by the extra calls to
> > > > > drm_gem_object_release() and kfree().
> > > > >
> > > > > Fix it by skipping the second release and free.
> > > > >
> > > > > This bug was originally found in the vgem driver by syzkaller using
> > > > > fault injection, but I noticed it's also present in the vkms driver.
> > > > >
> > > > > Fixes: 559e50fd34d1 ("drm/vkms: Add dumb operations")
> > > > > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > > > > Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
> > > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > Cc: stable@vger.kernel.org
> > > > > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > > > > ---
> > > > > drivers/gpu/drm/vkms/vkms_gem.c | 5 +----
> > > > > 1 file changed, 1 insertion(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c
> > > > > index 138b0bb325cf9..69048e73377dc 100644
> > > > > --- a/drivers/gpu/drm/vkms/vkms_gem.c
> > > > > +++ b/drivers/gpu/drm/vkms/vkms_gem.c
> > > > > @@ -111,11 +111,8 @@ struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
> > > > >
> > > > > ret = drm_gem_handle_create(file, &obj->gem, handle);
> > > > > drm_gem_object_put_unlocked(&obj->gem);
> > > > > - if (ret) {
> > > > > - drm_gem_object_release(&obj->gem);
> > > > > - kfree(obj);
> > > > > + if (ret)
> > > > > return ERR_PTR(ret);
> > > > > - }
> > > > >
> > > > > return &obj->gem;
> > > > > }
> > > > > --
> > > > > 2.21.0.rc2.261.ga7da99ff1b-goog
> > > > >
> > > >
> > > > Hi,
> > > >
> > > > Thanks for your patch! :)
> > > >
> > > > The patch looks good for me. I also tested it under the IGT tests on my
> > > > local VM and everything was fine.
> >
> > Hi,
> >
> > Patch applied to drm-misc-fixes.
> >
> > > Hi Rodrigo,
> > >
> > > What are IGT tests? How can I run them?
> >
> > Hi Dmitry,
> >
> > IGT is a test suite focused on DRM drivers.
> >
> > You can clone the project using the link below:
> >
> > https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
> >
> > In the README, you will find the software dependencies. After you
> > install all the required package, just use:
> >
> > mkdir build && meson build && cd build && ninja
>
> Hi Rodrigo,
>
> Thanks for the info, but this did not work for me.
> I installed all recommended packages (including libdw-dev), but then got:
Hi Dmitry,
I would like to recommend you to join the dri-devel channel (Freenode).
There you can quickly get help from me and others ;)
My nick in the dri-devel is 'siqueira'.
Best Regards
> igt-gpu-tools$ mkdir -p build && meson build && cd build && ninja
> The Meson build system
> Version: 0.46.1
> Source dir: /src/igt-gpu-tools
> Build dir: /src/igt-gpu-tools/build
> Build type: native build
> Project name: igt-gpu-tools
> Native C compiler: ccache cc (gcc 7.3.0 "cc (Debian 7.3.0-5) 7.3.0")
> Build machine cpu family: x86_64
> Build machine cpu: x86_64
> Compiler for C supports arguments -Wbad-function-cast: YES
> Compiler for C supports arguments -Wdeclaration-after-statement: YES
> Compiler for C supports arguments -Wformat=2: YES
> Compiler for C supports arguments -Wimplicit-fallthrough=0: YES
> Compiler for C supports arguments -Wlogical-op: YES
> Compiler for C supports arguments -Wmissing-declarations: YES
> Compiler for C supports arguments -Wmissing-format-attribute: YES
> Compiler for C supports arguments -Wmissing-noreturn: YES
> Compiler for C supports arguments -Wmissing-prototypes: YES
> Compiler for C supports arguments -Wnested-externs: YES
> Compiler for C supports arguments -Wold-style-definition: YES
> Compiler for C supports arguments -Wpointer-arith: YES
> Compiler for C supports arguments -Wredundant-decls: YES
> Compiler for C supports arguments -Wshadow: YES
> Compiler for C supports arguments -Wstrict-prototypes: YES
> Compiler for C supports arguments -Wuninitialized: YES
> Compiler for C supports arguments -Wunused: YES
> Compiler for C supports arguments -Wno-clobbered -Wclobbered: YES
> Compiler for C supports arguments -Wno-maybe-uninitialized
> -Wmaybe-uninitialized: YES
> Compiler for C supports arguments -Wno-missing-field-initializers
> -Wmissing-field-initializers: YES
> Compiler for C supports arguments -Wno-pointer-arith -Wpointer-arith: YES
> Compiler for C supports arguments -Wno-sign-compare -Wsign-compare: YES
> Compiler for C supports arguments -Wno-type-limits -Wtype-limits: YES
> Compiler for C supports arguments -Wno-unused-parameter -Wunused-parameter: YES
> Compiler for C supports arguments -Wno-unused-result -Wunused-result: YES
> Compiler for C supports arguments -Werror=address: YES
> Compiler for C supports arguments -Werror=array-bounds: YES
> Compiler for C supports arguments -Werror=implicit: YES
> Compiler for C supports arguments -Werror=init-self: YES
> Compiler for C supports arguments -Werror=int-to-pointer-cast: YES
> Compiler for C supports arguments -Werror=main: YES
> Compiler for C supports arguments -Werror=missing-braces: YES
> Compiler for C supports arguments -Werror=nonnull: YES
> Compiler for C supports arguments -Werror=pointer-to-int-cast: YES
> Compiler for C supports arguments -Werror=return-type: YES
> Compiler for C supports arguments -Werror=sequence-point: YES
> Compiler for C supports arguments -Werror=trigraphs: YES
> Compiler for C supports arguments -Werror=write-strings: YES
> Found pkg-config: /usr/bin/pkg-config (0.29)
> Native dependency libdrm found: YES 2.4.91
> Native dependency libdrm_intel found: YES 2.4.91
> Native dependency libdrm_nouveau found: YES 2.4.91
> Native dependency libdrm_amdgpu found: YES 2.4.91
> Native dependency pciaccess found: YES 0.13.4
> Native dependency libkmod found: YES 24
> Native dependency libprocps found: YES 3.3.15
> Native dependency libunwind found: YES 1.21
>
> meson.build:151:0: ERROR: Could not generate cargs for libdw:
>
> A full log can be found at /src/igt-gpu-tools/build/meson-logs/meson-log.txt
>
>
> and meson-log.txt ends with:
>
> Compiler for C supports arguments -Werror=write-strings: YES
> Found pkg-config: /usr/bin/pkg-config (0.29)
> Determining dependency 'libdrm' with pkg-config executable '/usr/bin/pkg-config'
> Native dependency libdrm found: YES 2.4.91
> Determining dependency 'libdrm_intel' with pkg-config executable
> '/usr/bin/pkg-config'
> Native dependency libdrm_intel found: YES 2.4.91
> Determining dependency 'libdrm_nouveau' with pkg-config executable
> '/usr/bin/pkg-config'
> Native dependency libdrm_nouveau found: YES 2.4.91
> Determining dependency 'libdrm_amdgpu' with pkg-config executable
> '/usr/bin/pkg-config'
> Native dependency libdrm_amdgpu found: YES 2.4.91
> Determining dependency 'pciaccess' with pkg-config executable
> '/usr/bin/pkg-config'
> Native dependency pciaccess found: YES 0.13.4
> Determining dependency 'libkmod' with pkg-config executable
> '/usr/bin/pkg-config'
> Native dependency libkmod found: YES 24
> Determining dependency 'libprocps' with pkg-config executable
> '/usr/bin/pkg-config'
> Native dependency libprocps found: YES 3.3.15
> Determining dependency 'libunwind' with pkg-config executable
> '/usr/bin/pkg-config'
> Native dependency libunwind found: YES 1.21
> Determining dependency 'libdw' with pkg-config executable '/usr/bin/pkg-config'
>
> meson.build:151:0: ERROR: Could not generate cargs for libdw:
--
Rodrigo Siqueira
https://siqueira.tech
Graduate Student
Department of Computer Science
University of São Paulo
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-03-10 15:36 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20190226213053.GC218103@gmail.com>
2019-02-26 21:44 ` [PATCH v2] drm/vgem: fix use-after-free when drm_gem_handle_create() fails Eric Biggers
2019-02-27 23:52 ` Laura Abbott
2019-03-04 23:24 ` Rodrigo Siqueira
2019-02-26 22:08 ` [PATCH] drm/vkms: " Eric Biggers
2019-02-26 22:14 ` Chris Wilson
2019-02-27 23:12 ` Rodrigo Siqueira
2019-02-28 6:41 ` Dmitry Vyukov
2019-03-04 23:23 ` Rodrigo Siqueira
2019-03-05 14:25 ` Dmitry Vyukov
2019-03-10 15:36 ` Rodrigo Siqueira
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).