Jason Wang
2022-May-05 07:46 UTC
[PATCH] vringh: Fix maximum number check for indirect descriptors
On Wed, May 4, 2022 at 4:12 PM Xie Yongji <xieyongji at bytedance.com> wrote:> > We should use size of descriptor chain to check the maximum > number of consumed descriptors in indirect case.AFAIK, it's a guard for loop descriptors.> And the > statistical counts should also be reset to zero each time > we get an indirect descriptor.What might happen if we don't have this patch?> > Fixes: f87d0fbb5798 ("vringh: host-side implementation of virtio rings.") > Signed-off-by: Xie Yongji <xieyongji at bytedance.com> > Signed-off-by: Fam Zheng <fam.zheng at bytedance.com> > --- > drivers/vhost/vringh.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c > index 14e2043d7685..c1810b77a05e 100644 > --- a/drivers/vhost/vringh.c > +++ b/drivers/vhost/vringh.c > @@ -344,12 +344,13 @@ __vringh_iov(struct vringh *vrh, u16 i, > addr = (void *)(long)(a + range.offset); > err = move_to_indirect(vrh, &up_next, &i, addr, &desc, > &descs, &desc_max); > + count = 0;Then it looks to me we can detect a loop indirect descriptor chain? Thanks> if (err) > goto fail; > continue; > } > > - if (count++ == vrh->vring.num) { > + if (count++ == desc_max) { > vringh_bad("Descriptor loop in %p", descs); > err = -ELOOP; > goto fail; > @@ -410,6 +411,7 @@ __vringh_iov(struct vringh *vrh, u16 i, > if (unlikely(up_next > 0)) { > i = return_from_indirect(vrh, &up_next, > &descs, &desc_max); > + count = 0; > slow = false; > } else > break; > -- > 2.20.1 >
Jason Wang
2022-May-05 07:57 UTC
[PATCH] vringh: Fix maximum number check for indirect descriptors
On Thu, May 5, 2022 at 3:46 PM Jason Wang <jasowang at redhat.com> wrote:> > On Wed, May 4, 2022 at 4:12 PM Xie Yongji <xieyongji at bytedance.com> wrote: > > > > We should use size of descriptor chain to check the maximum > > number of consumed descriptors in indirect case. > > AFAIK, it's a guard for loop descriptors. > > > And the > > statistical counts should also be reset to zero each time > > we get an indirect descriptor. > > What might happen if we don't have this patch? > > > > > Fixes: f87d0fbb5798 ("vringh: host-side implementation of virtio rings.") > > Signed-off-by: Xie Yongji <xieyongji at bytedance.com> > > Signed-off-by: Fam Zheng <fam.zheng at bytedance.com> > > --- > > drivers/vhost/vringh.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c > > index 14e2043d7685..c1810b77a05e 100644 > > --- a/drivers/vhost/vringh.c > > +++ b/drivers/vhost/vringh.c > > @@ -344,12 +344,13 @@ __vringh_iov(struct vringh *vrh, u16 i, > > addr = (void *)(long)(a + range.offset); > > err = move_to_indirect(vrh, &up_next, &i, addr, &desc, > > &descs, &desc_max); > > + count = 0; > > Then it looks to me we can detect a loop indirect descriptor chain?I meant "can't" actually. Thanks> > Thanks > > > if (err) > > goto fail; > > continue; > > } > > > > - if (count++ == vrh->vring.num) { > > + if (count++ == desc_max) { > > vringh_bad("Descriptor loop in %p", descs); > > err = -ELOOP; > > goto fail; > > @@ -410,6 +411,7 @@ __vringh_iov(struct vringh *vrh, u16 i, > > if (unlikely(up_next > 0)) { > > i = return_from_indirect(vrh, &up_next, > > &descs, &desc_max); > > + count = 0; > > slow = false; > > } else > > break; > > -- > > 2.20.1 > >