[PATCH 0/2] vringh: fix infinite loop on cyclic top-level indirect descriptor

[PATCH 0/2] vringh: fix infinite loop on cyclic top-level indirect descriptor

[Fang Xieyan]

__vringh_iov()'s F_INDIRECT branch does `continue` before the descriptor
accounting block, so a top-level indirect descriptor is never charged to
`count`.  When such a descriptor also carries NEXT pointing to itself,
returning from the indirect table resumes at the same uncharged descriptor;
`count` stays flat and `indirect_count` is reset to 0 on every return, so
neither bound in the loop check can ever trip.  A guest can keep the host
vringh worker spinning inside __vringh_iov(), causing a host DoS.

Patch 1/2 moves the accounting block above the INDIRECT switch so each
top-level descriptor, including INDIRECT ones, is charged one traversal step
before the walk descends.  Legitimate chains and indirect descriptors stay
within vring.num; a cyclic indirect descriptor is now rejected with -ELOOP.
Patch 1 carries the Fixes tag and is Cc'd to stable.

Patch 2/2 appends a regression test to tools/virtio/vringh_test.c.  It builds
the offending ring: top-level desc[1].flags = VRING_DESC_F_INDIRECT |
VRING_DESC_F_NEXT and desc[1].next = 1, with a single-entry indirect table,
and asserts that vringh_getdesc_user() returns -ELOOP.

Verified against v7.3-rc3: both patches pass git apply --check.  With the fix,
vringh_test --indirect observes -ELOOP and exits 0.  With the fix reverted, the
walk re-enters the same descriptor without making forward progress and never
returns -ELOOP, so the assertion does not hold and the regression is caught
either way.

Fang Xieyan (2):
  vringh: bound top-level re-entry into indirect tables
  vringh: add regression test for cyclic indirect descriptor

 drivers/vhost/vringh.c     | 22 ++++++++++----------
 tools/virtio/vringh_test.c | 41 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 11 deletions(-)

-- 
2.50.1

[PATCH 1/2] vringh: bound top-level re-entry into indirect tables

[Fang Xieyan]

In __vringh_iov(), the F_INDIRECT branch descends into the indirect
table and continues before the descriptor accounting runs. A top-level
indirect descriptor is therefore never charged to count. If such a
descriptor sets NEXT to point back at itself, returning from the
indirect table resumes at the same top-level descriptor, which is again
not counted, so the walk never makes forward progress.

Because count stays flat and indirect_count is reset to 0 on every
return to the top-level table, neither bound in the loop check trips.
A guest can spin the vringh worker at 100% CPU inside __vringh_iov(),
an uninterruptible host DoS.

Move the descriptor accounting above the indirect switch so that a
top-level indirect descriptor is charged one top-level traversal step
before the walk descends into its table, bringing this re-entry under
the existing vrh->vring.num bound. Each top-level descriptor is still
charged at most one step and indirect_count still bounds a single
table, so legitimate chains (including the multiple-indirect case in
tools/virtio/vringh_test.c) stay within vring.num, while a cyclic
indirect descriptor is now rejected with -ELOOP.

Fixes: dbd29e075228 ("vringh: Fix loop descriptors check in the indirect cases")
Cc: Xie Yongji <xieyongji@bytedance.com>
Cc: stable@vger.kernel.org
Assisted-by: Hawkeye:GLM-5.3-flash
Assisted-by: Qoder:Qwen3.8-Max
Signed-off-by: Fang Xieyan <fangxy@xiaopeng.com>
---
 drivers/vhost/vringh.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
