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

[Zack Rusin]

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