On 9/25/23 12:37, Morten Brørup wrote:
From: Maxime Coquelin [mailto:maxime.coque...@redhat.com]
Sent: Monday, 25 September 2023 10.15
On 9/12/23 09:42, Li Feng wrote:
If the user calls rte_vhost_vring_call() on a ring that has been
invalidated, we will encounter SEGV.
We should check the pointer firstly before accessing it.
Signed-off-by: Li Feng <fen...@smartx.com>
---
v2 -> v3:
- Also fix the rte_vhost_vring_call_nonblock.
v1 -> v2:
- Fix rebase error.
lib/vhost/vhost.c | 14 ++++++++------
lib/vhost/vhost.h | 12 ++++++++++--
2 files changed, 18 insertions(+), 8 deletions(-)
Thanks for posting the fix, the segmentation fault may indeed happen
when injecting IRQ from the app directly using the Vhost API. It cannot
happen when vhost_vring_call() is calle directly from
rte_enqueue_burst/rte_dequeue_burst though.
so I think below patch would be better:
diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
index eb6309b681..733e0ab289 100644
--- a/lib/vhost/vhost.c
+++ b/lib/vhost/vhost.c
@@ -1341,6 +1341,9 @@ rte_vhost_vring_call(int vid, uint16_t vring_idx)
rte_rwlock_read_lock(&vq->access_lock);
+ if (unlikely(!vq->access_ok))
+ return -1;
Don't you need to release the lock before returning here?
Of course yes, I actually caught it after sending my reply, it is
already fixed locally.
But thanks for the review, much appreciated!
Maxime
+
if (vq_is_packed(dev))
vhost_vring_call_packed(dev, vq);
else
@@ -1371,6 +1374,9 @@ 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))
+ return -1;
Don't you need to release the lock before returning here?
+
if (vq_is_packed(dev))
vhost_vring_call_packed(dev, vq);
else
Do you confirm that fixes your issue?
Thanks,
Maxime
