| Message ID | 20210331080519.172-4-xieyongji@bytedance.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce VDUSE - vDPA Device in Userspace | expand |
On Wed, Mar 31, 2021 at 04:05:12PM +0800, Xie Yongji wrote: > Use vhost_dev->mutex to protect vhost device iotlb from > concurrent access. > > Fixes: 4c8cf318("vhost: introduce vDPA-based backend") > Cc: stable@vger.kernel.org > Signed-off-by: Xie Yongji <xieyongji@bytedance.com> > Acked-by: Jason Wang <jasowang@redhat.com> > Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> I could not figure out whether there's a bug there now. If yes when is the concurrent access triggered? > --- > drivers/vhost/vdpa.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c > index 3947fbc2d1d5..63b28d3aee7c 100644 > --- a/drivers/vhost/vdpa.c > +++ b/drivers/vhost/vdpa.c > @@ -725,9 +725,11 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, > const struct vdpa_config_ops *ops = vdpa->config; > int r = 0; > > + mutex_lock(&dev->mutex); > + > r = vhost_dev_check_owner(dev); > if (r) > - return r; > + goto unlock; > > switch (msg->type) { > case VHOST_IOTLB_UPDATE: > @@ -748,6 +750,8 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, > r = -EINVAL; > break; > } > +unlock: > + mutex_unlock(&dev->mutex); > > return r; > } > -- > 2.11.0
On Sat, Apr 10, 2021 at 12:16 AM Michael S. Tsirkin <mst@redhat.com> wrote: > > On Wed, Mar 31, 2021 at 04:05:12PM +0800, Xie Yongji wrote: > > Use vhost_dev->mutex to protect vhost device iotlb from > > concurrent access. > > > > Fixes: 4c8cf318("vhost: introduce vDPA-based backend") > > Cc: stable@vger.kernel.org > > Signed-off-by: Xie Yongji <xieyongji@bytedance.com> > > Acked-by: Jason Wang <jasowang@redhat.com> > > Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> > > I could not figure out whether there's a bug there now. > If yes when is the concurrent access triggered? > When userspace sends the VHOST_IOTLB_MSG_V2 message concurrently? vhost_vdpa_chr_write_iter -> vhost_chr_write_iter -> vhost_vdpa_process_iotlb_msg() Thanks, Yongji
On Sun, Apr 11, 2021 at 01:36:18PM +0800, Yongji Xie wrote: > On Sat, Apr 10, 2021 at 12:16 AM Michael S. Tsirkin <mst@redhat.com> wrote: > > > > On Wed, Mar 31, 2021 at 04:05:12PM +0800, Xie Yongji wrote: > > > Use vhost_dev->mutex to protect vhost device iotlb from > > > concurrent access. > > > > > > Fixes: 4c8cf318("vhost: introduce vDPA-based backend") > > > Cc: stable@vger.kernel.org > > > Signed-off-by: Xie Yongji <xieyongji@bytedance.com> > > > Acked-by: Jason Wang <jasowang@redhat.com> > > > Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> > > > > I could not figure out whether there's a bug there now. > > If yes when is the concurrent access triggered? > > > > When userspace sends the VHOST_IOTLB_MSG_V2 message concurrently? > > vhost_vdpa_chr_write_iter -> vhost_chr_write_iter -> > vhost_vdpa_process_iotlb_msg() > > Thanks, > Yongji And then what happens currently?
On Mon, Apr 12, 2021 at 4:49 AM Michael S. Tsirkin <mst@redhat.com> wrote: > > On Sun, Apr 11, 2021 at 01:36:18PM +0800, Yongji Xie wrote: > > On Sat, Apr 10, 2021 at 12:16 AM Michael S. Tsirkin <mst@redhat.com> wrote: > > > > > > On Wed, Mar 31, 2021 at 04:05:12PM +0800, Xie Yongji wrote: > > > > Use vhost_dev->mutex to protect vhost device iotlb from > > > > concurrent access. > > > > > > > > Fixes: 4c8cf318("vhost: introduce vDPA-based backend") > > > > Cc: stable@vger.kernel.org > > > > Signed-off-by: Xie Yongji <xieyongji@bytedance.com> > > > > Acked-by: Jason Wang <jasowang@redhat.com> > > > > Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> > > > > > > I could not figure out whether there's a bug there now. > > > If yes when is the concurrent access triggered? > > > > > > > When userspace sends the VHOST_IOTLB_MSG_V2 message concurrently? > > > > vhost_vdpa_chr_write_iter -> vhost_chr_write_iter -> > > vhost_vdpa_process_iotlb_msg() > > > > Thanks, > > Yongji > > And then what happens currently? > Then we might access vhost_vdpa_map() concurrently and cause corruption of the list and interval tree in struct vhost_iotlb. Thanks, Yongji
On Mon, Apr 12, 2021 at 10:29:17AM +0800, Yongji Xie wrote: > On Mon, Apr 12, 2021 at 4:49 AM Michael S. Tsirkin <mst@redhat.com> wrote: > > > > On Sun, Apr 11, 2021 at 01:36:18PM +0800, Yongji Xie wrote: > > > On Sat, Apr 10, 2021 at 12:16 AM Michael S. Tsirkin <mst@redhat.com> wrote: > > > > > > > > On Wed, Mar 31, 2021 at 04:05:12PM +0800, Xie Yongji wrote: > > > > > Use vhost_dev->mutex to protect vhost device iotlb from > > > > > concurrent access. > > > > > > > > > > Fixes: 4c8cf318("vhost: introduce vDPA-based backend") > > > > > Cc: stable@vger.kernel.org > > > > > Signed-off-by: Xie Yongji <xieyongji@bytedance.com> > > > > > Acked-by: Jason Wang <jasowang@redhat.com> > > > > > Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> > > > > > > > > I could not figure out whether there's a bug there now. > > > > If yes when is the concurrent access triggered? > > > > > > > > > > When userspace sends the VHOST_IOTLB_MSG_V2 message concurrently? > > > > > > vhost_vdpa_chr_write_iter -> vhost_chr_write_iter -> > > > vhost_vdpa_process_iotlb_msg() > > > > > > Thanks, > > > Yongji > > > > And then what happens currently? > > > > Then we might access vhost_vdpa_map() concurrently and cause > corruption of the list and interval tree in struct vhost_iotlb. > > Thanks, > Yongji OK. Sounds like it's actually needed in this release if possible. Pls add this info in the commit log and post it as a separate patch.
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c index 3947fbc2d1d5..63b28d3aee7c 100644 --- a/drivers/vhost/vdpa.c +++ b/drivers/vhost/vdpa.c @@ -725,9 +725,11 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, const struct vdpa_config_ops *ops = vdpa->config; int r = 0; + mutex_lock(&dev->mutex); + r = vhost_dev_check_owner(dev); if (r) - return r; + goto unlock; switch (msg->type) { case VHOST_IOTLB_UPDATE: @@ -748,6 +750,8 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev *dev, r = -EINVAL; break; } +unlock: + mutex_unlock(&dev->mutex); return r; }
Use vhost_dev->mutex to protect vhost device iotlb from concurrent access. Fixes: 4c8cf318("vhost: introduce vDPA-based backend") Cc: stable@vger.kernel.org Signed-off-by: Xie Yongji <xieyongji@bytedance.com> Acked-by: Jason Wang <jasowang@redhat.com> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> --- drivers/vhost/vdpa.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)