index 9066f9f..f672e11 100644
--- a/drivers/vhost/vringh.c
+++ b/drivers/vhost/vringh.c
@@ -333,6 +333,17 @@ __vringh_iov(struct vringh *vrh, u16 i,
 		if (unlikely(err))
 			goto fail;
 
+		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;
+		}
+
 		if (unlikely(desc.flags &
 			     cpu_to_vringh16(vrh, VRING_DESC_F_INDIRECT))) {
 			u64 a = vringh64_to_cpu(vrh, desc.addr);
@@ -358,17 +369,6 @@ __vringh_iov(struct vringh *vrh, u16 i,
 			continue;
 		}
 
-		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;
-		}
-
 		if (desc.flags & cpu_to_vringh16(vrh, VRING_DESC_F_WRITE))
 			iov = wiov;
 		else {
-- 
2.50.1

Re: [PATCH 1/2] vringh: bound top-level re-entry into indirect tables

[sashiko-bot]

> In __vringh_iov(), the F_INDIRECT branch descends into the indirect
> table and continues before the descriptor accounting runs. A top-level
> indirect descriptor is therefore never charged to count. If such a
> descriptor sets NEXT to point back at itself, returning from the
> indirect table resumes at the same top-level descriptor, which is again
> not counted, so the walk never makes forward progress.
> 
> Because count stays flat and indirect_count is reset to 0 on every
> return to the top-level table, neither bound in the loop check trips.
> A guest can spin the vringh worker at 100% CPU inside __vringh_iov(),
> an uninterruptible host DoS.
> 
> Move the descriptor accounting above the indirect switch so that a
> top-level indirect descriptor is charged one top-level traversal step
> before the walk descends into its table, bringing this re-entry under
> the existing vrh->vring.num bound. Each top-level descriptor is still
> charged at most one step and indirect_count still bounds a single
> table, so legitimate chains (including the multiple-indirect case in
> tools/virtio/vringh_test.c) stay within vring.num, while a cyclic
> indirect descriptor is now rejected with -ELOOP.
> 
> Fixes: dbd29e075228 ("vringh: Fix loop descriptors check in the indirect cases")
> Cc: Xie Yongji <xieyongji@bytedance.com>
> Cc: stable@vger.kernel.org
> Assisted-by: Hawkeye:GLM-5.3-flash
> Assisted-by: Qoder:Qwen3.8-Max
> Signed-off-by: Fang Xieyan <fangxy@xiaopeng.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922122955.69433-1-fangxy@xiaopeng.com?part=1

[PATCH 2/2] vringh: add regression test for cyclic indirect descriptor

[Fang Xieyan]

Add a case to tools/virtio/vringh_test.c that builds a top-level
indirect descriptor whose NEXT points back at itself and checks that
vringh_getdesc_user() rejects it with -ELOOP.

Without the preceding fix, the walk re-enters the same top-level
descriptor without making forward progress: count stays flat, so the
traversal limit is never reached and -ELOOP is never returned. With the
fix, the top-level count advances on each re-entry and
vringh_getdesc_user() returns -ELOOP once the traversal limit is reached.

Use index 1 rather than 0 for the self-cycle, since returning from an
indirect table is only performed for a positive up_next value. A
self-cycle at index 0 would instead terminate the walk and would not
reproduce the bug.

Assisted-by: Hawkeye:GLM-5.3-flash
Assisted-by: Qoder:Qwen3.8-Max
Signed-off-by: Fang Xieyan <fangxy@xiaopeng.com>
---
 tools/virtio/vringh_test.c | 41 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/tools/virtio/vringh_test.c b/tools/virtio/vringh_test.c
index 84961b9..2a5d7f7 100644
--- a/tools/virtio/vringh_test.c
+++ b/tools/virtio/vringh_test.c
@@ -458,6 +458,8 @@ int main(int argc, char *argv[])
 	int err;
 	unsigned i;
 	void *ret;
+	struct vring_desc *ind;
+	char *data;
 	bool (*getrange)(struct vringh *vrh, u64 addr, struct vringh_range *r);
 	bool fast_vringh = false, parallel = false;
 
@@ -755,6 +757,45 @@ int main(int argc, char *argv[])
 		vringh_iov_cleanup(&riov);
 	}
 
+	/*
+	 * Regression test: a top-level indirect descriptor whose NEXT
+	 * points back to itself must be rejected with -ELOOP instead of
+	 * looping forever. Use index 1 rather than 0 so that returning
+	 * from the indirect table re-enters the same top-level descriptor.
+	 */
+	ind = __user_addr_max - USER_MEM/2;
+	data = __user_addr_max - USER_MEM/4;
+
+	/* Fresh ring and host state; resets last_avail_idx to 0. */
+	vring_init(&vrh.vring, RINGSIZE, __user_addr_min, ALIGN);
+	vringh_init_user(&vrh, vdev.features, RINGSIZE, true,
+			 vrh.vring.desc, vrh.vring.avail, vrh.vring.used);
+
+	/* Single-entry indirect table pointing at valid data. */
+	ind[0].addr = (unsigned long)data;
+	ind[0].len = 1;
+	ind[0].flags = 0;
+
+	/* Top-level desc[1]: INDIRECT, and NEXT loops back to itself. */
+	vrh.vring.desc[1].addr = (unsigned long)ind;
+	vrh.vring.desc[1].len = sizeof(*ind);
+	vrh.vring.desc[1].flags = VRING_DESC_F_INDIRECT | VRING_DESC_F_NEXT;
+	vrh.vring.desc[1].next = 1;
+
+	/* Publish head 1 on the avail ring. */
+	vrh.vring.avail->ring[0] = 1;
+	vrh.vring.avail->idx = 1;
+
+	vringh_iov_init(&riov, host_riov, ARRAY_SIZE(host_riov));
+	vringh_iov_init(&wiov, host_wiov, ARRAY_SIZE(host_wiov));
+
+	err = vringh_getdesc_user(&vrh, &riov, &wiov, getrange, &head);
+	if (err != -ELOOP)
+		errx(1, "self-referential indirect: %i not -ELOOP", err);
+
+	vringh_iov_cleanup(&riov);
+	vringh_iov_cleanup(&wiov);
+
 	/* Don't leak memory... */
 	vring_del_virtqueue(vq);
 	free(__user_addr_min);
-- 
2.50.1

Re: [PATCH 2/2] vringh: add regression test for cyclic indirect descriptor

[sashiko-bot]

