Jason Wang
2018-Jan-23 09:27 UTC
[PATCH net 1/2] vhost: use mutex_lock_nested() in vhost_dev_lock_vqs()
We used to call mutex_lock() in vhost_dev_lock_vqs() which tries to
hold mutexes of all virtqueues. This may confuse lockdep to report a
possible deadlock because of trying to hold locks belong to same
class. Switch to use mutex_lock_nested() to avoid false positive.
Fixes: 6b1e6cc7855b0 ("vhost: new device IOTLB API")
Reported-by: syzbot+dbb7c1161485e61b0241 at syzkaller.appspotmail.com
Signed-off-by: Jason Wang <jasowang at redhat.com>
---
drivers/vhost/vhost.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 33ac2b1..549771a 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -904,7 +904,7 @@ static void vhost_dev_lock_vqs(struct vhost_dev *d)
{
int i = 0;
for (i = 0; i < d->nvqs; ++i)
- mutex_lock(&d->vqs[i]->mutex);
+ mutex_lock_nested(&d->vqs[i]->mutex, i);
}
static void vhost_dev_unlock_vqs(struct vhost_dev *d)
--
2.7.4
Jason Wang
2018-Jan-23 09:27 UTC
[PATCH net 2/2] vhost: do not try to access device IOTLB when not initialized
The code will try to access dev->iotlb when processing
VHOST_IOTLB_INVALIDATE even if it was not initialized which may lead
to NULL pointer dereference. Fixes this by check dev->iotlb before.
Fixes: 6b1e6cc7855b0 ("vhost: new device IOTLB API")
Signed-off-by: Jason Wang <jasowang at redhat.com>
---
drivers/vhost/vhost.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 549771a..5727b18 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1015,6 +1015,10 @@ static int vhost_process_iotlb_msg(struct vhost_dev *dev,
vhost_iotlb_notify_vq(dev, msg);
break;
case VHOST_IOTLB_INVALIDATE:
+ if (!dev->iotlb) {
+ ret = -EFAULT;
+ break;
+ }
vhost_vq_meta_reset(dev);
vhost_del_umem_range(dev->iotlb, msg->iova,
msg->iova + msg->size - 1);
--
2.7.4
Michael S. Tsirkin
2018-Jan-23 15:57 UTC
[PATCH net 1/2] vhost: use mutex_lock_nested() in vhost_dev_lock_vqs()
On Tue, Jan 23, 2018 at 05:27:25PM +0800, Jason Wang wrote:> We used to call mutex_lock() in vhost_dev_lock_vqs() which tries to > hold mutexes of all virtqueues. This may confuse lockdep to report a > possible deadlock because of trying to hold locks belong to same > class. Switch to use mutex_lock_nested() to avoid false positive. > > Fixes: 6b1e6cc7855b0 ("vhost: new device IOTLB API") > Reported-by: syzbot+dbb7c1161485e61b0241 at syzkaller.appspotmail.com > Signed-off-by: Jason Wang <jasowang at redhat.com>Acked-by: Michael S. Tsirkin <mst at redhat.com>> --- > drivers/vhost/vhost.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > index 33ac2b1..549771a 100644 > --- a/drivers/vhost/vhost.c > +++ b/drivers/vhost/vhost.c > @@ -904,7 +904,7 @@ static void vhost_dev_lock_vqs(struct vhost_dev *d) > { > int i = 0; > for (i = 0; i < d->nvqs; ++i) > - mutex_lock(&d->vqs[i]->mutex); > + mutex_lock_nested(&d->vqs[i]->mutex, i); > } > > static void vhost_dev_unlock_vqs(struct vhost_dev *d) > -- > 2.7.4
Michael S. Tsirkin
2018-Jan-23 15:58 UTC
[PATCH net 2/2] vhost: do not try to access device IOTLB when not initialized
On Tue, Jan 23, 2018 at 05:27:26PM +0800, Jason Wang wrote:> The code will try to access dev->iotlb when processing > VHOST_IOTLB_INVALIDATE even if it was not initialized which may lead > to NULL pointer dereference. Fixes this by check dev->iotlb before. > > 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>> --- > drivers/vhost/vhost.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > index 549771a..5727b18 100644 > --- a/drivers/vhost/vhost.c > +++ b/drivers/vhost/vhost.c > @@ -1015,6 +1015,10 @@ static int vhost_process_iotlb_msg(struct vhost_dev *dev, > vhost_iotlb_notify_vq(dev, msg); > break; > case VHOST_IOTLB_INVALIDATE: > + if (!dev->iotlb) { > + ret = -EFAULT; > + break; > + } > vhost_vq_meta_reset(dev); > vhost_del_umem_range(dev->iotlb, msg->iova, > msg->iova + msg->size - 1); > -- > 2.7.4
David Miller
2018-Jan-24 21:38 UTC
[PATCH net 1/2] vhost: use mutex_lock_nested() in vhost_dev_lock_vqs()
From: Jason Wang <jasowang at redhat.com> Date: Tue, 23 Jan 2018 17:27:25 +0800> We used to call mutex_lock() in vhost_dev_lock_vqs() which tries to > hold mutexes of all virtqueues. This may confuse lockdep to report a > possible deadlock because of trying to hold locks belong to same > class. Switch to use mutex_lock_nested() to avoid false positive. > > Fixes: 6b1e6cc7855b0 ("vhost: new device IOTLB API") > Reported-by: syzbot+dbb7c1161485e61b0241 at syzkaller.appspotmail.com > Signed-off-by: Jason Wang <jasowang at redhat.com>Michael, I see you ACK'd this, meaning that you're OK with these two fixes going via my net tree? Thanks.
Michael S. Tsirkin
2018-Jan-24 21:46 UTC
[PATCH net 1/2] vhost: use mutex_lock_nested() in vhost_dev_lock_vqs()
On Wed, Jan 24, 2018 at 04:38:30PM -0500, David Miller wrote:> From: Jason Wang <jasowang at redhat.com> > Date: Tue, 23 Jan 2018 17:27:25 +0800 > > > We used to call mutex_lock() in vhost_dev_lock_vqs() which tries to > > hold mutexes of all virtqueues. This may confuse lockdep to report a > > possible deadlock because of trying to hold locks belong to same > > class. Switch to use mutex_lock_nested() to avoid false positive. > > > > Fixes: 6b1e6cc7855b0 ("vhost: new device IOTLB API") > > Reported-by: syzbot+dbb7c1161485e61b0241 at syzkaller.appspotmail.com > > Signed-off-by: Jason Wang <jasowang at redhat.com> > > Michael, I see you ACK'd this, meaning that you're OK with these two > fixes going via my net tree? > > Thanks.Yes - this seems to be what Jason wanted (judging by the net tag in the subject) and I'm fine with it. Thanks a lot. -- MST