virtualization.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH v2] vringh: Fix loop descriptors check in the indirect cases
       [not found] <20220505100910.137-1-xieyongji@bytedance.com>
@ 2022-05-10  7:43 ` Jason Wang
       [not found]   ` <CACycT3uakPB_JeXb11hrBaPjcdqign3FmuQd3FXgFR7orO_Eaw@mail.gmail.com>
  0 siblings, 1 reply; 3+ messages in thread
From: Jason Wang @ 2022-05-10  7:43 UTC (permalink / raw)
  To: Xie Yongji; +Cc: fam.zheng, rusty, virtualization, kvm, mst

On Thu, May 5, 2022 at 6:08 PM Xie Yongji <xieyongji@bytedance.com> wrote:
>
> We should use size of descriptor chain to test loop condition
> in the indirect case. And another statistical count is also introduced
> for indirect descriptors to avoid conflict with the statistical count
> of direct descriptors.
>
> Fixes: f87d0fbb5798 ("vringh: host-side implementation of virtio rings.")
> Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
> Signed-off-by: Fam Zheng <fam.zheng@bytedance.com>
> ---
>  drivers/vhost/vringh.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
> index 14e2043d7685..eab55accf381 100644
> --- a/drivers/vhost/vringh.c
> +++ b/drivers/vhost/vringh.c
> @@ -292,7 +292,7 @@ __vringh_iov(struct vringh *vrh, u16 i,
>              int (*copy)(const struct vringh *vrh,
>                          void *dst, const void *src, size_t len))
>  {
> -       int err, count = 0, up_next, desc_max;
> +       int err, count = 0, indirect_count = 0, up_next, desc_max;
>         struct vring_desc desc, *descs;
>         struct vringh_range range = { -1ULL, 0 }, slowrange;
>         bool slow = false;
> @@ -349,7 +349,12 @@ __vringh_iov(struct vringh *vrh, u16 i,
>                         continue;
>                 }
>
> -               if (count++ == vrh->vring.num) {
> +               if (up_next == -1)
> +                       count++;
> +               else
> +                       indirect_count++;
> +
> +               if (count > vrh->vring.num || indirect_count > desc_max) {
>                         vringh_bad("Descriptor loop in %p", descs);
>                         err = -ELOOP;
>                         goto fail;
> @@ -411,6 +416,7 @@ __vringh_iov(struct vringh *vrh, u16 i,
>                                 i = return_from_indirect(vrh, &up_next,
>                                                          &descs, &desc_max);
>                                 slow = false;
> +                               indirect_count = 0;

Do we need to reset up_next to -1 here?

Thanks

>                         } else
>                                 break;
>                 }
> --
> 2.20.1
>

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH v2] vringh: Fix loop descriptors check in the indirect cases
       [not found]   ` <CACycT3uakPB_JeXb11hrBaPjcdqign3FmuQd3FXgFR7orO_Eaw@mail.gmail.com>
@ 2022-05-10  7:56     ` Jason Wang
       [not found]       ` <CACycT3v8bo=6YHmb-F3fEjVSCsJdWSLwLy4RTz6hCW39FAZZPA@mail.gmail.com>
  0 siblings, 1 reply; 3+ messages in thread
From: Jason Wang @ 2022-05-10  7:56 UTC (permalink / raw)
  To: Yongji Xie; +Cc: fam.zheng, rusty, virtualization, kvm, mst

On Tue, May 10, 2022 at 3:54 PM Yongji Xie <xieyongji@bytedance.com> wrote:
>
> On Tue, May 10, 2022 at 3:44 PM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Thu, May 5, 2022 at 6:08 PM Xie Yongji <xieyongji@bytedance.com> wrote:
> > >
> > > We should use size of descriptor chain to test loop condition
> > > in the indirect case. And another statistical count is also introduced
> > > for indirect descriptors to avoid conflict with the statistical count
> > > of direct descriptors.
> > >
> > > Fixes: f87d0fbb5798 ("vringh: host-side implementation of virtio rings.")
> > > Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
> > > Signed-off-by: Fam Zheng <fam.zheng@bytedance.com>
> > > ---
> > >  drivers/vhost/vringh.c | 10 ++++++++--
> > >  1 file changed, 8 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
> > > index 14e2043d7685..eab55accf381 100644
> > > --- a/drivers/vhost/vringh.c
> > > +++ b/drivers/vhost/vringh.c
> > > @@ -292,7 +292,7 @@ __vringh_iov(struct vringh *vrh, u16 i,
> > >              int (*copy)(const struct vringh *vrh,
> > >                          void *dst, const void *src, size_t len))
> > >  {
> > > -       int err, count = 0, up_next, desc_max;
> > > +       int err, count = 0, indirect_count = 0, up_next, desc_max;
> > >         struct vring_desc desc, *descs;
> > >         struct vringh_range range = { -1ULL, 0 }, slowrange;
> > >         bool slow = false;
> > > @@ -349,7 +349,12 @@ __vringh_iov(struct vringh *vrh, u16 i,
> > >                         continue;
> > >                 }
> > >
> > > -               if (count++ == vrh->vring.num) {
> > > +               if (up_next == -1)
> > > +                       count++;
> > > +               else
> > > +                       indirect_count++;
> > > +
> > > +               if (count > vrh->vring.num || indirect_count > desc_max) {
> > >                         vringh_bad("Descriptor loop in %p", descs);
> > >                         err = -ELOOP;
> > >                         goto fail;
> > > @@ -411,6 +416,7 @@ __vringh_iov(struct vringh *vrh, u16 i,
> > >                                 i = return_from_indirect(vrh, &up_next,
> > >                                                          &descs, &desc_max);
> > >                                 slow = false;
> > > +                               indirect_count = 0;
> >
> > Do we need to reset up_next to -1 here?
> >
>
> It will be reset to -1 in return_from_indirect().

Right. Then

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks

>
> Thanks,
> Yongji
>

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH v2] vringh: Fix loop descriptors check in the indirect cases
       [not found]       ` <CACycT3v8bo=6YHmb-F3fEjVSCsJdWSLwLy4RTz6hCW39FAZZPA@mail.gmail.com>
@ 2022-06-02  9:51         ` Michael S. Tsirkin
  0 siblings, 0 replies; 3+ messages in thread
From: Michael S. Tsirkin @ 2022-06-02  9:51 UTC (permalink / raw)
  To: Yongji Xie; +Cc: fam.zheng, rusty, kvm, virtualization

On Thu, Jun 02, 2022 at 12:55:50PM +0800, Yongji Xie wrote:
> Ping.


Thanks for the reminder!
Will queue for rc2, rc1 has too much stuff already.

> On Tue, May 10, 2022 at 3:56 PM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Tue, May 10, 2022 at 3:54 PM Yongji Xie <xieyongji@bytedance.com> wrote:
> > >
> > > On Tue, May 10, 2022 at 3:44 PM Jason Wang <jasowang@redhat.com> wrote:
> > > >
> > > > On Thu, May 5, 2022 at 6:08 PM Xie Yongji <xieyongji@bytedance.com> wrote:
> > > > >
> > > > > We should use size of descriptor chain to test loop condition
> > > > > in the indirect case. And another statistical count is also introduced
> > > > > for indirect descriptors to avoid conflict with the statistical count
> > > > > of direct descriptors.
> > > > >
> > > > > Fixes: f87d0fbb5798 ("vringh: host-side implementation of virtio rings.")
> > > > > Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
> > > > > Signed-off-by: Fam Zheng <fam.zheng@bytedance.com>
> > > > > ---
> > > > >  drivers/vhost/vringh.c | 10 ++++++++--
> > > > >  1 file changed, 8 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
> > > > > index 14e2043d7685..eab55accf381 100644
> > > > > --- a/drivers/vhost/vringh.c
> > > > > +++ b/drivers/vhost/vringh.c
> > > > > @@ -292,7 +292,7 @@ __vringh_iov(struct vringh *vrh, u16 i,
> > > > >              int (*copy)(const struct vringh *vrh,
> > > > >                          void *dst, const void *src, size_t len))
> > > > >  {
> > > > > -       int err, count = 0, up_next, desc_max;
> > > > > +       int err, count = 0, indirect_count = 0, up_next, desc_max;
> > > > >         struct vring_desc desc, *descs;
> > > > >         struct vringh_range range = { -1ULL, 0 }, slowrange;
> > > > >         bool slow = false;
> > > > > @@ -349,7 +349,12 @@ __vringh_iov(struct vringh *vrh, u16 i,
> > > > >                         continue;
> > > > >                 }
> > > > >
> > > > > -               if (count++ == vrh->vring.num) {
> > > > > +               if (up_next == -1)
> > > > > +                       count++;
> > > > > +               else
> > > > > +                       indirect_count++;
> > > > > +
> > > > > +               if (count > vrh->vring.num || indirect_count > desc_max) {
> > > > >                         vringh_bad("Descriptor loop in %p", descs);
> > > > >                         err = -ELOOP;
> > > > >                         goto fail;
> > > > > @@ -411,6 +416,7 @@ __vringh_iov(struct vringh *vrh, u16 i,
> > > > >                                 i = return_from_indirect(vrh, &up_next,
> > > > >                                                          &descs, &desc_max);
> > > > >                                 slow = false;
> > > > > +                               indirect_count = 0;
> > > >
> > > > Do we need to reset up_next to -1 here?
> > > >
> > >
> > > It will be reset to -1 in return_from_indirect().
> >
> > Right. Then
> >
> > Acked-by: Jason Wang <jasowang@redhat.com>
> >
> > Thanks
> >
> > >
> > > Thanks,
> > > Yongji
> > >
> >

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-06-02  9:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20220505100910.137-1-xieyongji@bytedance.com>
2022-05-10  7:43 ` [PATCH v2] vringh: Fix loop descriptors check in the indirect cases Jason Wang
     [not found]   ` <CACycT3uakPB_JeXb11hrBaPjcdqign3FmuQd3FXgFR7orO_Eaw@mail.gmail.com>
2022-05-10  7:56     ` Jason Wang
     [not found]       ` <CACycT3v8bo=6YHmb-F3fEjVSCsJdWSLwLy4RTz6hCW39FAZZPA@mail.gmail.com>
2022-06-02  9:51         ` Michael S. Tsirkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).