> Add a case to tools/virtio/vringh_test.c that builds a top-level
> indirect descriptor whose NEXT points back at itself and checks that
> vringh_getdesc_user() rejects it with -ELOOP.
> 
> Without the preceding fix, the walk re-enters the same top-level
> descriptor without making forward progress: count stays flat, so the
> traversal limit is never reached and -ELOOP is never returned. With the
> fix, the top-level count advances on each re-entry and
> vringh_getdesc_user() returns -ELOOP once the traversal limit is reached.
> 
> Use index 1 rather than 0 for the self-cycle, since returning from an
> indirect table is only performed for a positive up_next value. A
> self-cycle at index 0 would instead terminate the walk and would not
> reproduce the bug.
> 
> Assisted-by: Hawkeye:GLM-5.3-flash
> Assisted-by: Qoder:Qwen3.8-Max
> Signed-off-by: Fang Xieyan <fangxy@xiaopeng.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922122955.69433-1-fangxy@xiaopeng.com?part=2

Re: [PATCH 2/2] vringh: add regression test for cyclic indirect descriptor

[Jason Wang]

On Tue, Sep 22, 2026 at 8:30 PM Fang Xieyan <fangxy@xiaopeng.com> wrote:
>
> Add a case to tools/virtio/vringh_test.c that builds a top-level
> indirect descriptor whose NEXT points back at itself and checks that
> vringh_getdesc_user() rejects it with -ELOOP.
>
> Without the preceding fix, the walk re-enters the same top-level
> descriptor without making forward progress: count stays flat, so the
> traversal limit is never reached and -ELOOP is never returned. With the
> fix, the top-level count advances on each re-entry and
> vringh_getdesc_user() returns -ELOOP once the traversal limit is reached.
>
> Use index 1 rather than 0 for the self-cycle, since returning from an
> indirect table is only performed for a positive up_next value. A
> self-cycle at index 0 would instead terminate the walk and would not
> reproduce the bug.
>
> Assisted-by: Hawkeye:GLM-5.3-flash
> Assisted-by: Qoder:Qwen3.8-Max
> Signed-off-by: Fang Xieyan <fangxy@xiaopeng.com>
> ---
>  tools/virtio/vringh_test.c | 41 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>
> diff --git a/tools/virtio/vringh_test.c b/tools/virtio/vringh_test.c
> index 84961b9..2a5d7f7 100644
> --- a/tools/virtio/vringh_test.c
> +++ b/tools/virtio/vringh_test.c
> @@ -458,6 +458,8 @@ int main(int argc, char *argv[])
>         int err;
>         unsigned i;
>         void *ret;
> +       struct vring_desc *ind;
> +       char *data;
>         bool (*getrange)(struct vringh *vrh, u64 addr, struct vringh_range *r);
>         bool fast_vringh = false, parallel = false;
>
> @@ -755,6 +757,45 @@ int main(int argc, char *argv[])
>                 vringh_iov_cleanup(&riov);
>         }
>
> +       /*
> +        * Regression test: a top-level indirect descriptor whose NEXT
> +        * points back to itself must be rejected with -ELOOP instead of
> +        * looping forever. Use index 1 rather than 0 so that returning
> +        * from the indirect table re-enters the same top-level descriptor.
> +        */
> +       ind = __user_addr_max - USER_MEM/2;
> +       data = __user_addr_max - USER_MEM/4;
> +
> +       /* Fresh ring and host state; resets last_avail_idx to 0. */
> +       vring_init(&vrh.vring, RINGSIZE, __user_addr_min, ALIGN);
> +       vringh_init_user(&vrh, vdev.features, RINGSIZE, true,
> +                        vrh.vring.desc, vrh.vring.avail, vrh.vring.used);
> +
> +       /* Single-entry indirect table pointing at valid data. */
> +       ind[0].addr = (unsigned long)data;
> +       ind[0].len = 1;
> +       ind[0].flags = 0;
> +
> +       /* Top-level desc[1]: INDIRECT, and NEXT loops back to itself. */
> +       vrh.vring.desc[1].addr = (unsigned long)ind;
> +       vrh.vring.desc[1].len = sizeof(*ind);
> +       vrh.vring.desc[1].flags = VRING_DESC_F_INDIRECT | VRING_DESC_F_NEXT;
> +       vrh.vring.desc[1].next = 1;
> +
> +       /* Publish head 1 on the avail ring. */
> +       vrh.vring.avail->ring[0] = 1;
> +       vrh.vring.avail->idx = 1;
> +
> +       vringh_iov_init(&riov, host_riov, ARRAY_SIZE(host_riov));
> +       vringh_iov_init(&wiov, host_wiov, ARRAY_SIZE(host_wiov));
> +
> +       err = vringh_getdesc_user(&vrh, &riov, &wiov, getrange, &head);
> +       if (err != -ELOOP)
> +               errx(1, "self-referential indirect: %i not -ELOOP", err);
> +
> +       vringh_iov_cleanup(&riov);
> +       vringh_iov_cleanup(&wiov);
> +
>         /* Don't leak memory... */
>         vring_del_virtqueue(vq);
>         free(__user_addr_min);
> --
> 2.50.1
>

Acked-by: Jason Wang <jasowangio@gmail.com>

Thanks