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
Possibly Parallel Threads
- [PATCH net 1/2] vhost: use mutex_lock_nested() in vhost_dev_lock_vqs()
- [PATCH net 1/2] vhost: use mutex_lock_nested() in vhost_dev_lock_vqs()
- [PATCH net 1/2] vhost: use mutex_lock_nested() in vhost_dev_lock_vqs()
- [PATCH net] vhost: synchronize IOTLB message with dev cleanup
- KASAN: use-after-free Read in vhost_chr_write_iter