public inbox for [email protected]
 help / color / mirror / Atom feed
* [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: [email protected]
Fixes: 493108d95f146 ("io_uring/net: zerocopy sendmsg")
Signed-off-by: Pavel Begunkov <[email protected]>
---
 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: [email protected]
> Fixes: 493108d95f146 ("io_uring/net: zerocopy sendmsg")
> Signed-off-by: Pavel Begunkov <[email protected]>

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: [email protected]
>> Fixes: 493108d95f146 ("io_uring/net: zerocopy sendmsg")
>> Signed-off-by: Pavel Begunkov <[email protected]>
> 
> 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 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: [email protected]
>> Fixes: 493108d95f146 ("io_uring/net: zerocopy sendmsg")
>> Signed-off-by: Pavel Begunkov <[email protected]>
> 
> 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

* 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: [email protected]
>>> Fixes: 493108d95f146 ("io_uring/net: zerocopy sendmsg")
>>> Signed-off-by: Pavel Begunkov <[email protected]>
>>
>> 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

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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox