[v2] vhost: add sanity check when operating the split ring

Message ID 20211014124008.3570044-1-fengli@smartx.com (mailing list archive)
State Accepted, archived
Delegated to: Maxime Coquelin
Headers
Series [v2] vhost: add sanity check when operating the split ring |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/github-robot: build success github build: passed
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS

Commit Message

Li Feng Oct. 14, 2021, 12:40 p.m. UTC
  The idx in rte_vhost_set_last_inflight_io_split is from the frontend
driver, check if it's in the virtqueue range.

Fixes: bb0c2de9602b ("vhost: add APIs to operate inflight ring")
Cc: stable@dpdk.org

Signed-off-by: Li Feng <fengli@smartx.com>
---
 lib/vhost/vhost.c | 3 +++
 1 file changed, 3 insertions(+)
  

Comments

Maxime Coquelin Oct. 15, 2021, 8:51 a.m. UTC | #1
The title is too vague, I would put something like:

vhost: add sanity check on inflight last index

On 10/14/21 14:40, Li Feng wrote:
> The idx in rte_vhost_set_last_inflight_io_split is from the frontend

s/idx/index/

> driver, check if it's in the virtqueue range.
> 
> Fixes: bb0c2de9602b ("vhost: add APIs to operate inflight ring")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Li Feng <fengli@smartx.com>
> ---
>   lib/vhost/vhost.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
> index 9540522dac..3b674ac320 100644
> --- a/lib/vhost/vhost.c
> +++ b/lib/vhost/vhost.c
> @@ -1226,6 +1226,9 @@ rte_vhost_set_last_inflight_io_split(int vid, uint16_t vring_idx,
>   	if (unlikely(!vq->inflight_split))
>   		return -1;
>   
> +	if (unlikely(idx >= vq->size))
> +		return -1;
> +
>   	vq->inflight_split->last_inflight_io = idx;
>   	return 0;
>   }
> 

Other than that, this is fine to me:

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

If my suggestions are fine for you, I can fix while applying.

Thanks,
Maxime
  
Li Feng Oct. 15, 2021, 9:25 a.m. UTC | #2
On Fri, Oct 15, 2021 at 4:52 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
> The title is too vague, I would put something like:
>
> vhost: add sanity check on inflight last index
>
> On 10/14/21 14:40, Li Feng wrote:
> > The idx in rte_vhost_set_last_inflight_io_split is from the frontend
>
> s/idx/index/
>
> > driver, check if it's in the virtqueue range.
> >
> > Fixes: bb0c2de9602b ("vhost: add APIs to operate inflight ring")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Li Feng <fengli@smartx.com>
> > ---
> >   lib/vhost/vhost.c | 3 +++
> >   1 file changed, 3 insertions(+)
> >
> > diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
> > index 9540522dac..3b674ac320 100644
> > --- a/lib/vhost/vhost.c
> > +++ b/lib/vhost/vhost.c
> > @@ -1226,6 +1226,9 @@ rte_vhost_set_last_inflight_io_split(int vid, uint16_t vring_idx,
> >       if (unlikely(!vq->inflight_split))
> >               return -1;
> >
> > +     if (unlikely(idx >= vq->size))
> > +             return -1;
> > +
> >       vq->inflight_split->last_inflight_io = idx;
> >       return 0;
> >   }
> >
>
> Other than that, this is fine to me:
>
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>
> If my suggestions are fine for you, I can fix while applying.
>
It's fine.

> Thanks,
> Maxime
>
  
Maxime Coquelin Oct. 21, 2021, 12:32 p.m. UTC | #3
On 10/15/21 11:25, Li Feng wrote:
> On Fri, Oct 15, 2021 at 4:52 PM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
>>
>> The title is too vague, I would put something like:
>>
>> vhost: add sanity check on inflight last index
>>
>> On 10/14/21 14:40, Li Feng wrote:
>>> The idx in rte_vhost_set_last_inflight_io_split is from the frontend
>>
>> s/idx/index/
>>
>>> driver, check if it's in the virtqueue range.
>>>
>>> Fixes: bb0c2de9602b ("vhost: add APIs to operate inflight ring")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Li Feng <fengli@smartx.com>
>>> ---
>>>    lib/vhost/vhost.c | 3 +++
>>>    1 file changed, 3 insertions(+)
>>>
>>> diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
>>> index 9540522dac..3b674ac320 100644
>>> --- a/lib/vhost/vhost.c
>>> +++ b/lib/vhost/vhost.c
>>> @@ -1226,6 +1226,9 @@ rte_vhost_set_last_inflight_io_split(int vid, uint16_t vring_idx,
>>>        if (unlikely(!vq->inflight_split))
>>>                return -1;
>>>
>>> +     if (unlikely(idx >= vq->size))
>>> +             return -1;
>>> +
>>>        vq->inflight_split->last_inflight_io = idx;
>>>        return 0;
>>>    }
>>>
>>
>> Other than that, this is fine to me:
>>
>> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>
>> If my suggestions are fine for you, I can fix while applying.
>>
> It's fine.
> 
>> Thanks,
>> Maxime
>>
> 

Applied to dpdk-next-virtio/main.

Thanks,
Maxime
  

Patch

diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
index 9540522dac..3b674ac320 100644
--- a/lib/vhost/vhost.c
+++ b/lib/vhost/vhost.c
@@ -1226,6 +1226,9 @@  rte_vhost_set_last_inflight_io_split(int vid, uint16_t vring_idx,
 	if (unlikely(!vq->inflight_split))
 		return -1;
 
+	if (unlikely(idx >= vq->size))
+		return -1;
+
 	vq->inflight_split->last_inflight_io = idx;
 	return 0;
 }