[PATCH v2] drm/vmwgfx: Work around VMW_ALLOC_DMABUF

Meng Tang posted 1 patch 3 years, 1 month ago
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 4 ++++
1 file changed, 4 insertions(+)
[PATCH v2] drm/vmwgfx: Work around VMW_ALLOC_DMABUF
Posted by Meng Tang 3 years, 1 month ago
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 <tangmeng@uniontech.com>
---
 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
Re: [PATCH v2] drm/vmwgfx: Work around VMW_ALLOC_DMABUF
Posted by Zack Rusin 3 years, 1 month ago
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

Re: [PATCH v2] drm/vmwgfx: Work around VMW_ALLOC_DMABUF
Posted by Meng Tang 3 years, 1 month ago
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.[58421.735593] ------------[ cut here ]------------
[58421.735595] kernel BUG at mm/slub.c:379!
[58421.735603] invalid opcode: 0000 [#2] PREEMPT SMP PTI
[58421.735607] CPU: 0 PID: 31670 Comm: poc Tainted: G
[58421.735611] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[58421.735613] RIP: 0010:kfree+0x278/0x2a0
[58421.735621] Code: ff 4c 8d 72 ff e9 26 fe ff ff 48 8b 55 c8 4d 89 f9 41 b8 01 00 00 00 4c 89 e1 4c 89 f6 4c 89 ef e8 bd fa ff ff e9 af fe ff ff <0f> 0b 0f 0b 80 3d 4d f1 93 01 00 75 a5 e9 f5 a9 94 00 48 8b 15 1f
[58421.735624] RSP: 0018:ffff9eeac63f7c70 EFLAGS: 00010246
[58421.735627] RAX: ffff93b5142ed400 RBX: ffff93b515182000 RCX: ffff93b5142ed600
[58421.735629] RDX: 00000000009c1c40 RSI: 0000000000000000 RDI: ffff93b501042b00
[58421.735631] RBP: ffff9eeac63f7ca8 R08: 0000000000000000 R09: c0000000ffffdfff
[58421.735633] R10: 0000000000000001 R11: ffff9eeac63f79c0 R12: ffff93b5142ed400
[58421.735635] R13: ffff93b501042b00 R14: ffffd6460050bb00 R15: ffffffffc0355911
[58421.735637] FS:  00007f7990a55700(0000) GS:ffff93b5bda00000(0000) knlGS:0000000000000000
[58421.735639] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[58421.735641] CR2: 00007f798c000010 CR3: 000000000e91a002 CR4: 00000000000706f0
[58421.735646] Call Trace:
[58421.735648]  <TASK>
[58421.735652]  vmw_bo_create+0x91/0xc0 [vmwgfx]
[58421.735664]  vmw_gem_object_create_with_handle+0x4c/0xc0 [vmwgfx]
[58421.735676]  ? _raw_spin_unlock_irqrestore+0x27/0x43
[58421.735683]  ? vmw_gem_object_create_with_handle+0xc0/0xc0 [vmwgfx]
[58421.735692]  vmw_gem_object_create_ioctl+0x3b/0x90 [vmwgfx]
[58421.735704]  drm_ioctl_kernel+0xba/0x150 [drm]
[58421.735769]  drm_ioctl+0x258/0x430 [drm]
[58421.735786]  ? vmw_gem_object_create_with_handle+0xc0/0xc0 [vmwgfx]
[58421.735795]  ? security_capable+0x3f/0x60
[58421.735803]  ? drm_ioctl_kernel+0x150/0x150 [drm]
[58421.735821]  ? drm_ioctl_kernel+0x150/0x150 [drm]
[58421.735839]  vmw_generic_ioctl+0x88/0x160 [vmwgfx]
[58421.735851]  vmw_unlocked_ioctl+0x15/0x20 [vmwgfx]
[58421.735861]  __x64_sys_ioctl+0x96/0xd0
[58421.735869]  do_syscall_64+0x3a/0xc0
[58421.735874]  entry_SYSCALL_64_after_hwframe+0x61/0xcb
[58421.735878] RIP: 0033:0x7f7990b49597
[58421.735882] Code: 00 00 90 48 8b 05 f9 a8 0c 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c9 a8 0c 00 f7 d8 64 89 01 48
[58421.735885] RSP: 002b:00007f7990a54ed8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[58421.735887] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f7990b49597
[58421.735889] RDX: 0000000000404080 RSI: 00000000c0186441 RDI: 0000000000000003
[58421.735890] RBP: 00007f7990a54ef0 R08: 0000000000000000 R09: 0000000000000077
[58421.735891] R10: 00007f798c0008d0 R11: 0000000000000246 R12: 00007ffdb3e89f2e
[58421.735892] R13: 00007ffdb3e89f2f R14: 00007f7990a55700 R15: 0000000000000000
[58421.735894]  </TASK>
Re: [PATCH v2] drm/vmwgfx: Work around VMW_ALLOC_DMABUF
Posted by Zack Rusin 3 years, 1 month ago
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

Re: [PATCH v2] drm/vmwgfx: Work around VMW_ALLOC_DMABUF
Posted by Meng Tang 3 years, 1 month ago

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").

Re: [PATCH v2] drm/vmwgfx: Work around VMW_ALLOC_DMABUF
Posted by Zack Rusin 3 years, 1 month ago
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