[PATCH 0/2] syzbot reports on sqo_dead

[PATCH 0/2] syzbot reports on sqo_dead

[Pavel Begunkov]

It deals with two ->sqo_dead related bugs reported by syzbot. 1/2 is for
overlooked ->ring==NULL case. 2/2 is not a real problem but rather a
false positive, but still can backfire in the future.

Pavel Begunkov (2):
  io_uring: fix null-deref in io_disable_sqo_submit
  io_uring: do sqo disable on install_fd error

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

-- 
2.24.0

[PATCH 1/2] io_uring: fix null-deref in io_disable_sqo_submit

[Pavel Begunkov]

general protection fault, probably for non-canonical address
	0xdffffc0000000022: 0000 [#1] KASAN: null-ptr-deref
	in range [0x0000000000000110-0x0000000000000117]
RIP: 0010:io_ring_set_wakeup_flag fs/io_uring.c:6929 [inline]
RIP: 0010:io_disable_sqo_submit+0xdb/0x130 fs/io_uring.c:8891
Call Trace:
 io_uring_create fs/io_uring.c:9711 [inline]
 io_uring_setup+0x12b1/0x38e0 fs/io_uring.c:9739
 do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

io_disable_sqo_submit() might be called before user rings were
allocated, don't do io_ring_set_wakeup_flag() in those cases.

Reported-by: syzbot+ab412638aeb652ded540@syzkaller.appspotmail.com
Fixes: d9d05217cb69 ("io_uring: stop SQPOLL submit on creator's death")
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 fs/io_uring.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 2f305c097bd5..bf043c600e55 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -8888,7 +8888,8 @@ static void io_disable_sqo_submit(struct io_ring_ctx *ctx)
 	mutex_unlock(&ctx->uring_lock);
 
 	/* make sure callers enter the ring to get error */
-	io_ring_set_wakeup_flag(ctx);
+	if (ctx->rings)
+		io_ring_set_wakeup_flag(ctx);
 }
 
 /*
-- 
2.24.0

[PATCH 2/2] io_uring: do sqo disable on install_fd error

[Pavel Begunkov]

WARNING: CPU: 0 PID: 8494 at fs/io_uring.c:8717
	io_ring_ctx_wait_and_kill+0x4f2/0x600 fs/io_uring.c:8717
Call Trace:
 io_uring_release+0x3e/0x50 fs/io_uring.c:8759
 __fput+0x283/0x920 fs/file_table.c:280
 task_work_run+0xdd/0x190 kernel/task_work.c:140
 tracehook_notify_resume include/linux/tracehook.h:189 [inline]
 exit_to_user_mode_loop kernel/entry/common.c:174 [inline]
 exit_to_user_mode_prepare+0x249/0x250 kernel/entry/common.c:201
 __syscall_exit_to_user_mode_work kernel/entry/common.c:291 [inline]
 syscall_exit_to_user_mode+0x19/0x50 kernel/entry/common.c:302
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

failed io_uring_install_fd() is a special case, we don't do
io_ring_ctx_wait_and_kill() directly but defer it to fput, though still
need to io_disable_sqo_submit() before.

note: it doesn't fix any real problem, just a warning. That's because
sqring won't be available to the userspace in this case and so SQPOLL
won't submit anything.

Reported-by: syzbot+9c9c35374c0ecac06516@syzkaller.appspotmail.com
Fixes: d9d05217cb69 ("io_uring: stop SQPOLL submit on creator's death")
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 fs/io_uring.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index bf043c600e55..81a7ec036330 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -9701,6 +9701,7 @@ static int io_uring_create(unsigned entries, struct io_uring_params *p,
 	 */
 	ret = io_uring_install_fd(ctx, file);
 	if (ret < 0) {
+		io_disable_sqo_submit(ctx);
 		/* fput will clean it up */
 		fput(file);
 		return ret;
-- 
2.24.0

Re: [PATCH 0/2] syzbot reports on sqo_dead

[Jens Axboe]

On 1/13/21 5:42 AM, Pavel Begunkov wrote:
> It deals with two ->sqo_dead related bugs reported by syzbot. 1/2 is for
> overlooked ->ring==NULL case. 2/2 is not a real problem but rather a
> false positive, but still can backfire in the future.
> 
> Pavel Begunkov (2):
>   io_uring: fix null-deref in io_disable_sqo_submit
>   io_uring: do sqo disable on install_fd error

Looks good, thanks. Applied.

-- 
Jens Axboe