2023-02-23 07:05:12

by tangmeng

[permalink] [raw]
Subject: [PATCH v2] drm/vmwgfx: Work around VMW_ALLOC_DMABUF

A privilege escalation vulnerability was found in vmwgfx driver
in drivers/gpu/drm/vmwgfx/vmwgfx_drv.c in GPU component of Linux
kernel with device file '/dev/dri/renderD128 (or Dxxx)'. This flaw
allows a local attacker with a user account on the system to gain
privilege, causing a denial of service(DoS).

This vulnerability can be quickly verified by the following code
logic:
...
dri_fd = open("/dev/dri/renderD128", O_RDWR);
ret = ioctl(dri_fd, 0xC0186441, &arg);
if (ret == 0) {
printf("[*] VMW_ALLOC_DMABUF Success!\n");
}
...

Submit this commit to fix it.

Signed-off-by: Meng Tang <[email protected]>
---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index bd02cb0e6837..115787697957 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -1263,6 +1263,10 @@ static long vmw_generic_ioctl(struct file *filp, unsigned int cmd,
if (!drm_is_current_master(file_priv) &&
!capable(CAP_SYS_ADMIN))
return -EACCES;
+ } else if (nr == DRM_COMMAND_BASE + DRM_VMW_ALLOC_DMABUF) {
+ if (!drm_is_current_master(file_priv) &&
+ !capable(CAP_SYS_ADMIN))
+ return -EPERM;
}

if (unlikely(ioctl->cmd != cmd))
--
2.20.1



2023-02-23 12:50:48

by Zack Rusin

[permalink] [raw]
Subject: Re: [PATCH v2] drm/vmwgfx: Work around VMW_ALLOC_DMABUF

On Thu, 2023-02-23 at 15:04 +0800, Meng Tang wrote:
> A privilege escalation vulnerability was found in vmwgfx driver
> in drivers/gpu/drm/vmwgfx/vmwgfx_drv.c in GPU component of Linux
> kernel with device file '/dev/dri/renderD128 (or Dxxx)'. This flaw
> allows a local attacker with a user account on the system to gain
> privilege, causing a denial of service(DoS).
>
> This vulnerability can be quickly verified by the following code
> logic:
> ...
> dri_fd = open("/dev/dri/renderD128", O_RDWR);
> ret = ioctl(dri_fd, 0xC0186441, &arg);
> if (ret == 0) {
>         printf("[*] VMW_ALLOC_DMABUF Success!\n");
> }
> ...

This is just regular usage of that ioctl. What's the vulnerability?

>
> Submit this commit to fix it.

No, this is incorrect. You're effectively just disabling the driver for normal
apps/users using OpenGL or any accelerated contexts, which is going to completely
break, well, essentially everything this driver is for. Being able to use ioctl's
that were meant to be used is not a bug.

If you have a proof of concept or at least a description of the vulnerability that
you've found I'd be happy to take a look at it.

z

2023-02-24 02:47:41

by tangmeng

[permalink] [raw]
Subject: Re: [PATCH v2] drm/vmwgfx: Work around VMW_ALLOC_DMABUF

On 2023/2/23 20:50, Zack Rusin wrote:
> On Thu, 2023-02-23 at 15:04 +0800, Meng Tang wrote:
>> A privilege escalation vulnerability was found in vmwgfx driver
>> in drivers/gpu/drm/vmwgfx/vmwgfx_drv.c in GPU component of Linux
>> kernel with device file '/dev/dri/renderD128 (or Dxxx)'. This flaw
>> allows a local attacker with a user account on the system to gain
>> privilege, causing a denial of service(DoS).
>>
>> This vulnerability can be quickly verified by the following code
>> logic:
>> ...
>> dri_fd = open("/dev/dri/renderD128", O_RDWR);
>> ret = ioctl(dri_fd, 0xC0186441, &arg);
>> if (ret == 0) {
>>         printf("[*] VMW_ALLOC_DMABUF Success!\n");
>> }
>> ...
>
> This is just regular usage of that ioctl. What's the vulnerability?
>
Yes, this is a very common regular usage of that ioctl.
But if any user can operate /dev/dri/renderD128 through ioctl, it will
lead to a vulnerability.
>>
>> Submit this commit to fix it.
>
> No, this is incorrect. You're effectively just disabling the driver for normal
> apps/users using OpenGL or any accelerated contexts, which is going to completely
> break, well, essentially everything this driver is for. Being able to use ioctl's
> that were meant to be used is not a bug.
>
> If you have a proof of concept or at least a description of the vulnerability that
> you've found I'd be happy to take a look at it.
>
> z


$ ls /dev/dri/renderD128 -la
crw-rw----+ 1 root render 226, 128 2? 15 11:45 /dev/dri/renderD128

The permission of the file is ”crw-rw----+”.
I think only the root user or users with certain privileges can access
the /dev/dri/renderD128 device file at this time.

But in fact, users can access /dev/dri/renderD128 through ioctl without
permission.

Attachment poc.c is a test case, any user can execute the
case(VMW_ALLOC_DMABUF) successfully, and eventually lead to a call
trace(log see attachment dmesg.txt).

This will cause the user with permission VMW_ALLOC_DMABUF failed.

Therefore, I think that according to the permissions shown by
“crw-rw----+ 1 root render”, only the root user or users with certain
privileges can VMW_ALLOC_DMABUF success.


Attachments:
poc.c (0.98 kB)
dmesg.txt (2.88 kB)
Download all attachments

2023-02-24 03:13:21

by Zack Rusin

[permalink] [raw]
Subject: Re: [PATCH v2] drm/vmwgfx: Work around VMW_ALLOC_DMABUF

