Re: [PATCH v3] vhost: avoid potential null pointer access

[Maxime Coquelin <maxime.coquelin@redhat.com>]

On 9/25/23 10:15, Maxime Coquelin wrote:
> 
> 
> 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 <fengli@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;
> +
>          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;
> +
>          if (vq_is_packed(dev))
>                  vhost_vring_call_packed(dev, vq);
>          else
> 
> 
> Do you confirm that fixes your issue?

As pointed out by David off-list, there are other places where we need
to add this check. I will prepare a patch fixing them all.

Thanks,
Maxime

> Thanks,
> Maxime