| Message ID | 20220505100910.137-1-xieyongji@bytedance.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] vringh: Fix loop descriptors check in the indirect cases | expand |
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 >
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(). Thanks, Yongji
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 >
Ping. 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 > > >
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 > > > > >
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; } else break; }
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(-)