On Fri, 2023-02-24 at 10:46 +0800, Meng Tang wrote:
> On 2023/2/23 20:50, Zack Rusin wrote:
> > On Thu, 2023-02-23 at 15:04 +0800, Meng Tang wrote:
> > > A privilege escalation vulnerability was found in vmwgfx driver
> > > in drivers/gpu/drm/vmwgfx/vmwgfx_drv.c in GPU component of Linux
> > > kernel with device file '/dev/dri/renderD128 (or Dxxx)'. This flaw
> > > allows a local attacker with a user account on the system to gain
> > > privilege, causing a denial of service(DoS).
> > >
> > > This vulnerability can be quickly verified by the following code
> > > logic:
> > > ...
> > > dri_fd = open("/dev/dri/renderD128", O_RDWR);
> > > ret = ioctl(dri_fd, 0xC0186441, &arg);
> > > if (ret == 0) {
> > >          printf("[*] VMW_ALLOC_DMABUF Success!\n");
> > > }
> > > ...
> >
> > This is just regular usage of that ioctl. What's the vulnerability?
> >
> Yes, this is a very common regular usage of that ioctl.
> But if any user can operate /dev/dri/renderD128 through ioctl, it will
> lead to a vulnerability.
> > > Submit this commit to fix it.
> >
> > No, this is incorrect. You're effectively just disabling the driver for normal
> > apps/users using OpenGL or any accelerated contexts, which is going to
> > completely
> > break, well, essentially everything this driver is for. Being able to use
> > ioctl's
> > that were meant to be used is not a bug.
> >
> > If you have a proof of concept or at least a description of the vulnerability
> > that
> > you've found I'd be happy to take a look at it.
> >
> > z
>
>
> $ ls /dev/dri/renderD128 -la
> crw-rw----+ 1 root render 226, 128 2?  15 11:45 /dev/dri/renderD128
>
> The permission of the file is ”crw-rw----+”.
> I think only the root user or users with certain privileges can access
> the /dev/dri/renderD128 device file at this time.
>
> But in fact, users can access /dev/dri/renderD128 through ioctl without
> permission.
>
> Attachment poc.c is a test case, any user can execute the
> case(VMW_ALLOC_DMABUF) successfully, and eventually lead to a call
> trace(log see attachment dmesg.txt).
>
> This will cause the user with permission VMW_ALLOC_DMABUF failed.

That's correct. That's the way this works. The ioctl is allocating a buffer, there's
no infinite space for buffers on a system and, given that your app just allocates
and never frees buffers, at some point the space will run out and the ioctl will
return a failure. 

As to the stack trace, I'm not sure what kernel you were testing it on so I don't
have access to the full log but I can't reproduce it and there was a change fixing
exactly this (i.e. buffer failed allocation but we were still accessing it) that was
fixed in in 6.2 in commit 1a6897921f52 ("drm/vmwgfx: Stop accessing buffer objects
which failed init") the change was backported as well, so you should be able to
verify on any kernel with it.

z

2023-02-24 03:30:32

by tangmeng

[permalink] [raw]
Subject: Re: [PATCH v2] drm/vmwgfx: Work around VMW_ALLOC_DMABUF



On 2023/2/24 11:13, Zack Rusin wrote:
>
> That's correct. That's the way this works. The ioctl is allocating a buffer, there's
> no infinite space for buffers on a system and, given that your app just allocates
> and never frees buffers, at some point the space will run out and the ioctl will
> return a failure.
>
Do you mean that users without certain privileges can access allocate a
buffer because it is designed like this? so we don't need to block
users without certain privileges to VMW_ALLOC_DMABUF success?
> As to the stack trace, I'm not sure what kernel you were testing it on so I don't
> have access to the full log but I can't reproduce it and there was a change fixing
> exactly this (i.e. buffer failed allocation but we were still accessing it) that was
> fixed in in 6.2 in commit 1a6897921f52 ("drm/vmwgfx: Stop accessing buffer objects
> which failed init") the change was backported as well, so you should be able to
> verify on any kernel with it.
>
> z
>
Thank you, the kernel version of my environment is lower than 6.2, I
will verify on my kernel with commit 1a6897921f52 ("drm/vmwgfx: Stop
accessing buffer objects which failed init").


2023-02-24 03:38:16

by Zack Rusin

[permalink] [raw]
Subject: Re: [PATCH v2] drm/vmwgfx: Work around VMW_ALLOC_DMABUF

On Fri, 2023-02-24 at 11:29 +0800, Meng Tang wrote:
>
>
> On 2023/2/24 11:13, Zack Rusin wrote:
> >
> > That's correct. That's the way this works. The ioctl is allocating a buffer,
> > there's
> > no infinite space for buffers on a system and, given that your app just
> > allocates
> > and never frees buffers, at some point the space will run out and the ioctl will
> > return a failure.
> >
> Do you mean that users without certain privileges can access allocate a
> buffer because it is designed like this? so we don't need to block
> users without certain privileges to VMW_ALLOC_DMABUF success?

That's correct. If only the drm master or admins could use rendering none of the
regular accelerated (e.g. OpenGL) apps would work.

> > As to the stack trace, I'm not sure what kernel you were testing it on so I
> > don't
> > have access to the full log but I can't reproduce it and there was a change
> > fixing
> > exactly this (i.e. buffer failed allocation but we were still accessing it) that
> > was
> > fixed in in 6.2 in commit 1a6897921f52 ("drm/vmwgfx: Stop accessing buffer
> > objects
> > which failed init") the change was backported as well, so you should be able to
> > verify on any kernel with it.
> >
> > z
> >
> Thank you, the kernel version of my environment is lower than 6.2, I
> will verify on my kernel with commit 1a6897921f52 ("drm/vmwgfx: Stop
> accessing buffer objects which failed init").

Great. Let me know if you have any problems with it.

z