lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 23 Jul 2021 14:44:35 +0800 From: Desmond Cheong Zhi Xi <desmondcheongzx@...il.com> To: Zack Rusin <zackr@...are.com>, linux-graphics-maintainer@...are.com, airlied@...ux.ie, daniel@...ll.ch, maarten.lankhorst@...ux.intel.com, mripard@...nel.org, tzimmermann@...e.de Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org, intel-gfx@...ts.freedesktop.org, skhan@...uxfoundation.org, gregkh@...uxfoundation.org, linux-kernel-mentees@...ts.linuxfoundation.org Subject: Re: [PATCH 3/3] drm/vmwgfx: fix potential UAF in vmwgfx_surface.c 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@...il.com> > > Reviewed-by: Zack Rusin <zackr@...are.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
Powered by blists - more mailing lists