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

[Andrey Misiurov <andrey.misyrov@xxxxxxxxx>]

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>
---
 drivers/xen/gntdev.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index b71f5ba..1ad0220 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -670,15 +670,19 @@ static long gntdev_ioctl_map_grant_ref(struct gntdev_priv 
*priv,
        mutex_lock(&priv->lock);
        gntdev_add_map(priv, map);
        op.index = map->index << PAGE_SHIFT;
+       mutex_unlock(&priv->lock);
 
        if (copy_to_user(u, &op, sizeof(op)) != 0) {
-               list_del(&map->next);
+               mutex_lock(&priv->lock);
+               map = gntdev_find_map_index(priv, op.index >> PAGE_SHIFT,
+                                           op.count);
+               if (map)
+                       list_del(&map->next);
                mutex_unlock(&priv->lock);
                gntdev_put_map(priv, map);
                return -EFAULT;
        }
 
-       mutex_unlock(&priv->lock);
        return 0;
 }
 

base-commit: 61b6d1df1d6234c38a6ea7e4f58e7da035ef6ab4
-- 
2.43.7