lkml.org 
[lkml]   [2021]   [Jul]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 3/3] drm/vmwgfx: fix potential UAF in vmwgfx_surface.c
From
Date
On 23/7/21 3:17 am, Zack Rusin wrote:
> On 7/22/21 5:29 AM, Desmond Cheong Zhi Xi wrote:
>> drm_file.master should be protected by either drm_device.master_mutex
>> or drm_file.master_lookup_lock when being dereferenced. However,
>> drm_master_get is called on unprotected file_priv->master pointers in
>> vmw_surface_define_ioctl and vmw_gb_surface_define_internal.
>>
>> This is fixed by replacing drm_master_get with drm_file_get_master.
>>
>> Signed-off-by: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
>
> Reviewed-by: Zack Rusin <zackr@vmware.com>
>
> Thanks for taking the time to fix this. Apart from the clear logic
> error, do you happen to know under what circumstances would this be hit?
> We have someone looking at writing some vmwgfx specific igt tests and I
> was wondering if I could add this to the list.
>
> z

Hi Zack,

Thanks for the review.

For some context, the use-after-free happens when there's a race between
accessing the value of drm_file.master, and a call to
drm_setmaster_ioctl. If drm_file is not the creator of master, then the
ioctl allocates a new master for drm_file and puts the old master.

Thus for example, the old value of drm_file.master could be freed in
between getting the value of file_priv->master, and the call to
drm_master_get.

I'm not entirely sure whether this scenario is a good candidate for a test?

For further reference, the issue was originally caught by Syzbot here:
https://syzkaller.appspot.com/bug?id=148d2f1dfac64af52ffd27b661981a540724f803

And from the logs it seems that the reproducer set up a race between
DRM_IOCTL_GET_UNIQUE and DRM_IOCTL_SET_MASTER. So possibly a race
between VMW_CREATE_SURFACE and DRM_IOCTL_SET_MASTER could trigger the
same bug.

Best wishes,
Desmond

\
 
 \ /
  Last update: 2021-07-23 08:46    [W:0.181 / U:0.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site