diff mbox series

[1/7] vhost: fix missing vring call check on virtqueue access

Message ID 20230925163610.3307750-2-maxime.coquelin@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series vhost: ensure vitqueue access status is checked |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Maxime Coquelin Sept. 25, 2023, 4:36 p.m. UTC 
  Acquiring the access lock is not enough to ensure
virtqueue's metadata such as vring pointers are valid.

The access status must also be checked.

Fixes: c5736998305d ("vhost: fix missing virtqueue lock protection")
Fixes: 830f7e790732 ("vhost: add non-blocking API for posting interrupt")
Cc: stable@dpdk.org

Reported-by: Li Feng <fengli@smartx.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/vhost/vhost.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

Comments

David Marchand Oct. 19, 2023, 7:24 a.m. UTC | #1 
On Mon, Sep 25, 2023 at 6:36 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
> Acquiring the access lock is not enough to ensure
> virtqueue's metadata such as vring pointers are valid.
>
> The access status must also be checked.

Even if adding the lock was not enough, I would flag Fixes:
6c299bb7322f ("vhost: introduce vring call API")

>
> Fixes: c5736998305d ("vhost: fix missing virtqueue lock protection")
> Fixes: 830f7e790732 ("vhost: add non-blocking API for posting interrupt")
> Cc: stable@dpdk.org
>
> Reported-by: Li Feng <fengli@smartx.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Maxime Coquelin Oct. 20, 2023, 8:40 a.m. UTC | #2 
On 10/19/23 09:24, David Marchand wrote:
> On Mon, Sep 25, 2023 at 6:36 PM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
>>
>> Acquiring the access lock is not enough to ensure
>> virtqueue's metadata such as vring pointers are valid.
>>
>> The access status must also be checked.
> 
> Even if adding the lock was not enough, I would flag Fixes:
> 6c299bb7322f ("vhost: introduce vring call API")

Makes sense, adding it to v2.

Thanks,
Maxime

>>
>> Fixes: c5736998305d ("vhost: fix missing virtqueue lock protection")
>> Fixes: 830f7e790732 ("vhost: add non-blocking API for posting interrupt")
>> Cc: stable@dpdk.org
>>
>> Reported-by: Li Feng <fengli@smartx.com>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> 
>
diff mbox series

Patch

diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
index c03bb9c6eb..e9c775fa26 100644
--- a/lib/vhost/vhost.c
+++ b/lib/vhost/vhost.c
@@ -1328,6 +1328,7 @@  rte_vhost_vring_call(int vid, uint16_t vring_idx)
 {
 	struct virtio_net *dev;
 	struct vhost_virtqueue *vq;
+	int ret = 0;
 
 	dev = get_device(vid);
 	if (!dev)
@@ -1342,14 +1343,20 @@  rte_vhost_vring_call(int vid, uint16_t vring_idx)
 
 	rte_rwlock_read_lock(&vq->access_lock);
 
+	if (unlikely(!vq->access_ok)) {
+		ret = -1;
+		goto out_unlock;
+	}
+
 	if (vq_is_packed(dev))
 		vhost_vring_call_packed(dev, vq);
 	else
 		vhost_vring_call_split(dev, vq);
 
+out_unlock:
 	rte_rwlock_read_unlock(&vq->access_lock);
 
-	return 0;
+	return ret;
 }
 
 int
@@ -1357,6 +1364,7 @@  rte_vhost_vring_call_nonblock(int vid, uint16_t vring_idx)
 {
 	struct virtio_net *dev;
 	struct vhost_virtqueue *vq;
+	int ret = 0;
 
 	dev = get_device(vid);
 	if (!dev)
@@ -1372,14 +1380,20 @@  rte_vhost_vring_call_nonblock(int vid, uint16_t vring_idx)
 	if (rte_rwlock_read_trylock(&vq->access_lock))
 		return -EAGAIN;
 
+	if (unlikely(!vq->access_ok)) {
+		ret = -1;
+		goto out_unlock;
+	}
+
 	if (vq_is_packed(dev))
 		vhost_vring_call_packed(dev, vq);
 	else
 		vhost_vring_call_split(dev, vq);
 
+out_unlock:
 	rte_rwlock_read_unlock(&vq->access_lock);
 
-	return 0;
+	return ret;
 }
 
 uint16_t