* [PATCH for-next 1/1] io_uring/net: fix cleanup double free free_iov init
@ 2022-09-26 13:35 Pavel Begunkov
2022-09-26 14:37 ` Jens Axboe
2022-12-19 10:23 ` User-triggerable 6.1 crash [was: io_uring/net: fix cleanup double free free_iov init] Jiri Slaby
0 siblings, 2 replies; 6+ messages in thread
From: Pavel Begunkov @ 2022-09-26 13:35 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, asml.silence, syzbot+edfd15cd4246a3fc615a
Having ->async_data doesn't mean it's initialised and previously we vere
relying on setting F_CLEANUP at the right moment. With zc sendmsg
though, we set F_CLEANUP early in prep when we alloc a notif and so we
may allocate async_data, fail in copy_msg_hdr() leaving
struct io_async_msghdr not initialised correctly but with F_CLEANUP
set, which causes a ->free_iov double free and probably other nastiness.
Always initialise ->free_iov. Also, now it might point to fast_iov when
fails, so avoid freeing it during cleanups.
Reported-by: syzbot+edfd15cd4246a3fc615a@syzkaller.appspotmail.com
Fixes: 493108d95f146 ("io_uring/net: zerocopy sendmsg")
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
io_uring/net.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/io_uring/net.c b/io_uring/net.c
index 2af56661590a..6b69eff6887e 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -124,20 +124,22 @@ static struct io_async_msghdr *io_msg_alloc_async(struct io_kiocb *req,
{
struct io_ring_ctx *ctx = req->ctx;
struct io_cache_entry *entry;
+ struct io_async_msghdr *hdr;
if (!(issue_flags & IO_URING_F_UNLOCKED) &&
(entry = io_alloc_cache_get(&ctx->netmsg_cache)) != NULL) {
- struct io_async_msghdr *hdr;
-
hdr = container_of(entry, struct io_async_msghdr, cache);
+ hdr->free_iov = NULL;
req->flags |= REQ_F_ASYNC_DATA;
req->async_data = hdr;
return hdr;
}
- if (!io_alloc_async_data(req))
- return req->async_data;
-
+ if (!io_alloc_async_data(req)) {
+ hdr = req->async_data;
+ hdr->free_iov = NULL;
+ return hdr;
+ }
return NULL;
}
@@ -192,7 +194,6 @@ int io_send_prep_async(struct io_kiocb *req)
io = io_msg_alloc_async_prep(req);
if (!io)
return -ENOMEM;
- io->free_iov = NULL;
ret = move_addr_to_kernel(zc->addr, zc->addr_len, &io->addr);
return ret;
}
@@ -209,7 +210,6 @@ static int io_setup_async_addr(struct io_kiocb *req,
io = io_msg_alloc_async(req, issue_flags);
if (!io)
return -ENOMEM;
- io->free_iov = NULL;
memcpy(&io->addr, addr_storage, sizeof(io->addr));
return -EAGAIN;
}
@@ -479,7 +479,6 @@ static int __io_compat_recvmsg_copy_hdr(struct io_kiocb *req,
if (msg.msg_iovlen == 0) {
sr->len = 0;
- iomsg->free_iov = NULL;
} else if (msg.msg_iovlen > 1) {
return -EINVAL;
} else {
@@ -490,7 +489,6 @@ static int __io_compat_recvmsg_copy_hdr(struct io_kiocb *req,
if (clen < 0)
return -EINVAL;
sr->len = clen;
- iomsg->free_iov = NULL;
}
if (req->flags & REQ_F_APOLL_MULTISHOT) {
@@ -913,7 +911,9 @@ void io_send_zc_cleanup(struct io_kiocb *req)
if (req_has_async_data(req)) {
io = req->async_data;
- kfree(io->free_iov);
+ /* might be ->fast_iov if *msg_copy_hdr failed */
+ if (io->free_iov != io->fast_iov)
+ kfree(io->free_iov);
}
if (zc->notif) {
zc->notif->flags |= REQ_F_CQE_SKIP;
--
2.37.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH for-next 1/1] io_uring/net: fix cleanup double free free_iov init 2022-09-26 13:35 [PATCH for-next 1/1] io_uring/net: fix cleanup double free free_iov init Pavel Begunkov @ 2022-09-26 14:37 ` Jens Axboe 2022-12-19 10:23 ` User-triggerable 6.1 crash [was: io_uring/net: fix cleanup double free free_iov init] Jiri Slaby 1 sibling, 0 replies; 6+ messages in thread From: Jens Axboe @ 2022-09-26 14:37 UTC (permalink / raw) To: Pavel Begunkov, io-uring; +Cc: syzbot+edfd15cd4246a3fc615a On 9/26/22 7:35 AM, Pavel Begunkov wrote: > Having ->async_data doesn't mean it's initialised and previously we vere > relying on setting F_CLEANUP at the right moment. With zc sendmsg > though, we set F_CLEANUP early in prep when we alloc a notif and so we > may allocate async_data, fail in copy_msg_hdr() leaving > struct io_async_msghdr not initialised correctly but with F_CLEANUP > set, which causes a ->free_iov double free and probably other nastiness. > > Always initialise ->free_iov. Also, now it might point to fast_iov when > fails, so avoid freeing it during cleanups. APplied, thanks. -- Jens Axboe ^ permalink raw reply [flat|nested] 6+ messages in thread
* User-triggerable 6.1 crash [was: io_uring/net: fix cleanup double free free_iov init] 2022-09-26 13:35 [PATCH for-next 1/1] io_uring/net: fix cleanup double free free_iov init Pavel Begunkov 2022-09-26 14:37 ` Jens Axboe @ 2022-12-19 10:23 ` Jiri Slaby 2022-12-19 12:32 ` Jiri Slaby 2022-12-19 13:41 ` Jens Axboe 1 sibling, 2 replies; 6+ messages in thread From: Jiri Slaby @ 2022-12-19 10:23 UTC (permalink / raw) To: Pavel Begunkov, io-uring; +Cc: Jens Axboe, syzbot+edfd15cd4246a3fc615a On 26. 09. 22, 15:35, Pavel Begunkov wrote: > Having ->async_data doesn't mean it's initialised and previously we vere > relying on setting F_CLEANUP at the right moment. With zc sendmsg > though, we set F_CLEANUP early in prep when we alloc a notif and so we > may allocate async_data, fail in copy_msg_hdr() leaving > struct io_async_msghdr not initialised correctly but with F_CLEANUP > set, which causes a ->free_iov double free and probably other nastiness. > > Always initialise ->free_iov. Also, now it might point to fast_iov when > fails, so avoid freeing it during cleanups. > > Reported-by: syzbot+edfd15cd4246a3fc615a@syzkaller.appspotmail.com > Fixes: 493108d95f146 ("io_uring/net: zerocopy sendmsg") > Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> Hi, it's rather easy to crash 6.1 with this patch now. Compile liburing-2.2/test/send_recvmsg.c with -m32, run it as an ordinary user and see the below WARNING followed by many BUGs. It dies in this kfree() in io_recvmsg(): if (mshot_finished) { io_netmsg_recycle(req, issue_flags); /* fast path, check for non-NULL to avoid function call */ if (kmsg->free_iov) kfree(kmsg->free_iov); req->flags &= ~REQ_F_NEED_CLEANUP; } WARNING: CPU: 1 PID: 739 at mm/slub.c:3567 kfree (mm/slub.c:3567 mm/slub.c:4558) Modules linked in: CPU: 1 PID: 739 Comm: send_recvmsg.t Not tainted 6.0.0-rc6-default+ #31 090abe0ed83c945329aed053e1acb9f3614bf165 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015 RIP: 0010:kfree (mm/slub.c:3567 mm/slub.c:4558) Code: 68 fd ff ff 41 f7 c4 ff 0f 00 00 75 be 49 8b 04 24 a9 00 00 01 00 74 b3 49 8b 44 24 48 a8 01 74 aa 48 83 e8 01 49 39 c4 74 a1 <0f> 0b 80 3d 2e 14 9a 01 00 0f 84 3a 39 8f 00 be 00 f0 ff ff 31 ed All code ======== 0: 68 fd ff ff 41 push $0x41fffffd 5: f7 c4 ff 0f 00 00 test $0xfff,%esp b: 75 be jne 0xffffffffffffffcb d: 49 8b 04 24 mov (%r12),%rax 11: a9 00 00 01 00 test $0x10000,%eax 16: 74 b3 je 0xffffffffffffffcb 18: 49 8b 44 24 48 mov 0x48(%r12),%rax 1d: a8 01 test $0x1,%al 1f: 74 aa je 0xffffffffffffffcb 21: 48 83 e8 01 sub $0x1,%rax 25: 49 39 c4 cmp %rax,%r12 28: 74 a1 je 0xffffffffffffffcb 2a:* 0f 0b ud2 <-- trapping instruction 2c: 80 3d 2e 14 9a 01 00 cmpb $0x0,0x19a142e(%rip) # 0x19a1461 33: 0f 84 3a 39 8f 00 je 0x8f3973 39: be 00 f0 ff ff mov $0xfffff000,%esi 3e: 31 ed xor %ebp,%ebp Code starting with the faulting instruction =========================================== 0: 0f 0b ud2 2: 80 3d 2e 14 9a 01 00 cmpb $0x0,0x19a142e(%rip) # 0x19a1437 9: 0f 84 3a 39 8f 00 je 0x8f3949 f: be 00 f0 ff ff mov $0xfffff000,%esi 14: 31 ed xor %ebp,%ebp RSP: 0018:ffffbce7c0e17980 EFLAGS: 00010246 RAX: 0017ffffc0001000 RBX: ffffffffbd06ea69 RCX: ffff9ccb84548c00 RDX: ffffeae904969b88 RSI: ffffeae900000000 RDI: ffffffffbd06ea69 RBP: ffffbce7c0e17c20 R08: 0000000000000035 R09: 0000000080100002 R10: 0000000000000001 R11: 0000000000000001 R12: ffffeae904969b80 R13: 0000000000590005 R14: ffff9ccb8aeedd00 R15: ffff9ccb84548c00 FS: 0000000000000000(0000) GS:ffff9ccbc0c80000(0063) knlGS:00000000f7bffb40 CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033 CR2: 00000000f7d51830 CR3: 000000010177c000 CR4: 00000000000006e0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: <TASK> io_recvmsg (io_uring/net.c:809) io_issue_sqe (io_uring/io_uring.c:1740) io_req_task_submit (io_uring/io_uring.c:1920 io_uring/io_uring.c:1258) handle_tw_list (io_uring/io_uring.c:1023) tctx_task_work (io_uring/io_uring.c:1072 io_uring/io_uring.c:1086) task_work_run (include/linux/sched.h:2056 (discriminator 1) kernel/task_work.c:179 (discriminator 1)) io_run_task_work_sig (io_uring/io_uring.h:242 io_uring/io_uring.h:260 io_uring/io_uring.c:2349) __do_sys_io_uring_enter (io_uring/io_uring.c:2365 io_uring/io_uring.c:2446 io_uring/io_uring.c:3261) __do_fast_syscall_32 (arch/x86/entry/common.c:112 arch/x86/entry/common.c:178) do_fast_syscall_32 (arch/x86/entry/common.c:203) entry_SYSENTER_compat_after_hwframe (arch/x86/entry/entry_64_compat.S:122) RIP: 0023:0xf7fb0549 Code: 03 74 c0 01 10 05 03 74 b8 01 10 06 03 74 b4 01 10 07 03 74 b0 01 10 08 03 74 d8 01 00 00 00 00 00 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 c3 cc 90 90 90 8d b4 26 00 00 00 00 8d b4 26 00 00 00 00 All code ======== 0: 03 74 c0 01 add 0x1(%rax,%rax,8),%esi 4: 10 05 03 74 b8 01 adc %al,0x1b87403(%rip) # 0x1b8740d a: 10 06 adc %al,(%rsi) c: 03 74 b4 01 add 0x1(%rsp,%rsi,4),%esi 10: 10 07 adc %al,(%rdi) 12: 03 74 b0 01 add 0x1(%rax,%rsi,4),%esi 16: 10 08 adc %cl,(%rax) 18: 03 74 d8 01 add 0x1(%rax,%rbx,8),%esi 1c: 00 00 add %al,(%rax) 1e: 00 00 add %al,(%rax) 20: 00 51 52 add %dl,0x52(%rcx) 23: 55 push %rbp 24: 89 e5 mov %esp,%ebp 26: 0f 34 sysenter 28: cd 80 int $0x80 2a:* 5d pop %rbp <-- trapping instruction 2b: 5a pop %rdx 2c: 59 pop %rcx 2d: c3 ret 2e: cc int3 2f: 90 nop 30: 90 nop 31: 90 nop 32: 8d b4 26 00 00 00 00 lea 0x0(%rsi,%riz,1),%esi 39: 8d b4 26 00 00 00 00 lea 0x0(%rsi,%riz,1),%esi Code starting with the faulting instruction =========================================== 0: 5d pop %rbp 1: 5a pop %rdx 2: 59 pop %rcx 3: c3 ret 4: cc int3 5: 90 nop 6: 90 nop 7: 90 nop 8: 8d b4 26 00 00 00 00 lea 0x0(%rsi,%riz,1),%esi f: 8d b4 26 00 00 00 00 lea 0x0(%rsi,%riz,1),%esi RSP: 002b:00000000f7bff10c EFLAGS: 00000206 ORIG_RAX: 00000000000001aa RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000000000 RDX: 0000000000000001 RSI: 0000000000000001 RDI: 0000000000000000 RBP: 0000000000000008 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000206 R12: 0000000000000000 R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 </TASK> > --- > io_uring/net.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/io_uring/net.c b/io_uring/net.c > index 2af56661590a..6b69eff6887e 100644 > --- a/io_uring/net.c > +++ b/io_uring/net.c > @@ -124,20 +124,22 @@ static struct io_async_msghdr *io_msg_alloc_async(struct io_kiocb *req, > { > struct io_ring_ctx *ctx = req->ctx; > struct io_cache_entry *entry; > + struct io_async_msghdr *hdr; > > if (!(issue_flags & IO_URING_F_UNLOCKED) && > (entry = io_alloc_cache_get(&ctx->netmsg_cache)) != NULL) { > - struct io_async_msghdr *hdr; > - > hdr = container_of(entry, struct io_async_msghdr, cache); > + hdr->free_iov = NULL; > req->flags |= REQ_F_ASYNC_DATA; > req->async_data = hdr; > return hdr; > } > > - if (!io_alloc_async_data(req)) > - return req->async_data; > - > + if (!io_alloc_async_data(req)) { > + hdr = req->async_data; > + hdr->free_iov = NULL; > + return hdr; > + } > return NULL; > } > > @@ -192,7 +194,6 @@ int io_send_prep_async(struct io_kiocb *req) > io = io_msg_alloc_async_prep(req); > if (!io) > return -ENOMEM; > - io->free_iov = NULL; > ret = move_addr_to_kernel(zc->addr, zc->addr_len, &io->addr); > return ret; > } > @@ -209,7 +210,6 @@ static int io_setup_async_addr(struct io_kiocb *req, > io = io_msg_alloc_async(req, issue_flags); > if (!io) > return -ENOMEM; > - io->free_iov = NULL; > memcpy(&io->addr, addr_storage, sizeof(io->addr)); > return -EAGAIN; > } > @@ -479,7 +479,6 @@ static int __io_compat_recvmsg_copy_hdr(struct io_kiocb *req, > > if (msg.msg_iovlen == 0) { > sr->len = 0; > - iomsg->free_iov = NULL; > } else if (msg.msg_iovlen > 1) { > return -EINVAL; > } else { > @@ -490,7 +489,6 @@ static int __io_compat_recvmsg_copy_hdr(struct io_kiocb *req, > if (clen < 0) > return -EINVAL; > sr->len = clen; > - iomsg->free_iov = NULL; > } > > if (req->flags & REQ_F_APOLL_MULTISHOT) { > @@ -913,7 +911,9 @@ void io_send_zc_cleanup(struct io_kiocb *req) > > if (req_has_async_data(req)) { > io = req->async_data; > - kfree(io->free_iov); > + /* might be ->fast_iov if *msg_copy_hdr failed */ > + if (io->free_iov != io->fast_iov) > + kfree(io->free_iov); > } > if (zc->notif) { > zc->notif->flags |= REQ_F_CQE_SKIP; -- js ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: User-triggerable 6.1 crash [was: io_uring/net: fix cleanup double free free_iov init] 2022-12-19 10:23 ` User-triggerable 6.1 crash [was: io_uring/net: fix cleanup double free free_iov init] Jiri Slaby @ 2022-12-19 12:32 ` Jiri Slaby 2022-12-19 13:42 ` Pavel Begunkov 2022-12-19 13:41 ` Jens Axboe 1 sibling, 1 reply; 6+ messages in thread From: Jiri Slaby @ 2022-12-19 12:32 UTC (permalink / raw) To: Pavel Begunkov, io-uring; +Cc: Jens Axboe, syzbot+edfd15cd4246a3fc615a On 19. 12. 22, 11:23, Jiri Slaby wrote: > On 26. 09. 22, 15:35, Pavel Begunkov wrote: >> Having ->async_data doesn't mean it's initialised and previously we vere >> relying on setting F_CLEANUP at the right moment. With zc sendmsg >> though, we set F_CLEANUP early in prep when we alloc a notif and so we >> may allocate async_data, fail in copy_msg_hdr() leaving >> struct io_async_msghdr not initialised correctly but with F_CLEANUP >> set, which causes a ->free_iov double free and probably other nastiness. >> >> Always initialise ->free_iov. Also, now it might point to fast_iov when >> fails, so avoid freeing it during cleanups. >> >> Reported-by: syzbot+edfd15cd4246a3fc615a@syzkaller.appspotmail.com >> Fixes: 493108d95f146 ("io_uring/net: zerocopy sendmsg") >> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> > > Hi, > > it's rather easy to crash 6.1 with this patch now. Compile > liburing-2.2/test/send_recvmsg.c with -m32, run it as an ordinary user > and see the below WARNING followed by many BUGs. > > It dies in this kfree() in io_recvmsg(): > if (mshot_finished) { > io_netmsg_recycle(req, issue_flags); > /* fast path, check for non-NULL to avoid function call */ > if (kmsg->free_iov) > kfree(kmsg->free_iov); > req->flags &= ~REQ_F_NEED_CLEANUP; > } I am attaching a KASAN report instead: BUG: KASAN: invalid-free in __kmem_cache_free (mm/slub.c:3661 mm/slub.c:3674) Free of addr ffff8881049ff328 by task send_recvmsg.t/733 CPU: 3 PID: 733 Comm: send_recvmsg.t Not tainted 6.1.0-default #10 fd86e1993b1e4baedde4c3e698b88971c38217a0 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015 Call Trace: <TASK> dump_stack_lvl (lib/dump_stack.c:107 (discriminator 1)) print_report (mm/kasan/report.c:285 mm/kasan/report.c:395) kasan_report_invalid_free (mm/kasan/report.c:162 mm/kasan/report.c:462) ____kasan_slab_free (mm/kasan/common.c:217) slab_free_freelist_hook (mm/slub.c:1750) __kmem_cache_free (mm/slub.c:3661 mm/slub.c:3674) io_recvmsg (io_uring/net.c:812) io_issue_sqe (include/linux/audit.h:314 include/linux/audit.h:339 io_uring/io_uring.c:1746) io_req_task_submit (io_uring/io_uring.c:1922 io_uring/io_uring.c:1264) handle_tw_list (io_uring/io_uring.c:1028) tctx_task_work (include/linux/instrumented.h:87 io_uring/io_uring.c:1077 io_uring/io_uring.c:1091) task_work_run (kernel/task_work.c:180 (discriminator 1)) io_run_task_work_sig (io_uring/io_uring.h:250 io_uring/io_uring.h:239 io_uring/io_uring.h:272 io_uring/io_uring.c:2351) __do_sys_io_uring_enter (io_uring/io_uring.c:2367 io_uring/io_uring.c:2449 io_uring/io_uring.c:3267) __do_fast_syscall_32 (arch/x86/entry/common.c:112 arch/x86/entry/common.c:178) do_fast_syscall_32 (arch/x86/entry/common.c:203) entry_SYSENTER_compat_after_hwframe (arch/x86/entry/entry_64_compat.S:122) RIP: 0023:0xf7f6d549 Code: 03 74 c0 01 10 05 03 74 b8 01 10 06 03 74 b4 01 10 07 03 74 b0 01 10 08 03 74 d8 01 00 00 00 00 00 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 c3 cc 90 90 90 8d b4 26 00 00 00 00 8d b4 26 00 00 00 00 All code ======== 0: 03 74 c0 01 add 0x1(%rax,%rax,8),%esi 4: 10 05 03 74 b8 01 adc %al,0x1b87403(%rip) # 0x1b8740d a: 10 06 adc %al,(%rsi) c: 03 74 b4 01 add 0x1(%rsp,%rsi,4),%esi 10: 10 07 adc %al,(%rdi) 12: 03 74 b0 01 add 0x1(%rax,%rsi,4),%esi 16: 10 08 adc %cl,(%rax) 18: 03 74 d8 01 add 0x1(%rax,%rbx,8),%esi 1c: 00 00 add %al,(%rax) 1e: 00 00 add %al,(%rax) 20: 00 51 52 add %dl,0x52(%rcx) 23: 55 push %rbp 24: 89 e5 mov %esp,%ebp 26: 0f 34 sysenter 28: cd 80 int $0x80 2a:* 5d pop %rbp <-- trapping instruction 2b: 5a pop %rdx 2c: 59 pop %rcx 2d: c3 ret 2e: cc int3 2f: 90 nop 30: 90 nop 31: 90 nop 32: 8d b4 26 00 00 00 00 lea 0x0(%rsi,%riz,1),%esi 39: 8d b4 26 00 00 00 00 lea 0x0(%rsi,%riz,1),%esi Code starting with the faulting instruction =========================================== 0: 5d pop %rbp 1: 5a pop %rdx 2: 59 pop %rcx 3: c3 ret 4: cc int3 5: 90 nop 6: 90 nop 7: 90 nop 8: 8d b4 26 00 00 00 00 lea 0x0(%rsi,%riz,1),%esi f: 8d b4 26 00 00 00 00 lea 0x0(%rsi,%riz,1),%esi RSP: 002b:00000000f7bff10c EFLAGS: 00000206 ORIG_RAX: 00000000000001aa RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000000000 RDX: 0000000000000001 RSI: 0000000000000001 RDI: 0000000000000000 RBP: 0000000000000008 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000206 R12: 0000000000000000 R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 </TASK> Allocated by task 733: kasan_save_stack (mm/kasan/common.c:46) kasan_set_track (mm/kasan/common.c:52) __kasan_slab_alloc (mm/kasan/common.c:325) kmem_cache_alloc (mm/slab.h:737 mm/slub.c:3398 mm/slub.c:3406 mm/slub.c:3413 mm/slub.c:3422) sk_prot_alloc (net/core/sock.c:2024) sk_alloc (net/core/sock.c:2083) inet_create (net/ipv4/af_inet.c:319 net/ipv4/af_inet.c:245) __sock_create (net/socket.c:1516) __sys_socket (net/socket.c:1605 net/socket.c:1588 net/socket.c:1636) __ia32_compat_sys_socketcall (net/compat.c:447 net/compat.c:421 net/compat.c:421) __do_fast_syscall_32 (arch/x86/entry/common.c:112 arch/x86/entry/common.c:178) do_fast_syscall_32 (arch/x86/entry/common.c:203) entry_SYSENTER_compat_after_hwframe (arch/x86/entry/entry_64_compat.S:122) The buggy address belongs to the object at ffff8881049ff300 which belongs to the cache UDP of size 1152 The buggy address is located 40 bytes inside of 1152-byte region [ffff8881049ff300, ffff8881049ff780) The buggy address belongs to the physical page: page:000000005bc2cfe2 refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff8881049fe400 pfn:0x1049f8 head:000000005bc2cfe2 order:3 compound_mapcount:0 compound_pincount:0 memcg:ffff888107526401 flags: 0x17ffffc0010200(slab|head|node=0|zone=2|lastcpupid=0x1fffff) raw: 0017ffffc0010200 0000000000000000 dead000000000122 ffff8881018cc140 raw: ffff8881049fe400 0000000080190018 00000001ffffffff ffff888107526401 page dumped because: kasan: bad access detected Memory state around the buggy address: ffff8881049ff200: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc ffff8881049ff280: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc >ffff8881049ff300: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ^ ffff8881049ff380: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ffff8881049ff400: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ========================================================= -- js suse labs ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: User-triggerable 6.1 crash [was: io_uring/net: fix cleanup double free free_iov init] 2022-12-19 12:32 ` Jiri Slaby @ 2022-12-19 13:42 ` Pavel Begunkov 0 siblings, 0 replies; 6+ messages in thread From: Pavel Begunkov @ 2022-12-19 13:42 UTC (permalink / raw) To: Jiri Slaby, io-uring; +Cc: Jens Axboe, syzbot+edfd15cd4246a3fc615a On 12/19/22 12:32, Jiri Slaby wrote: > On 19. 12. 22, 11:23, Jiri Slaby wrote: >> On 26. 09. 22, 15:35, Pavel Begunkov wrote: >>> Having ->async_data doesn't mean it's initialised and previously we vere >>> relying on setting F_CLEANUP at the right moment. With zc sendmsg >>> though, we set F_CLEANUP early in prep when we alloc a notif and so we >>> may allocate async_data, fail in copy_msg_hdr() leaving >>> struct io_async_msghdr not initialised correctly but with F_CLEANUP >>> set, which causes a ->free_iov double free and probably other nastiness. >>> >>> Always initialise ->free_iov. Also, now it might point to fast_iov when >>> fails, so avoid freeing it during cleanups. >>> >>> Reported-by: syzbot+edfd15cd4246a3fc615a@syzkaller.appspotmail.com >>> Fixes: 493108d95f146 ("io_uring/net: zerocopy sendmsg") >>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> >> >> Hi, >> >> it's rather easy to crash 6.1 with this patch now. Compile liburing-2.2/test/send_recvmsg.c with -m32, run it as an ordinary user and see the below WARNING followed by many BUGs. >> >> It dies in this kfree() in io_recvmsg(): >> if (mshot_finished) { >> io_netmsg_recycle(req, issue_flags); >> /* fast path, check for non-NULL to avoid function call */ >> if (kmsg->free_iov) >> kfree(kmsg->free_iov); >> req->flags &= ~REQ_F_NEED_CLEANUP; >> } > > I am attaching a KASAN report instead: > > BUG: KASAN: invalid-free in __kmem_cache_free (mm/slub.c:3661 mm/slub.c:3674) > Free of addr ffff8881049ff328 by task send_recvmsg.t/733 Thanks for letting us know, I'll take a look -- Pavel Begunkov ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: User-triggerable 6.1 crash [was: io_uring/net: fix cleanup double free free_iov init] 2022-12-19 10:23 ` User-triggerable 6.1 crash [was: io_uring/net: fix cleanup double free free_iov init] Jiri Slaby 2022-12-19 12:32 ` Jiri Slaby @ 2022-12-19 13:41 ` Jens Axboe 1 sibling, 0 replies; 6+ messages in thread From: Jens Axboe @ 2022-12-19 13:41 UTC (permalink / raw) To: Jiri Slaby, Pavel Begunkov, io-uring; +Cc: syzbot+edfd15cd4246a3fc615a On 12/19/22 3:23 AM, Jiri Slaby wrote: > On 26. 09. 22, 15:35, Pavel Begunkov wrote: >> Having ->async_data doesn't mean it's initialised and previously we vere >> relying on setting F_CLEANUP at the right moment. With zc sendmsg >> though, we set F_CLEANUP early in prep when we alloc a notif and so we >> may allocate async_data, fail in copy_msg_hdr() leaving >> struct io_async_msghdr not initialised correctly but with F_CLEANUP >> set, which causes a ->free_iov double free and probably other nastiness. >> >> Always initialise ->free_iov. Also, now it might point to fast_iov when >> fails, so avoid freeing it during cleanups. >> >> Reported-by: syzbot+edfd15cd4246a3fc615a@syzkaller.appspotmail.com >> Fixes: 493108d95f146 ("io_uring/net: zerocopy sendmsg") >> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> > > Hi, > > it's rather easy to crash 6.1 with this patch now. Compile liburing-2.2/test/send_recvmsg.c with -m32, run it as an ordinary user and see the below WARNING followed by many BUGs. I'll take a look at this. -- Jens Axboe ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-12-19 13:43 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-09-26 13:35 [PATCH for-next 1/1] io_uring/net: fix cleanup double free free_iov init Pavel Begunkov 2022-09-26 14:37 ` Jens Axboe 2022-12-19 10:23 ` User-triggerable 6.1 crash [was: io_uring/net: fix cleanup double free free_iov init] Jiri Slaby 2022-12-19 12:32 ` Jiri Slaby 2022-12-19 13:42 ` Pavel Begunkov 2022-12-19 13:41 ` Jens Axboe
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.