[Devel] [PATCH RHEL7 COMMIT] ms/vhost: synchronize IOTLB message with dev cleanup

[Konstantin Khorenko]

The commit is pushed to "branch-rh7-3.10.0-957.12.2.vz7.96.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-957.12.2.vz7.96.1
------>
commit 8558415b2458d30406ac1d69f8751bf9cf8e77d4
Author: Jason Wang <jasowang at redhat.com>
Date:   Tue May 21 17:59:24 2019 +0300

    ms/vhost: synchronize IOTLB message with dev cleanup
    
    Port ms commit 1b15ad683ab4 ("vhost: synchronize IOTLB message with dev cleanup")
    
    DaeRyong Jeong reports a race between vhost_dev_cleanup() and
    vhost_process_iotlb_msg():
    
    Thread interleaving:
    CPU0 (vhost_process_iotlb_msg)                  CPU1 (vhost_dev_cleanup)
    (In the case of both VHOST_IOTLB_UPDATE and
    VHOST_IOTLB_INVALIDATE)
    
    =====                                           =====
                                                    vhost_umem_clean(dev->iotlb);
    if (!dev->iotlb) {
                    ret = -EFAULT;
                            break;
    }
                                                    dev->iotlb = NULL;
    
    The reason is we don't synchronize between them, fixing by protecting
    vhost_process_iotlb_msg() with dev mutex.
    
    Reported-by: DaeRyong Jeong <threeearcat at gmail.com>
    Fixes: 6b1e6cc7855b0 ("vhost: new device IOTLB API")
    Signed-off-by: Jason Wang <jasowang at redhat.com>
    Acked-by: Michael S. Tsirkin <mst at redhat.com>
    Signed-off-by: David S. Miller <davem at davemloft.net>
    
    Need some comment about locking here:
    
    On these paths below we have no mutex held on other (non
    vhost_process_iotlb_msg) side of critical section:
    
    ____fput
     __fput
      file->f_op->release
       vhost_net_release
       vhost_test_release
       vhost_scsi_release
       vhost_vsock_dev_release
    
        vhost_dev_cleanup
    
    _But_ it is not a problem as vhost_process_iotlb_msg can be called only
    from these code path:
    
    file->f_op->aio_write
     vhost_net_aio_write
      vhost_chr_write_iter
       vhost_process_iotlb_msg
    
    And locked variant of calling vhost_dev_cleanup is only called from:
    
    filp->f_op->unlocked_ioctl
     vhost_net_ioctl
      vhost_net_reset_owner <- holds the mutex
     vhost_test_ioctl
      vhost_test_reset_owner <- holds the mutex
    
       vhost_dev_reset_owner
        vhost_dev_cleanup
    
    When we call a write/ioctl helper the reference count on file struct
    should be non-zero, and we can't get to ____fput for these file struct.
    Thus ((struct vhost_net *)iocb->ki_filp->private_data)->dev is protected
    from simultaneous unlocked cleanup from ____fput ... vhost_dev_cleanup
    code path. Also these unlocked cleanup from ____fput can be called only
    once for each vhost_dev so it can't race with itself.
    
    https://jira.sw.ru/browse/PSBM-90318
    
    Signed-off-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
---
 drivers/vhost/vhost.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 697cdca02567..7f54402ff674 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -974,6 +974,7 @@ static int vhost_process_iotlb_msg(struct vhost_dev *dev,
 {
 	int ret = 0;
 
+	mutex_lock(&dev->mutex);
 	vhost_dev_lock_vqs(dev);
 	switch (msg->type) {
 	case VHOST_IOTLB_UPDATE:
@@ -1009,6 +1010,8 @@ static int vhost_process_iotlb_msg(struct vhost_dev *dev,
 	}
 
 	vhost_dev_unlock_vqs(dev);
+	mutex_unlock(&dev->mutex);
+
 	return ret;
 }