[1/7] vhost: fix missing vring call check on virtqueue access
Checks
Commit Message
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
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>
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>
>
>
@@ -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