[PATCH] io_uring/zcrx: don't clear master_ctx from the import path

[Woraphat Khiaodaeng <worapat.kd2@gmail.com>]

import_zcrx() attaches an existing ifq to another ring.  It never calls
zcrx_set_ring_ctx() and so never takes the ->master_ctx reference, but
its error path still passes @ctx to zcrx_unregister(), which clears
->master_ctx and drops its percpu_ref whenever ifq->master_ctx == ctx.

That condition is reachable.  A ring that registers an ifq with a
non-zero event type_mask gets ->master_ctx pointed at itself, and
nothing stops it from exporting that ifq with ZCRX_CTRL_EXPORT and
importing the resulting fd back into the same ring.  Failing the import
after the refcount bumps -- an argument page mapped PROT_READ makes the
copy_to_user() in import_zcrx() return -EFAULT -- then clears the
->master_ctx owned by the original registration, which is still live.

Refcounts stay balanced and nothing is freed early, so there is no
splat.  The ring silently stops receiving ZCRX_EVENT_ALLOC_FAIL and
ZCRX_EVENT_COPY: zcrx_send_notif() returns early on a NULL
->master_ctx, and ->master_ctx is only ever set on a freshly allocated
ifq, so it cannot be restored without tearing the ring down.

Pass NULL instead, matching zcrx_box_release() and the zcrx_export()
error path.  io_register_zcrx() only gets away with passing @ctx
because zcrx_set_ring_ctx() runs after its last goto err.

Fixes: 00d91481279f ("io_uring/zcrx: share an ifq between rings")
Signed-off-by: Woraphat Khiaodaeng <worapat.kd2@gmail.com>
---

Found by code inspection, then confirmed on v7.2-rc5 in a QEMU guest by
adding a pr_info() to zcrx_set_ring_ctx() and zcrx_unregister_user().
Writing A for the ring's io_ring_ctx:

Before, the failed self-import clears the binding:

  set_ring_ctx:    ctx=A           -> master_ctx = A
  unregister_user: ctx=A master=A  -> CLEARING   (failed self-import)
  unregister_user: ctx=A master=0                (ring teardown, nothing left)

After, it survives and is released where it should be:

  set_ring_ctx:    ctx=A           -> master_ctx = A
  unregister_user: ctx=0 master=A                (failed self-import)
  unregister_user: ctx=A master=A  -> CLEARING   (ring teardown)

io_uring_register() returns -EFAULT either way.

The reproducer below needs no hardware: ZCRX_REG_NODEV leaves ifq->dev
NULL, so no net device and no DMA mapping are involved.  Plain syscalls,
no liburing.  Run as root (CAP_NET_ADMIN).

Reproducer:

#define _GNU_SOURCE
#include <errno.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <sys/mman.h>
#include <sys/syscall.h>
#include <linux/io_uring.h>
#include <linux/io_uring/zcrx.h>

#define AREA_PAGES 64
#define RING_BYTES (1024 * 1024)

int main(void)
{
	struct io_uring_zcrx_ifq_reg reg, *ireg;
	struct io_uring_zcrx_area_reg area_reg;
	struct io_uring_region_desc region_reg;
	struct zcrx_event_desc ev;
	struct io_uring_params p;
	struct zcrx_ctrl ctrl;
	void *area, *ring, *ro;
	long ps = sysconf(_SC_PAGESIZE);
	int ringfd, boxfd, ret;

	memset(&p, 0, sizeof(p));
	p.flags = IORING_SETUP_DEFER_TASKRUN | IORING_SETUP_SINGLE_ISSUER |
		  IORING_SETUP_CQE32;
	ringfd = syscall(__NR_io_uring_setup, 8, &p);
	if (ringfd < 0)
		return perror("io_uring_setup"), 1;

	area = mmap(NULL, AREA_PAGES * ps, PROT_READ | PROT_WRITE,
		    MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
	ring = mmap(NULL, RING_BYTES, PROT_READ | PROT_WRITE,
		    MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);

	/* non-zero type_mask is what makes zcrx_set_ring_ctx() run */
	memset(&ev, 0, sizeof(ev));
	ev.user_data = 0x5a5a5a5a;
	ev.type_mask = 1U << ZCRX_EVENT_COPY;

	memset(&region_reg, 0, sizeof(region_reg));
	region_reg.size = RING_BYTES;
	region_reg.user_addr = (unsigned long)ring;
	region_reg.flags = IORING_MEM_REGION_TYPE_USER;

	memset(&area_reg, 0, sizeof(area_reg));
	area_reg.addr = (unsigned long)area;
	area_reg.len = AREA_PAGES * ps;

	memset(&reg, 0, sizeof(reg));
	reg.rq_entries = 64;
	reg.flags = ZCRX_REG_NODEV;
	reg.area_ptr = (unsigned long)&area_reg;
	reg.region_ptr = (unsigned long)&region_reg;
	reg.event_desc = (unsigned long)&ev;
	ret = syscall(__NR_io_uring_register, ringfd,
		      IORING_REGISTER_ZCRX_IFQ, &reg, 1);
	if (ret)
		return perror("register ifq"), 1;

	memset(&ctrl, 0, sizeof(ctrl));
	ctrl.zcrx_id = reg.zcrx_id;
	ctrl.op = ZCRX_CTRL_EXPORT;
	ret = syscall(__NR_io_uring_register, ringfd,
		      IORING_REGISTER_ZCRX_CTRL, &ctrl, 0);
	if (ret)
		return perror("zcrx export"), 1;
	boxfd = ctrl.zc_export.zcrx_fd;

	/* import back into the same ring, with an unwritable argument page */
	ro = mmap(NULL, ps, PROT_READ | PROT_WRITE,
		  MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
	ireg = ro;
	memset(ireg, 0, sizeof(*ireg));
	ireg->flags = ZCRX_REG_IMPORT;
	ireg->if_idx = boxfd;
	mprotect(ro, ps, PROT_READ);

	ret = syscall(__NR_io_uring_register, ringfd,
		      IORING_REGISTER_ZCRX_IFQ, ro, 1);
	printf("self-import: ret=%d errno=%d (%s)\n", ret, errno,
	       strerror(errno));	/* -1 / EFAULT */
	return 0;
}

 io_uring/zcrx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/io_uring/zcrx.c b/io_uring/zcrx.c
index 76b9b0d54af9e..fea2b27272ec2 100644
--- a/io_uring/zcrx.c
+++ b/io_uring/zcrx.c
@@ -808,7 +808,8 @@ static int import_zcrx(struct io_ring_ctx *ctx,
 	scoped_guard(mutex, &ctx->mmap_lock)
 		xa_erase(&ctx->zcrx_ctxs, id);
 err:
-	zcrx_unregister(ifq, ctx);
+	/* the import path never took the ->master_ctx ref, don't drop it */
+	zcrx_unregister(ifq, NULL);
 	return ret;
 }
 

base-commit: 11028ab62899e4191e074ee364c712b77823a9c4
-- 
2.45.4