operation completes after ring fd closed?

[erenon]

Given an async recvmsg operation, that references userspace memory. Is it possible, that the receive will complete slightly after the fd of the ring itself is closed, and the kernel modifies the userspace memory? I'm interested in both sqpoll on and off. I suspect I'm observing such a thing with sqpoll off. Linux 6.14.

[erenon]

Rust reproducer: https://github.com/erenon/write_after_close

[axboe]

Generally, everything post close should be canceled, but waiting on a completion for said events might be prudent. I'll take a look at this case, thanks a lot for making a reproducer!

[axboe]

Aha, caught it in action:

[    9.671787] WARNING: CPU: 7 PID: 831 at io_uring/net.c:1189 io_recv+0xa70/0xea8
[    9.672558] Modules linked in:
[    9.672794] CPU: 7 UID: 1000 PID: 831 Comm: write_after_clo Not tainted 6.17.0-rc6-00043-g8b789f2b7602-dirty #1884 NONE 
[    9.673617] Hardware name: linux,dummy-virt (DT)
[    9.673983] pstate: 11400005 (nzcV daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
[    9.674515] pc : io_recv+0xa70/0xea8
[    9.674809] lr : __io_issue_sqe+0xa8/0x758
[    9.675138] sp : ffff80008a607520
[    9.675363] x29: ffff80008a607650 x28: ffff000009e1bb80 x27: ffff000009e1bbd8
[    9.675907] x26: ffff00010b2ad800 x25: ffff000009e1bbc8 x24: 1fffe000013c3770
[    9.676422] x23: 0000000000000000 x22: 0000000080000001 x21: 0000000000000000
[    9.676948] x20: 0000000000000000 x19: ffff0000cfe73000 x18: ffff000009124418
[    9.677463] x17: 0000000000000000 x16: 1fffe00001224892 x15: ffff000009124430
[    9.677987] x14: 0000000000000000 x13: ffff00010ba1d000 x12: ffff6000013c3789
[    9.678492] x11: 1fffe000013c3788 x10: ffff6000013c3788 x9 : ffff800081baacd8
[    9.678999] x8 : 00009ffffec3c878 x7 : ffff000009e1bc43 x6 : 0000000000000001
[    9.679558] x5 : 0000000080000001 x4 : ffff800081c1fdb0 x3 : 00000000f2f2f200
[    9.680124] x2 : 1fffe00019fce603 x1 : 0000000000000000 x0 : 00007dfe68e28073
[    9.680686] Call trace:
[    9.680888]  io_recv+0xa70/0xea8 (P)
[    9.681176]  __io_issue_sqe+0xa8/0x758
[    9.681475]  io_issue_sqe+0xd8/0x1490
[    9.681862]  io_req_task_submit+0x80/0xf0
[    9.682175]  io_poll_task_func+0x4e4/0x9f8
[    9.682484]  io_handle_tw_list+0x104/0x228
[    9.682793]  tctx_task_work_run+0x9c/0x3c0
[    9.683094]  tctx_task_work+0x84/0xc8
[    9.683365]  task_work_run+0x220/0x3a0
[    9.683647]  get_signal+0xd58/0x2558
[    9.683941]  do_signal+0x204/0x7c0
[    9.684194]  do_notify_resume+0x120/0x1d8
[    9.684495]  el0_svc+0x80/0x88
[    9.684739]  el0t_64_sync_handler+0x98/0xe0
[    9.685046]  el0t_64_sync+0x170/0x178

which is checking for a closed ring doing the recv.

[axboe]

This should fix it, I'll post it. If you want a Reported-by tag in the commit, please let me know what name/identity and email to use for that.

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 93633613a165..efd9812523db 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -1406,8 +1406,10 @@ static void io_req_task_cancel(struct io_kiocb *req, io_tw_token_t tw)
 
 void io_req_task_submit(struct io_kiocb *req, io_tw_token_t tw)
 {
-	io_tw_lock(req->ctx, tw);
-	if (unlikely(io_should_terminate_tw()))
+	struct io_ring_ctx *ctx = req->ctx;
+
+	io_tw_lock(ctx, tw);
+	if (unlikely(io_should_terminate_tw(ctx)))
 		io_req_defer_failed(req, -EFAULT);
 	else if (req->flags & REQ_F_FORCE_ASYNC)
 		io_queue_iowq(req);
diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h
index abc6de227f74..1880902be6fd 100644
--- a/io_uring/io_uring.h
+++ b/io_uring/io_uring.h
@@ -476,9 +476,9 @@ static inline bool io_allowed_run_tw(struct io_ring_ctx *ctx)
  * 2) PF_KTHREAD is set, in which case the invoker of the task_work is
  *    our fallback task_work.
  */
-static inline bool io_should_terminate_tw(void)
+static inline bool io_should_terminate_tw(struct io_ring_ctx *ctx)
 {
-	return current->flags & (PF_KTHREAD | PF_EXITING);
+	return (current->flags & (PF_KTHREAD | PF_EXITING)) || percpu_ref_is_dying(&ctx->refs);
 }
 
 static inline void io_req_queue_tw_complete(struct io_kiocb *req, s32 res)
diff --git a/io_uring/poll.c b/io_uring/poll.c
index c786e587563b..6090a26975d4 100644
--- a/io_uring/poll.c
+++ b/io_uring/poll.c
@@ -224,7 +224,7 @@ static int io_poll_check_events(struct io_kiocb *req, io_tw_token_t tw)
 {
 	int v;
 
-	if (unlikely(io_should_terminate_tw()))
+	if (unlikely(io_should_terminate_tw(req->ctx)))
 		return -ECANCELED;
 
 	do {
diff --git a/io_uring/timeout.c b/io_uring/timeout.c
index 7f13bfa9f2b6..17e3aab0af36 100644
--- a/io_uring/timeout.c
+++ b/io_uring/timeout.c
@@ -324,7 +324,7 @@ static void io_req_task_link_timeout(struct io_kiocb *req, io_tw_token_t tw)
 	int ret;
 
 	if (prev) {
-		if (!io_should_terminate_tw()) {
+		if (!io_should_terminate_tw(req->ctx)) {
 			struct io_cancel_data cd = {
 				.ctx		= req->ctx,
 				.data		= prev->cqe.user_data,
diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index 053bac89b6c0..213716e10d70 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -118,7 +118,7 @@ static void io_uring_cmd_work(struct io_kiocb *req, io_tw_token_t tw)
 	struct io_uring_cmd *ioucmd = io_kiocb_to_cmd(req, struct io_uring_cmd);
 	unsigned int flags = IO_URING_F_COMPLETE_DEFER;
 
-	if (io_should_terminate_tw())
+	if (io_should_terminate_tw(req->ctx))
 		flags |= IO_URING_F_TASK_DEAD;
 
 	/* task_work executor checks the deffered list completion */

[erenon]

Reported-by: Benedek Thaler thaler@thaler.hu

[axboe]

Patch sent out with that tag added, and you on the CC.

[erenon]

Thanks for the very quick fix.

[axboe]

Not a problem, with a reproducer all bugs are shallow and generally quick to fix! So thank you for providing that, it saves me so much time.

[axboe]

Going to close this one up. The fix will go upstream tomorrow and then bubble back to stable kernels. Note: you said you are running 6.14, which isn't a maintained stable kernel. 6.12 or 6.16 would be better targets, I always recommend later ones for obvious reasons.