Re: [PATCH] xen/gntdev: fix deadlock between map ioctl and mmap

[Juergen Gross]

On 13.10.26 19:07, Andrey Misiurov wrote:
> Commit 45ca1afe2fd1 ("xen/gntdev: fix error handling in ioctl") moved
> copy_to_user() in gntdev_ioctl_map_grant_ref() under priv->lock. A
> fault in copy_to_user() takes mmap_lock, while gntdev_mmap() is called
> with mmap_lock held and takes priv->lock:
>
>    ioctl thread                        mmap thread
>    mutex_lock(&priv->lock)
>                                        mmap_write_lock(mm)
>                                        gntdev_mmap()
>                                          mutex_lock(&priv->lock)
>    copy_to_user() faults
>      mmap_read_lock(mm)
>
> Reproduced on Debian 6.12.111 in a Xen guest, with the ioctl argument
> on a MAP_PRIVATE file page that has not been written yet. sysrq-w:
>
>    task:repro state:D stack:0 pid:4479 tgid:4477 ppid:1 flags:0x00000000
>     rwsem_down_read_slowpath+0x25c/0x480
>     down_read_killable+0x48/0xc0
>     lock_mm_and_find_vma+0x96/0x250
>     do_user_addr_fault+0x12d/0x620
>     exc_page_fault+0x7e/0x180
>     asm_exc_page_fault+0x26/0x30
>     _copy_to_user+0x36/0x50
>     gntdev_ioctl+0x3b9/0xa00 [xen_gntdev]
>     __x64_sys_ioctl+0x97/0xc0
>    task:repro state:D stack:0 pid:4480 tgid:4477 ppid:1 flags:0x00000000
>     __mutex_lock.constprop.0+0x41b/0x720
>     gntdev_mmap+0x4d/0x330 [xen_gntdev]
>     __mmap_region+0x697/0xb10
>     mmap_region+0x78/0xa0
>     do_mmap+0x45d/0x630
>     vm_mmap_pgoff+0xf8/0x1c0
>
> Drop priv->lock before copy_to_user() again. On failure, re-take it
> and remove the map by index, as IOCTL_GNTDEV_UNMAP_GRANT_REF does,
> since another thread may have unmapped it meanwhile.
>
> Fixes: 45ca1afe2fd1 ("xen/gntdev: fix error handling in ioctl")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Andrey Misiurov <andrey.misyrov@xxxxxxxxx>
Reviewed-by: Juergen Gross <jgross@xxxxxxxx>


Juergen

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature