io-uring.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [syzbot] KASAN: stack-out-of-bounds Read in iov_iter_revert
@ 2021-08-12 13:43 syzbot
  2021-08-12 14:30 ` Pavel Begunkov
  2021-08-23  0:24 ` [syzbot] KASAN: stack-out-of-bounds Read in iov_iter_revert Pavel Begunkov
  0 siblings, 2 replies; 22+ messages in thread
From: syzbot @ 2021-08-12 13:43 UTC (permalink / raw)
  To: asml.silence, axboe, io-uring, linux-kernel, syzkaller-bugs

Hello,

syzbot found the following issue on:

HEAD commit:    1746f4db5135 Merge tag 'orphans-v5.14-rc6' of git://git.ke..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=111065fa300000
kernel config:  https://syzkaller.appspot.com/x/.config?x=e3a20bae04b96ccd
dashboard link: https://syzkaller.appspot.com/bug?extid=9671693590ef5aad8953
compiler:       gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.1
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=119dcaf6300000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=120d216e300000

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+9671693590ef5aad8953@syzkaller.appspotmail.com

==================================================================
BUG: KASAN: stack-out-of-bounds in iov_iter_revert lib/iov_iter.c:1093 [inline]
BUG: KASAN: stack-out-of-bounds in iov_iter_revert+0x803/0x900 lib/iov_iter.c:1033
Read of size 8 at addr ffffc9000cf478b0 by task syz-executor673/8439

CPU: 0 PID: 8439 Comm: syz-executor673 Not tainted 5.14.0-rc5-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:105
 print_address_description.constprop.0.cold+0xf/0x309 mm/kasan/report.c:233
 __kasan_report mm/kasan/report.c:419 [inline]
 kasan_report.cold+0x83/0xdf mm/kasan/report.c:436
 iov_iter_revert lib/iov_iter.c:1093 [inline]
 iov_iter_revert+0x803/0x900 lib/iov_iter.c:1033
 io_write+0x57b/0xed0 fs/io_uring.c:3459
 io_issue_sqe+0x28c/0x6920 fs/io_uring.c:6181
 __io_queue_sqe+0x1ac/0xf00 fs/io_uring.c:6464
 io_queue_sqe fs/io_uring.c:6507 [inline]
 io_submit_sqe fs/io_uring.c:6662 [inline]
 io_submit_sqes+0x63ea/0x7bc0 fs/io_uring.c:6778
 __do_sys_io_uring_enter+0xb03/0x1d40 fs/io_uring.c:9389
 do_syscall_x64 arch/x86/entry/common.c:50 [inline]
 do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
 entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x43f8a9
Code: 28 c3 e8 1a 15 00 00 66 2e 0f 1f 84 00 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffcc6759968 EFLAGS: 00000246 ORIG_RAX: 00000000000001aa
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 000000000043f8a9
RDX: 0000000000000000 RSI: 00000000000052fe RDI: 0000000000000003
RBP: 00007ffcc6759988 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffcc6759990
R13: 0000000000000000 R14: 00000000004ae018 R15: 0000000000400488


addr ffffc9000cf478b0 is located in stack of task syz-executor673/8439 at offset 152 in frame:
 io_write+0x0/0xed0 fs/io_uring.c:3335

this frame has 3 objects:
 [48, 56) 'iovec'
 [80, 120) '__iter'
 [160, 288) 'inline_vecs'

Memory state around the buggy address:
 ffffc9000cf47780: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 ffffc9000cf47800: 00 00 00 f1 f1 f1 f1 00 00 00 f2 f2 f2 00 00 00
>ffffc9000cf47880: 00 00 f2 f2 f2 f2 f2 00 00 00 00 00 00 00 00 00
                                     ^
 ffffc9000cf47900: 00 00 00 00 00 00 00 f3 f3 f3 f3 00 00 00 00 00
 ffffc9000cf47980: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
==================================================================


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [syzbot] KASAN: stack-out-of-bounds Read in iov_iter_revert
  2021-08-12 13:43 [syzbot] KASAN: stack-out-of-bounds Read in iov_iter_revert syzbot
@ 2021-08-12 14:30 ` Pavel Begunkov
  2021-08-12 20:36   ` syzbot
  2021-08-23  0:24 ` [syzbot] KASAN: stack-out-of-bounds Read in iov_iter_revert Pavel Begunkov
  1 sibling, 1 reply; 22+ messages in thread
From: Pavel Begunkov @ 2021-08-12 14:30 UTC (permalink / raw)
  To: syzbot, axboe, io-uring, linux-kernel, syzkaller-bugs

On 8/12/21 2:43 PM, syzbot wrote:
> Hello,
> 
> syzbot found the following issue on:
> 
> HEAD commit:    1746f4db5135 Merge tag 'orphans-v5.14-rc6' of git://git.ke..
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=111065fa300000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=e3a20bae04b96ccd
> dashboard link: https://syzkaller.appspot.com/bug?extid=9671693590ef5aad8953
> compiler:       gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.1
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=119dcaf6300000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=120d216e300000
> 
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+9671693590ef5aad8953@syzkaller.appspotmail.com

I believe, was already discovered and sent out, let's try v2:

#syz test: https://github.com/isilence/linux.git truncate

> 
> ==================================================================
> BUG: KASAN: stack-out-of-bounds in iov_iter_revert lib/iov_iter.c:1093 [inline]
> BUG: KASAN: stack-out-of-bounds in iov_iter_revert+0x803/0x900 lib/iov_iter.c:1033
> Read of size 8 at addr ffffc9000cf478b0 by task syz-executor673/8439
> 
> CPU: 0 PID: 8439 Comm: syz-executor673 Not tainted 5.14.0-rc5-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:88 [inline]
>  dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:105
>  print_address_description.constprop.0.cold+0xf/0x309 mm/kasan/report.c:233
>  __kasan_report mm/kasan/report.c:419 [inline]
>  kasan_report.cold+0x83/0xdf mm/kasan/report.c:436
>  iov_iter_revert lib/iov_iter.c:1093 [inline]
>  iov_iter_revert+0x803/0x900 lib/iov_iter.c:1033
>  io_write+0x57b/0xed0 fs/io_uring.c:3459
>  io_issue_sqe+0x28c/0x6920 fs/io_uring.c:6181
>  __io_queue_sqe+0x1ac/0xf00 fs/io_uring.c:6464
>  io_queue_sqe fs/io_uring.c:6507 [inline]
>  io_submit_sqe fs/io_uring.c:6662 [inline]
>  io_submit_sqes+0x63ea/0x7bc0 fs/io_uring.c:6778
>  __do_sys_io_uring_enter+0xb03/0x1d40 fs/io_uring.c:9389
>  do_syscall_x64 arch/x86/entry/common.c:50 [inline]
>  do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
>  entry_SYSCALL_64_after_hwframe+0x44/0xae
> RIP: 0033:0x43f8a9
> Code: 28 c3 e8 1a 15 00 00 66 2e 0f 1f 84 00 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48
> RSP: 002b:00007ffcc6759968 EFLAGS: 00000246 ORIG_RAX: 00000000000001aa
> RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 000000000043f8a9
> RDX: 0000000000000000 RSI: 00000000000052fe RDI: 0000000000000003
> RBP: 00007ffcc6759988 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffcc6759990
> R13: 0000000000000000 R14: 00000000004ae018 R15: 0000000000400488
> 
> 
> addr ffffc9000cf478b0 is located in stack of task syz-executor673/8439 at offset 152 in frame:
>  io_write+0x0/0xed0 fs/io_uring.c:3335
> 
> this frame has 3 objects:
>  [48, 56) 'iovec'
>  [80, 120) '__iter'
>  [160, 288) 'inline_vecs'
> 
> Memory state around the buggy address:
>  ffffc9000cf47780: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>  ffffc9000cf47800: 00 00 00 f1 f1 f1 f1 00 00 00 f2 f2 f2 00 00 00
>> ffffc9000cf47880: 00 00 f2 f2 f2 f2 f2 00 00 00 00 00 00 00 00 00
>                                      ^
>  ffffc9000cf47900: 00 00 00 00 00 00 00 f3 f3 f3 f3 00 00 00 00 00
>  ffffc9000cf47980: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ==================================================================
> 
> 
> ---
> This report is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
> 
> syzbot will keep track of this issue. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
> syzbot can test patches for this issue, for details see:
> https://goo.gl/tpsmEJ#testing-patches
> 

-- 
Pavel Begunkov



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [syzbot] KASAN: stack-out-of-bounds Read in iov_iter_revert
  2021-08-12 14:30 ` Pavel Begunkov
@ 2021-08-12 20:36   ` syzbot
  2021-11-03 17:01     ` Lee Jones
  0 siblings, 1 reply; 22+ messages in thread
From: syzbot @ 2021-08-12 20:36 UTC (permalink / raw)
  To: asml.silence, axboe, io-uring, linux-kernel, syzkaller-bugs

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-and-tested-by: syzbot+9671693590ef5aad8953@syzkaller.appspotmail.com

Tested on:

commit:         bff2c168 io_uring: don't retry with truncated iter
git tree:       https://github.com/isilence/linux.git truncate
kernel config:  https://syzkaller.appspot.com/x/.config?x=730106bfb5bf8ace
dashboard link: https://syzkaller.appspot.com/bug?extid=9671693590ef5aad8953
compiler:       Debian clang version 11.0.1-2, GNU ld (GNU Binutils for Debian) 2.35.1

Note: testing is done by a robot and is best-effort only.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [syzbot] KASAN: stack-out-of-bounds Read in iov_iter_revert
  2021-08-12 13:43 [syzbot] KASAN: stack-out-of-bounds Read in iov_iter_revert syzbot
  2021-08-12 14:30 ` Pavel Begunkov
@ 2021-08-23  0:24 ` Pavel Begunkov
  2021-08-23  0:44   ` syzbot
  1 sibling, 1 reply; 22+ messages in thread
From: Pavel Begunkov @ 2021-08-23  0:24 UTC (permalink / raw)
  To: syzbot, axboe, io-uring, linux-kernel, syzkaller-bugs

On 8/12/21 2:43 PM, syzbot wrote:
> Hello,
> 
> syzbot found the following issue on:
> 
> HEAD commit:    1746f4db5135 Merge tag 'orphans-v5.14-rc6' of git://git.ke..
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=111065fa300000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=e3a20bae04b96ccd
> dashboard link: https://syzkaller.appspot.com/bug?extid=9671693590ef5aad8953
> compiler:       gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.1
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=119dcaf6300000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=120d216e300000
> 
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+9671693590ef5aad8953@syzkaller.appspotmail.com

#syz test: https://github.com/isilence/linux.git syztest_trunc2

> 
> ==================================================================
> BUG: KASAN: stack-out-of-bounds in iov_iter_revert lib/iov_iter.c:1093 [inline]
> BUG: KASAN: stack-out-of-bounds in iov_iter_revert+0x803/0x900 lib/iov_iter.c:1033
> Read of size 8 at addr ffffc9000cf478b0 by task syz-executor673/8439
> 
> CPU: 0 PID: 8439 Comm: syz-executor673 Not tainted 5.14.0-rc5-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:88 [inline]
>  dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:105
>  print_address_description.constprop.0.cold+0xf/0x309 mm/kasan/report.c:233
>  __kasan_report mm/kasan/report.c:419 [inline]
>  kasan_report.cold+0x83/0xdf mm/kasan/report.c:436
>  iov_iter_revert lib/iov_iter.c:1093 [inline]
>  iov_iter_revert+0x803/0x900 lib/iov_iter.c:1033
>  io_write+0x57b/0xed0 fs/io_uring.c:3459
>  io_issue_sqe+0x28c/0x6920 fs/io_uring.c:6181
>  __io_queue_sqe+0x1ac/0xf00 fs/io_uring.c:6464
>  io_queue_sqe fs/io_uring.c:6507 [inline]
>  io_submit_sqe fs/io_uring.c:6662 [inline]
>  io_submit_sqes+0x63ea/0x7bc0 fs/io_uring.c:6778
>  __do_sys_io_uring_enter+0xb03/0x1d40 fs/io_uring.c:9389
>  do_syscall_x64 arch/x86/entry/common.c:50 [inline]
>  do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
>  entry_SYSCALL_64_after_hwframe+0x44/0xae
> RIP: 0033:0x43f8a9
> Code: 28 c3 e8 1a 15 00 00 66 2e 0f 1f 84 00 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48
> RSP: 002b:00007ffcc6759968 EFLAGS: 00000246 ORIG_RAX: 00000000000001aa
> RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 000000000043f8a9
> RDX: 0000000000000000 RSI: 00000000000052fe RDI: 0000000000000003
> RBP: 00007ffcc6759988 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffcc6759990
> R13: 0000000000000000 R14: 00000000004ae018 R15: 0000000000400488
> 
> 
> addr ffffc9000cf478b0 is located in stack of task syz-executor673/8439 at offset 152 in frame:
>  io_write+0x0/0xed0 fs/io_uring.c:3335
> 
> this frame has 3 objects:
>  [48, 56) 'iovec'
>  [80, 120) '__iter'
>  [160, 288) 'inline_vecs'
> 
> Memory state around the buggy address:
>  ffffc9000cf47780: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>  ffffc9000cf47800: 00 00 00 f1 f1 f1 f1 00 00 00 f2 f2 f2 00 00 00
>> ffffc9000cf47880: 00 00 f2 f2 f2 f2 f2 00 00 00 00 00 00 00 00 00
>                                      ^
>  ffffc9000cf47900: 00 00 00 00 00 00 00 f3 f3 f3 f3 00 00 00 00 00
>  ffffc9000cf47980: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ==================================================================
> 
> 
> ---
> This report is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
> 
> syzbot will keep track of this issue. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
> syzbot can test patches for this issue, for details see:
> https://goo.gl/tpsmEJ#testing-patches
> 

-- 
Pavel Begunkov

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [syzbot] KASAN: stack-out-of-bounds Read in iov_iter_revert
  2021-08-23  0:24 ` [syzbot] KASAN: stack-out-of-bounds Read in iov_iter_revert Pavel Begunkov
@ 2021-08-23  0:44   ` syzbot
  0 siblings, 0 replies; 22+ messages in thread
From: syzbot @ 2021-08-23  0:44 UTC (permalink / raw)
  To: asml.silence, axboe, io-uring, linux-kernel, syzkaller-bugs

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-and-tested-by: syzbot+9671693590ef5aad8953@syzkaller.appspotmail.com

Tested on:

commit:         b917c794 io_uring: fix revert truncated vecs
git tree:       https://github.com/isilence/linux.git syztest_trunc2
kernel config:  https://syzkaller.appspot.com/x/.config?x=4aa932b5eaeee9ef
dashboard link: https://syzkaller.appspot.com/bug?extid=9671693590ef5aad8953
compiler:       Debian clang version 11.0.1-2, GNU ld (GNU Binutils for Debian) 2.35.1

Note: testing is done by a robot and is best-effort only.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [syzbot] KASAN: stack-out-of-bounds Read in iov_iter_revert
  2021-08-12 20:36   ` syzbot
@ 2021-11-03 17:01     ` Lee Jones
  2021-11-08 15:29       ` Pavel Begunkov
  0 siblings, 1 reply; 22+ messages in thread
From: Lee Jones @ 2021-11-03 17:01 UTC (permalink / raw)
  To: syzbot; +Cc: asml.silence, axboe, io-uring, linux-kernel, syzkaller-bugs

Good afternoon Pavel,

> syzbot has tested the proposed patch and the reproducer did not trigger any issue:
> 
> Reported-and-tested-by: syzbot+9671693590ef5aad8953@syzkaller.appspotmail.com
> 
> Tested on:
> 
> commit:         bff2c168 io_uring: don't retry with truncated iter
> git tree:       https://github.com/isilence/linux.git truncate
> kernel config:  https://syzkaller.appspot.com/x/.config?x=730106bfb5bf8ace
> dashboard link: https://syzkaller.appspot.com/bug?extid=9671693590ef5aad8953
> compiler:       Debian clang version 11.0.1-2, GNU ld (GNU Binutils for Debian) 2.35.1
> 
> Note: testing is done by a robot and is best-effort only.

As you can see in the 'dashboard link' above this bug also affects
android-5-10 which is currently based on v5.10.75.

I see that the back-port of this patch failed in v5.10.y:

  https://lore.kernel.org/stable/163152589512611@kroah.com/

And after solving the build-error by back-porting both:

  2112ff5ce0c11 iov_iter: track truncated size
  89c2b3b749182 io_uring: reexpand under-reexpanded iters

I now see execution tripping the WARN() in iov_iter_revert():

  if (WARN_ON(unroll > MAX_RW_COUNT))
      return

Am I missing any additional patches required to fix stable/v5.10.y?

Any help would be gratefully received.

Kind regards,
Lee

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [syzbot] KASAN: stack-out-of-bounds Read in iov_iter_revert
  2021-11-03 17:01     ` Lee Jones
@ 2021-11-08 15:29       ` Pavel Begunkov
  2021-11-08 15:41         ` Jens Axboe
  0 siblings, 1 reply; 22+ messages in thread
From: Pavel Begunkov @ 2021-11-08 15:29 UTC (permalink / raw)
  To: Lee Jones, syzbot; +Cc: axboe, io-uring, linux-kernel, syzkaller-bugs

On 11/3/21 17:01, Lee Jones wrote:
> Good afternoon Pavel,
> 
>> syzbot has tested the proposed patch and the reproducer did not trigger any issue:
>>
>> Reported-and-tested-by: syzbot+9671693590ef5aad8953@syzkaller.appspotmail.com
>>
>> Tested on:
>>
>> commit:         bff2c168 io_uring: don't retry with truncated iter
>> git tree:       https://github.com/isilence/linux.git truncate
>> kernel config:  https://syzkaller.appspot.com/x/.config?x=730106bfb5bf8ace
>> dashboard link: https://syzkaller.appspot.com/bug?extid=9671693590ef5aad8953
>> compiler:       Debian clang version 11.0.1-2, GNU ld (GNU Binutils for Debian) 2.35.1
>>
>> Note: testing is done by a robot and is best-effort only.
> 
> As you can see in the 'dashboard link' above this bug also affects
> android-5-10 which is currently based on v5.10.75.
> 
> I see that the back-port of this patch failed in v5.10.y:
> 
>    https://lore.kernel.org/stable/163152589512611@kroah.com/
> 
> And after solving the build-error by back-porting both:
> 
>    2112ff5ce0c11 iov_iter: track truncated size
>    89c2b3b749182 io_uring: reexpand under-reexpanded iters
> 
> I now see execution tripping the WARN() in iov_iter_revert():
> 
>    if (WARN_ON(unroll > MAX_RW_COUNT))
>        return
> 
> Am I missing any additional patches required to fix stable/v5.10.y?

Is it the same syz test? There was a couple more patches for
IORING_SETUP_IOPOLL, but strange if that's not the case.


fwiw, Jens decided to replace it with another mechanism shortly
after, so it may be a better idea to backport those. Jens,
what do you think?


commit 8fb0f47a9d7acf620d0fd97831b69da9bc5e22ed
Author: Jens Axboe <axboe@kernel.dk>
Date:   Fri Sep 10 11:18:36 2021 -0600

     iov_iter: add helper to save iov_iter state

commit cd65869512ab5668a5d16f789bc4da1319c435c4
Author: Jens Axboe <axboe@kernel.dk>
Date:   Fri Sep 10 11:19:14 2021 -0600

     io_uring: use iov_iter state save/restore helpers


-- 
Pavel Begunkov

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [syzbot] KASAN: stack-out-of-bounds Read in iov_iter_revert
  2021-11-08 15:29       ` Pavel Begunkov
@ 2021-11-08 15:41         ` Jens Axboe
  2021-11-09 13:33           ` Lee Jones
  2022-05-05 14:11           ` linux-stable-5.10-y CVE-2022-1508 of io_uring module Guo Xuenan
  0 siblings, 2 replies; 22+ messages in thread
From: Jens Axboe @ 2021-11-08 15:41 UTC (permalink / raw)
  To: Pavel Begunkov, Lee Jones, syzbot; +Cc: io-uring, linux-kernel, syzkaller-bugs

On 11/8/21 8:29 AM, Pavel Begunkov wrote:
> On 11/3/21 17:01, Lee Jones wrote:
>> Good afternoon Pavel,
>>
>>> syzbot has tested the proposed patch and the reproducer did not trigger any issue:
>>>
>>> Reported-and-tested-by: syzbot+9671693590ef5aad8953@syzkaller.appspotmail.com
>>>
>>> Tested on:
>>>
>>> commit:         bff2c168 io_uring: don't retry with truncated iter
>>> git tree:       https://github.com/isilence/linux.git truncate
>>> kernel config:  https://syzkaller.appspot.com/x/.config?x=730106bfb5bf8ace
>>> dashboard link: https://syzkaller.appspot.com/bug?extid=9671693590ef5aad8953
>>> compiler:       Debian clang version 11.0.1-2, GNU ld (GNU Binutils for Debian) 2.35.1
>>>
>>> Note: testing is done by a robot and is best-effort only.
>>
>> As you can see in the 'dashboard link' above this bug also affects
>> android-5-10 which is currently based on v5.10.75.
>>
>> I see that the back-port of this patch failed in v5.10.y:
>>
>>    https://lore.kernel.org/stable/163152589512611@kroah.com/
>>
>> And after solving the build-error by back-porting both:
>>
>>    2112ff5ce0c11 iov_iter: track truncated size
>>    89c2b3b749182 io_uring: reexpand under-reexpanded iters
>>
>> I now see execution tripping the WARN() in iov_iter_revert():
>>
>>    if (WARN_ON(unroll > MAX_RW_COUNT))
>>        return
>>
>> Am I missing any additional patches required to fix stable/v5.10.y?
> 
> Is it the same syz test? There was a couple more patches for
> IORING_SETUP_IOPOLL, but strange if that's not the case.
> 
> 
> fwiw, Jens decided to replace it with another mechanism shortly
> after, so it may be a better idea to backport those. Jens,
> what do you think?
> 
> 
> commit 8fb0f47a9d7acf620d0fd97831b69da9bc5e22ed
> Author: Jens Axboe <axboe@kernel.dk>
> Date:   Fri Sep 10 11:18:36 2021 -0600
> 
>      iov_iter: add helper to save iov_iter state
> 
> commit cd65869512ab5668a5d16f789bc4da1319c435c4
> Author: Jens Axboe <axboe@kernel.dk>
> Date:   Fri Sep 10 11:19:14 2021 -0600
> 
>      io_uring: use iov_iter state save/restore helpers

Yes, I think backporting based on the save/restore setup is the
sanest way by far.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [syzbot] KASAN: stack-out-of-bounds Read in iov_iter_revert
  2021-11-08 15:41         ` Jens Axboe
@ 2021-11-09 13:33           ` Lee Jones
  2021-12-15  8:06             ` Lee Jones
  2022-05-05 14:11           ` linux-stable-5.10-y CVE-2022-1508 of io_uring module Guo Xuenan
  1 sibling, 1 reply; 22+ messages in thread
From: Lee Jones @ 2021-11-09 13:33 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Pavel Begunkov, syzbot, io-uring, linux-kernel, syzkaller-bugs,
	Greg Kroah-Hartman

On Mon, 08 Nov 2021, Jens Axboe wrote:
> On 11/8/21 8:29 AM, Pavel Begunkov wrote:
> > On 11/3/21 17:01, Lee Jones wrote:
> >> Good afternoon Pavel,
> >>
> >>> syzbot has tested the proposed patch and the reproducer did not trigger any issue:
> >>>
> >>> Reported-and-tested-by: syzbot+9671693590ef5aad8953@syzkaller.appspotmail.com
> >>>
> >>> Tested on:
> >>>
> >>> commit:         bff2c168 io_uring: don't retry with truncated iter
> >>> git tree:       https://github.com/isilence/linux.git truncate
> >>> kernel config:  https://syzkaller.appspot.com/x/.config?x=730106bfb5bf8ace
> >>> dashboard link: https://syzkaller.appspot.com/bug?extid=9671693590ef5aad8953
> >>> compiler:       Debian clang version 11.0.1-2, GNU ld (GNU Binutils for Debian) 2.35.1
> >>>
> >>> Note: testing is done by a robot and is best-effort only.
> >>
> >> As you can see in the 'dashboard link' above this bug also affects
> >> android-5-10 which is currently based on v5.10.75.
> >>
> >> I see that the back-port of this patch failed in v5.10.y:
> >>
> >>    https://lore.kernel.org/stable/163152589512611@kroah.com/
> >>
> >> And after solving the build-error by back-porting both:
> >>
> >>    2112ff5ce0c11 iov_iter: track truncated size
> >>    89c2b3b749182 io_uring: reexpand under-reexpanded iters
> >>
> >> I now see execution tripping the WARN() in iov_iter_revert():
> >>
> >>    if (WARN_ON(unroll > MAX_RW_COUNT))
> >>        return
> >>
> >> Am I missing any additional patches required to fix stable/v5.10.y?
> > 
> > Is it the same syz test? There was a couple more patches for
> > IORING_SETUP_IOPOLL, but strange if that's not the case.
> > 
> > 
> > fwiw, Jens decided to replace it with another mechanism shortly
> > after, so it may be a better idea to backport those. Jens,
> > what do you think?
> > 
> > 
> > commit 8fb0f47a9d7acf620d0fd97831b69da9bc5e22ed
> > Author: Jens Axboe <axboe@kernel.dk>
> > Date:   Fri Sep 10 11:18:36 2021 -0600
> > 
> >      iov_iter: add helper to save iov_iter state
> > 
> > commit cd65869512ab5668a5d16f789bc4da1319c435c4
> > Author: Jens Axboe <axboe@kernel.dk>
> > Date:   Fri Sep 10 11:19:14 2021 -0600
> > 
> >      io_uring: use iov_iter state save/restore helpers
> 
> Yes, I think backporting based on the save/restore setup is the
> sanest way by far.

Would you be kind enough to attempt to send these patches to Stable?

When I tried to back-port them, the second one gave me trouble.  And
without the in depth knowledge of the driver/subsystem that you guys
have, I found it almost impossible to resolve all of the conflicts:

diff --cc fs/io_uring.c
index 104dff9c71314,25bda8a5a4e5d..0000000000000
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@@ -2567,57 -2603,20 +2568,64 @@@ static void io_complete_rw_common(struc
  }
  
  #ifdef CONFIG_BLOCK
 -static bool io_resubmit_prep(struct io_kiocb *req)
 +static bool io_resubmit_prep(struct io_kiocb *req, int error)
  {
 -	struct io_async_rw *rw = req->async_data;
 +	struct iovec inline_vecs[UIO_FASTIOV], *iovec = inline_vecs;
 +	ssize_t ret = -ECANCELED;
 +	struct iov_iter iter;
 +	int rw;
 +
++<<<<<<< HEAD
 +	if (error) {
 +		ret = error;
 +		goto end_req;
 +	}
 +
 +	switch (req->opcode) {
 +	case IORING_OP_READV:
 +	case IORING_OP_READ_FIXED:
 +	case IORING_OP_READ:
 +		rw = READ;
 +		break;
 +	case IORING_OP_WRITEV:
 +	case IORING_OP_WRITE_FIXED:
 +	case IORING_OP_WRITE:
 +		rw = WRITE;
 +		break;
 +	default:
 +		printk_once(KERN_WARNING "io_uring: bad opcode in resubmit %d\n",
 +				req->opcode);
 +		goto end_req;
 +	}
  
 +	if (!req->async_data) {
 +		ret = io_import_iovec(rw, req, &iovec, &iter, false);
 +		if (ret < 0)
 +			goto end_req;
 +		ret = io_setup_async_rw(req, iovec, inline_vecs, &iter, false);
 +		if (!ret)
 +			return true;
 +		kfree(iovec);
 +	} else {
 +		return true;
 +	}
 +end_req:
 +	req_set_fail_links(req);
 +	return false;
++=======
+ 	if (!rw)
+ 		return !io_req_prep_async(req);
+ 	iov_iter_restore(&rw->iter, &rw->iter_state);
+ 	return true;
++>>>>>>> cd65869512ab5 (io_uring: use iov_iter state save/restore helpers)
  }
 +#endif
  
 -static bool io_rw_should_reissue(struct io_kiocb *req)
 +static bool io_rw_reissue(struct io_kiocb *req, long res)
  {
 +#ifdef CONFIG_BLOCK
  	umode_t mode = file_inode(req->file)->i_mode;
 -	struct io_ring_ctx *ctx = req->ctx;
 +	int ret;
  
  	if (!S_ISBLK(mode) && !S_ISREG(mode))
  		return false;
@@@ -3268,13 -3307,20 +3276,23 @@@ static int io_setup_async_rw(struct io_
  			     const struct iovec *fast_iov,
  			     struct iov_iter *iter, bool force)
  {
 -	if (!force && !io_op_defs[req->opcode].needs_async_setup)
 +	if (!force && !io_op_defs[req->opcode].needs_async_data)
  		return 0;
  	if (!req->async_data) {
++<<<<<<< HEAD
 +		if (__io_alloc_async_data(req))
++=======
+ 		struct io_async_rw *iorw;
+ 
+ 		if (io_alloc_async_data(req)) {
+ 			kfree(iovec);
++>>>>>>> cd65869512ab5 (io_uring: use iov_iter state save/restore helpers)
  			return -ENOMEM;
 -		}
  
  		io_req_map_rw(req, iovec, fast_iov, iter);
+ 		iorw = req->async_data;
+ 		/* we've copied and mapped the iter, ensure state is saved */
+ 		iov_iter_save_state(&iorw->iter, &iorw->iter_state);
  	}
  	return 0;
  }
@@@ -3417,18 -3443,28 +3436,43 @@@ static int io_read(struct io_kiocb *req
  	struct kiocb *kiocb = &req->rw.kiocb;
  	struct iov_iter __iter, *iter = &__iter;
  	struct io_async_rw *rw = req->async_data;
++<<<<<<< HEAD
 +	ssize_t io_size, ret, ret2;
 +	bool no_async;
++=======
+ 	bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
+ 	struct iov_iter_state __state, *state;
+ 	ssize_t ret, ret2;
++>>>>>>> cd65869512ab5 (io_uring: use iov_iter state save/restore helpers)
  
 -	if (rw) {
 +	if (rw)
  		iter = &rw->iter;
++<<<<<<< HEAD
 +
 +	ret = io_import_iovec(READ, req, &iovec, iter, !force_nonblock);
 +	if (ret < 0)
 +		return ret;
 +	io_size = iov_iter_count(iter);
 +	req->result = io_size;
 +	ret = 0;
++=======
+ 		state = &rw->iter_state;
+ 		/*
+ 		 * We come here from an earlier attempt, restore our state to
+ 		 * match in case it doesn't. It's cheap enough that we don't
+ 		 * need to make this conditional.
+ 		 */
+ 		iov_iter_restore(iter, state);
+ 		iovec = NULL;
+ 	} else {
+ 		ret = io_import_iovec(READ, req, &iovec, iter, !force_nonblock);
+ 		if (ret < 0)
+ 			return ret;
+ 		state = &__state;
+ 		iov_iter_save_state(iter, state);
+ 	}
+ 	req->result = iov_iter_count(iter);
++>>>>>>> cd65869512ab5 (io_uring: use iov_iter state save/restore helpers)
  
  	/* Ensure we clear previously set non-block flag */
  	if (!force_nonblock)
@@@ -3436,15 -3472,17 +3480,23 @@@
  	else
  		kiocb->ki_flags |= IOCB_NOWAIT;
  
 +
  	/* If the file doesn't support async, just async punt */
 -	if (force_nonblock && !io_file_supports_nowait(req, READ)) {
 -		ret = io_setup_async_rw(req, iovec, inline_vecs, iter, true);
 -		return ret ?: -EAGAIN;
 -	}
 +	no_async = force_nonblock && !io_file_supports_async(req->file, READ);
 +	if (no_async)
 +		goto copy_iov;
  
++<<<<<<< HEAD
 +	ret = rw_verify_area(READ, req->file, io_kiocb_ppos(kiocb), io_size);
 +	if (unlikely(ret))
 +		goto out_free;
++=======
+ 	ret = rw_verify_area(READ, req->file, io_kiocb_ppos(kiocb), req->result);
+ 	if (unlikely(ret)) {
+ 		kfree(iovec);
+ 		return ret;
+ 	}
++>>>>>>> cd65869512ab5 (io_uring: use iov_iter state save/restore helpers)
  
  	ret = io_iter_do_read(req, iter);
  
@@@ -3457,68 -3491,78 +3509,133 @@@
  		/* IOPOLL retry should happen for io-wq threads */
  		if (!force_nonblock && !(req->ctx->flags & IORING_SETUP_IOPOLL))
  			goto done;
 -		/* no retry on NONBLOCK nor RWF_NOWAIT */
 -		if (req->flags & REQ_F_NOWAIT)
 +		/* no retry on NONBLOCK marked file */
 +		if (req->file->f_flags & O_NONBLOCK)
  			goto done;
++<<<<<<< HEAD
 +		/* some cases will consume bytes even on error returns */
 +		iov_iter_revert(iter, io_size - iov_iter_count(iter));
 +		ret = 0;
 +		goto copy_iov;
 +	} else if (ret < 0) {
 +		/* make sure -ERESTARTSYS -> -EINTR is done */
 +		goto done;
 +	}
 +
 +	/* read it all, or we did blocking attempt. no retry. */
 +	if (!iov_iter_count(iter) || !force_nonblock ||
 +	    (req->file->f_flags & O_NONBLOCK) || !(req->flags & REQ_F_ISREG))
 +		goto done;
++=======
+ 		ret = 0;
+ 	} else if (ret == -EIOCBQUEUED) {
+ 		goto out_free;
+ 	} else if (ret <= 0 || ret == req->result || !force_nonblock ||
+ 		   (req->flags & REQ_F_NOWAIT) || !need_read_all(req)) {
+ 		/* read all, failed, already did sync or don't want to retry */
+ 		goto done;
+ 	}
+ 
+ 	/*
+ 	 * Don't depend on the iter state matching what was consumed, or being
+ 	 * untouched in case of error. Restore it and we'll advance it
+ 	 * manually if we need to.
+ 	 */
+ 	iov_iter_restore(iter, state);
+ 
+ 	ret2 = io_setup_async_rw(req, iovec, inline_vecs, iter, true);
+ 	if (ret2)
+ 		return ret2;
++>>>>>>> cd65869512ab5 (io_uring: use iov_iter state save/restore helpers)
  
 -	iovec = NULL;
 +	io_size -= ret;
 +copy_iov:
 +	ret2 = io_setup_async_rw(req, iovec, inline_vecs, iter, true);
 +	if (ret2) {
 +		ret = ret2;
 +		goto out_free;
 +	}
 +	if (no_async)
 +		return -EAGAIN;
  	rw = req->async_data;
++<<<<<<< HEAD
 +	/* it's copied and will be cleaned with ->io */
 +	iovec = NULL;
 +	/* now use our persistent iterator, if we aren't already */
 +	iter = &rw->iter;
 +retry:
 +	rw->bytes_done += ret;
 +	/* if we can retry, do so with the callbacks armed */
 +	if (!io_rw_should_retry(req)) {
 +		kiocb->ki_flags &= ~IOCB_WAITQ;
 +		return -EAGAIN;
 +	}
 +
  	/*
 -	 * Now use our persistent iterator and state, if we aren't already.
 -	 * We've restored and mapped the iter to match.
 +	 * Now retry read with the IOCB_WAITQ parts set in the iocb. If we
 +	 * get -EIOCBQUEUED, then we'll get a notification when the desired
 +	 * page gets unlocked. We can also get a partial read here, and if we
 +	 * do, then just retry at the new offset.
  	 */
 -	if (iter != &rw->iter) {
 -		iter = &rw->iter;
 +	ret = io_iter_do_read(req, iter);
 +	if (ret == -EIOCBQUEUED) {
 +		ret = 0;
 +		goto out_free;
 +	} else if (ret > 0 && ret < io_size) {
 +		/* we got some bytes, but not all. retry. */
 +		kiocb->ki_flags &= ~IOCB_WAITQ;
 +		goto retry;
 +	}
++=======
++	/*
++	 * Now use our persistent iterator and state, if we aren't already.
++	 * We've restored and mapped the iter to match.
++	 */
++	if (iter != &rw->iter) {
++		iter = &rw->iter;
+ 		state = &rw->iter_state;
+ 	}
+ 
+ 	do {
+ 		/*
+ 		 * We end up here because of a partial read, either from
+ 		 * above or inside this loop. Advance the iter by the bytes
+ 		 * that were consumed.
+ 		 */
+ 		iov_iter_advance(iter, ret);
+ 		if (!iov_iter_count(iter))
+ 			break;
+ 		rw->bytes_done += ret;
+ 		iov_iter_save_state(iter, state);
+ 
+ 		/* if we can retry, do so with the callbacks armed */
+ 		if (!io_rw_should_retry(req)) {
+ 			kiocb->ki_flags &= ~IOCB_WAITQ;
+ 			return -EAGAIN;
+ 		}
+ 
+ 		/*
+ 		 * Now retry read with the IOCB_WAITQ parts set in the iocb. If
+ 		 * we get -EIOCBQUEUED, then we'll get a notification when the
+ 		 * desired page gets unlocked. We can also get a partial read
+ 		 * here, and if we do, then just retry at the new offset.
+ 		 */
+ 		ret = io_iter_do_read(req, iter);
+ 		if (ret == -EIOCBQUEUED)
+ 			return 0;
+ 		/* we got some bytes, but not all. retry. */
+ 		kiocb->ki_flags &= ~IOCB_WAITQ;
+ 		iov_iter_restore(iter, state);
+ 	} while (ret > 0);
++>>>>>>> cd65869512ab5 (io_uring: use iov_iter state save/restore helpers)
  done:
 -	kiocb_done(kiocb, ret, issue_flags);
 +	kiocb_done(kiocb, ret, cs);
 +	ret = 0;
  out_free:
 -	/* it's faster to check here then delegate to kfree */
 +	/* it's reportedly faster than delegating the null check to kfree() */
  	if (iovec)
  		kfree(iovec);
 -	return 0;
 +	return ret;
  }
  
  static int io_write_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
@@@ -3545,16 -3578,24 +3662,37 @@@ static int io_write(struct io_kiocb *re
  	struct kiocb *kiocb = &req->rw.kiocb;
  	struct iov_iter __iter, *iter = &__iter;
  	struct io_async_rw *rw = req->async_data;
++<<<<<<< HEAD
 +	ssize_t ret, ret2, io_size;
++=======
+ 	bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
+ 	struct iov_iter_state __state, *state;
+ 	ssize_t ret, ret2;
++>>>>>>> cd65869512ab5 (io_uring: use iov_iter state save/restore helpers)
  
 -	if (rw) {
 +	if (rw)
  		iter = &rw->iter;
++<<<<<<< HEAD
 +
 +	ret = io_import_iovec(WRITE, req, &iovec, iter, !force_nonblock);
 +	if (ret < 0)
 +		return ret;
 +	io_size = iov_iter_count(iter);
 +	req->result = io_size;
++=======
+ 		state = &rw->iter_state;
+ 		iov_iter_restore(iter, state);
+ 		iovec = NULL;
+ 	} else {
+ 		ret = io_import_iovec(WRITE, req, &iovec, iter, !force_nonblock);
+ 		if (ret < 0)
+ 			return ret;
+ 		state = &__state;
+ 		iov_iter_save_state(iter, state);
+ 	}
+ 	req->result = iov_iter_count(iter);
+ 	ret2 = 0;
++>>>>>>> cd65869512ab5 (io_uring: use iov_iter state save/restore helpers)
  
  	/* Ensure we clear previously set non-block flag */
  	if (!force_nonblock)
@@@ -3610,14 -3656,14 +3748,20 @@@
  		if ((req->ctx->flags & IORING_SETUP_IOPOLL) && ret2 == -EAGAIN)
  			goto copy_iov;
  done:
 -		kiocb_done(kiocb, ret2, issue_flags);
 +		kiocb_done(kiocb, ret2, cs);
  	} else {
  copy_iov:
++<<<<<<< HEAD
 +		/* some cases will consume bytes even on error returns */
 +		iov_iter_revert(iter, io_size - iov_iter_count(iter));
++=======
+ 		iov_iter_restore(iter, state);
+ 		if (ret2 > 0)
+ 			iov_iter_advance(iter, ret2);
++>>>>>>> cd65869512ab5 (io_uring: use iov_iter state save/restore helpers)
  		ret = io_setup_async_rw(req, iovec, inline_vecs, iter, false);
 -		return ret ?: -EAGAIN;
 +		if (!ret)
 +			return -EAGAIN;
  	}
  out_free:
  	/* it's reportedly faster than delegating the null check to kfree() */

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [syzbot] KASAN: stack-out-of-bounds Read in iov_iter_revert
  2021-11-09 13:33           ` Lee Jones
@ 2021-12-15  8:06             ` Lee Jones
  2021-12-15 11:16               ` Pavel Begunkov
  0 siblings, 1 reply; 22+ messages in thread
From: Lee Jones @ 2021-12-15  8:06 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Pavel Begunkov, syzbot, io-uring, linux-kernel, syzkaller-bugs,
	Greg Kroah-Hartman

On Tue, 09 Nov 2021, Lee Jones wrote:

> On Mon, 08 Nov 2021, Jens Axboe wrote:
> > On 11/8/21 8:29 AM, Pavel Begunkov wrote:
> > > On 11/3/21 17:01, Lee Jones wrote:
> > >> Good afternoon Pavel,
> > >>
> > >>> syzbot has tested the proposed patch and the reproducer did not trigger any issue:
> > >>>
> > >>> Reported-and-tested-by: syzbot+9671693590ef5aad8953@syzkaller.appspotmail.com
> > >>>
> > >>> Tested on:
> > >>>
> > >>> commit:         bff2c168 io_uring: don't retry with truncated iter
> > >>> git tree:       https://github.com/isilence/linux.git truncate
> > >>> kernel config:  https://syzkaller.appspot.com/x/.config?x=730106bfb5bf8ace
> > >>> dashboard link: https://syzkaller.appspot.com/bug?extid=9671693590ef5aad8953
> > >>> compiler:       Debian clang version 11.0.1-2, GNU ld (GNU Binutils for Debian) 2.35.1
> > >>>
> > >>> Note: testing is done by a robot and is best-effort only.
> > >>
> > >> As you can see in the 'dashboard link' above this bug also affects
> > >> android-5-10 which is currently based on v5.10.75.
> > >>
> > >> I see that the back-port of this patch failed in v5.10.y:
> > >>
> > >>    https://lore.kernel.org/stable/163152589512611@kroah.com/
> > >>
> > >> And after solving the build-error by back-porting both:
> > >>
> > >>    2112ff5ce0c11 iov_iter: track truncated size
> > >>    89c2b3b749182 io_uring: reexpand under-reexpanded iters
> > >>
> > >> I now see execution tripping the WARN() in iov_iter_revert():
> > >>
> > >>    if (WARN_ON(unroll > MAX_RW_COUNT))
> > >>        return
> > >>
> > >> Am I missing any additional patches required to fix stable/v5.10.y?
> > > 
> > > Is it the same syz test? There was a couple more patches for
> > > IORING_SETUP_IOPOLL, but strange if that's not the case.
> > > 
> > > 
> > > fwiw, Jens decided to replace it with another mechanism shortly
> > > after, so it may be a better idea to backport those. Jens,
> > > what do you think?
> > > 
> > > 
> > > commit 8fb0f47a9d7acf620d0fd97831b69da9bc5e22ed
> > > Author: Jens Axboe <axboe@kernel.dk>
> > > Date:   Fri Sep 10 11:18:36 2021 -0600
> > > 
> > >      iov_iter: add helper to save iov_iter state
> > > 
> > > commit cd65869512ab5668a5d16f789bc4da1319c435c4
> > > Author: Jens Axboe <axboe@kernel.dk>
> > > Date:   Fri Sep 10 11:19:14 2021 -0600
> > > 
> > >      io_uring: use iov_iter state save/restore helpers
> > 
> > Yes, I think backporting based on the save/restore setup is the
> > sanest way by far.
> 
> Would you be kind enough to attempt to send these patches to Stable?
> 
> When I tried to back-port them, the second one gave me trouble.  And
> without the in depth knowledge of the driver/subsystem that you guys
> have, I found it almost impossible to resolve all of the conflicts:

Any movement on this chaps?

Not sure I am able to do this back-port without your help.

> diff --cc fs/io_uring.c
> index 104dff9c71314,25bda8a5a4e5d..0000000000000
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@@ -2567,57 -2603,20 +2568,64 @@@ static void io_complete_rw_common(struc
>   }
>   
>   #ifdef CONFIG_BLOCK
>  -static bool io_resubmit_prep(struct io_kiocb *req)
>  +static bool io_resubmit_prep(struct io_kiocb *req, int error)
>   {
>  -	struct io_async_rw *rw = req->async_data;
>  +	struct iovec inline_vecs[UIO_FASTIOV], *iovec = inline_vecs;
>  +	ssize_t ret = -ECANCELED;
>  +	struct iov_iter iter;
>  +	int rw;
>  +
> ++<<<<<<< HEAD
>  +	if (error) {
>  +		ret = error;
>  +		goto end_req;
>  +	}
>  +
>  +	switch (req->opcode) {
>  +	case IORING_OP_READV:
>  +	case IORING_OP_READ_FIXED:
>  +	case IORING_OP_READ:
>  +		rw = READ;
>  +		break;
>  +	case IORING_OP_WRITEV:
>  +	case IORING_OP_WRITE_FIXED:
>  +	case IORING_OP_WRITE:
>  +		rw = WRITE;
>  +		break;
>  +	default:
>  +		printk_once(KERN_WARNING "io_uring: bad opcode in resubmit %d\n",
>  +				req->opcode);
>  +		goto end_req;
>  +	}
>   
>  +	if (!req->async_data) {
>  +		ret = io_import_iovec(rw, req, &iovec, &iter, false);
>  +		if (ret < 0)
>  +			goto end_req;
>  +		ret = io_setup_async_rw(req, iovec, inline_vecs, &iter, false);
>  +		if (!ret)
>  +			return true;
>  +		kfree(iovec);
>  +	} else {
>  +		return true;
>  +	}
>  +end_req:
>  +	req_set_fail_links(req);
>  +	return false;
> ++=======
> + 	if (!rw)
> + 		return !io_req_prep_async(req);
> + 	iov_iter_restore(&rw->iter, &rw->iter_state);
> + 	return true;
> ++>>>>>>> cd65869512ab5 (io_uring: use iov_iter state save/restore helpers)
>   }
>  +#endif
>   
>  -static bool io_rw_should_reissue(struct io_kiocb *req)
>  +static bool io_rw_reissue(struct io_kiocb *req, long res)
>   {
>  +#ifdef CONFIG_BLOCK
>   	umode_t mode = file_inode(req->file)->i_mode;
>  -	struct io_ring_ctx *ctx = req->ctx;
>  +	int ret;
>   
>   	if (!S_ISBLK(mode) && !S_ISREG(mode))
>   		return false;
> @@@ -3268,13 -3307,20 +3276,23 @@@ static int io_setup_async_rw(struct io_
>   			     const struct iovec *fast_iov,
>   			     struct iov_iter *iter, bool force)
>   {
>  -	if (!force && !io_op_defs[req->opcode].needs_async_setup)
>  +	if (!force && !io_op_defs[req->opcode].needs_async_data)
>   		return 0;
>   	if (!req->async_data) {
> ++<<<<<<< HEAD
>  +		if (__io_alloc_async_data(req))
> ++=======
> + 		struct io_async_rw *iorw;
> + 
> + 		if (io_alloc_async_data(req)) {
> + 			kfree(iovec);
> ++>>>>>>> cd65869512ab5 (io_uring: use iov_iter state save/restore helpers)
>   			return -ENOMEM;
>  -		}
>   
>   		io_req_map_rw(req, iovec, fast_iov, iter);
> + 		iorw = req->async_data;
> + 		/* we've copied and mapped the iter, ensure state is saved */
> + 		iov_iter_save_state(&iorw->iter, &iorw->iter_state);
>   	}
>   	return 0;
>   }
> @@@ -3417,18 -3443,28 +3436,43 @@@ static int io_read(struct io_kiocb *req
>   	struct kiocb *kiocb = &req->rw.kiocb;
>   	struct iov_iter __iter, *iter = &__iter;
>   	struct io_async_rw *rw = req->async_data;
> ++<<<<<<< HEAD
>  +	ssize_t io_size, ret, ret2;
>  +	bool no_async;
> ++=======
> + 	bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
> + 	struct iov_iter_state __state, *state;
> + 	ssize_t ret, ret2;
> ++>>>>>>> cd65869512ab5 (io_uring: use iov_iter state save/restore helpers)
>   
>  -	if (rw) {
>  +	if (rw)
>   		iter = &rw->iter;
> ++<<<<<<< HEAD
>  +
>  +	ret = io_import_iovec(READ, req, &iovec, iter, !force_nonblock);
>  +	if (ret < 0)
>  +		return ret;
>  +	io_size = iov_iter_count(iter);
>  +	req->result = io_size;
>  +	ret = 0;
> ++=======
> + 		state = &rw->iter_state;
> + 		/*
> + 		 * We come here from an earlier attempt, restore our state to
> + 		 * match in case it doesn't. It's cheap enough that we don't
> + 		 * need to make this conditional.
> + 		 */
> + 		iov_iter_restore(iter, state);
> + 		iovec = NULL;
> + 	} else {
> + 		ret = io_import_iovec(READ, req, &iovec, iter, !force_nonblock);
> + 		if (ret < 0)
> + 			return ret;
> + 		state = &__state;
> + 		iov_iter_save_state(iter, state);
> + 	}
> + 	req->result = iov_iter_count(iter);
> ++>>>>>>> cd65869512ab5 (io_uring: use iov_iter state save/restore helpers)
>   
>   	/* Ensure we clear previously set non-block flag */
>   	if (!force_nonblock)
> @@@ -3436,15 -3472,17 +3480,23 @@@
>   	else
>   		kiocb->ki_flags |= IOCB_NOWAIT;
>   
>  +
>   	/* If the file doesn't support async, just async punt */
>  -	if (force_nonblock && !io_file_supports_nowait(req, READ)) {
>  -		ret = io_setup_async_rw(req, iovec, inline_vecs, iter, true);
>  -		return ret ?: -EAGAIN;
>  -	}
>  +	no_async = force_nonblock && !io_file_supports_async(req->file, READ);
>  +	if (no_async)
>  +		goto copy_iov;
>   
> ++<<<<<<< HEAD
>  +	ret = rw_verify_area(READ, req->file, io_kiocb_ppos(kiocb), io_size);
>  +	if (unlikely(ret))
>  +		goto out_free;
> ++=======
> + 	ret = rw_verify_area(READ, req->file, io_kiocb_ppos(kiocb), req->result);
> + 	if (unlikely(ret)) {
> + 		kfree(iovec);
> + 		return ret;
> + 	}
> ++>>>>>>> cd65869512ab5 (io_uring: use iov_iter state save/restore helpers)
>   
>   	ret = io_iter_do_read(req, iter);
>   
> @@@ -3457,68 -3491,78 +3509,133 @@@
>   		/* IOPOLL retry should happen for io-wq threads */
>   		if (!force_nonblock && !(req->ctx->flags & IORING_SETUP_IOPOLL))
>   			goto done;
>  -		/* no retry on NONBLOCK nor RWF_NOWAIT */
>  -		if (req->flags & REQ_F_NOWAIT)
>  +		/* no retry on NONBLOCK marked file */
>  +		if (req->file->f_flags & O_NONBLOCK)
>   			goto done;
> ++<<<<<<< HEAD
>  +		/* some cases will consume bytes even on error returns */
>  +		iov_iter_revert(iter, io_size - iov_iter_count(iter));
>  +		ret = 0;
>  +		goto copy_iov;
>  +	} else if (ret < 0) {
>  +		/* make sure -ERESTARTSYS -> -EINTR is done */
>  +		goto done;
>  +	}
>  +
>  +	/* read it all, or we did blocking attempt. no retry. */
>  +	if (!iov_iter_count(iter) || !force_nonblock ||
>  +	    (req->file->f_flags & O_NONBLOCK) || !(req->flags & REQ_F_ISREG))
>  +		goto done;
> ++=======
> + 		ret = 0;
> + 	} else if (ret == -EIOCBQUEUED) {
> + 		goto out_free;
> + 	} else if (ret <= 0 || ret == req->result || !force_nonblock ||
> + 		   (req->flags & REQ_F_NOWAIT) || !need_read_all(req)) {
> + 		/* read all, failed, already did sync or don't want to retry */
> + 		goto done;
> + 	}
> + 
> + 	/*
> + 	 * Don't depend on the iter state matching what was consumed, or being
> + 	 * untouched in case of error. Restore it and we'll advance it
> + 	 * manually if we need to.
> + 	 */
> + 	iov_iter_restore(iter, state);
> + 
> + 	ret2 = io_setup_async_rw(req, iovec, inline_vecs, iter, true);
> + 	if (ret2)
> + 		return ret2;
> ++>>>>>>> cd65869512ab5 (io_uring: use iov_iter state save/restore helpers)
>   
>  -	iovec = NULL;
>  +	io_size -= ret;
>  +copy_iov:
>  +	ret2 = io_setup_async_rw(req, iovec, inline_vecs, iter, true);
>  +	if (ret2) {
>  +		ret = ret2;
>  +		goto out_free;
>  +	}
>  +	if (no_async)
>  +		return -EAGAIN;
>   	rw = req->async_data;
> ++<<<<<<< HEAD
>  +	/* it's copied and will be cleaned with ->io */
>  +	iovec = NULL;
>  +	/* now use our persistent iterator, if we aren't already */
>  +	iter = &rw->iter;
>  +retry:
>  +	rw->bytes_done += ret;
>  +	/* if we can retry, do so with the callbacks armed */
>  +	if (!io_rw_should_retry(req)) {
>  +		kiocb->ki_flags &= ~IOCB_WAITQ;
>  +		return -EAGAIN;
>  +	}
>  +
>   	/*
>  -	 * Now use our persistent iterator and state, if we aren't already.
>  -	 * We've restored and mapped the iter to match.
>  +	 * Now retry read with the IOCB_WAITQ parts set in the iocb. If we
>  +	 * get -EIOCBQUEUED, then we'll get a notification when the desired
>  +	 * page gets unlocked. We can also get a partial read here, and if we
>  +	 * do, then just retry at the new offset.
>   	 */
>  -	if (iter != &rw->iter) {
>  -		iter = &rw->iter;
>  +	ret = io_iter_do_read(req, iter);
>  +	if (ret == -EIOCBQUEUED) {
>  +		ret = 0;
>  +		goto out_free;
>  +	} else if (ret > 0 && ret < io_size) {
>  +		/* we got some bytes, but not all. retry. */
>  +		kiocb->ki_flags &= ~IOCB_WAITQ;
>  +		goto retry;
>  +	}
> ++=======
> ++	/*
> ++	 * Now use our persistent iterator and state, if we aren't already.
> ++	 * We've restored and mapped the iter to match.
> ++	 */
> ++	if (iter != &rw->iter) {
> ++		iter = &rw->iter;
> + 		state = &rw->iter_state;
> + 	}
> + 
> + 	do {
> + 		/*
> + 		 * We end up here because of a partial read, either from
> + 		 * above or inside this loop. Advance the iter by the bytes
> + 		 * that were consumed.
> + 		 */
> + 		iov_iter_advance(iter, ret);
> + 		if (!iov_iter_count(iter))
> + 			break;
> + 		rw->bytes_done += ret;
> + 		iov_iter_save_state(iter, state);
> + 
> + 		/* if we can retry, do so with the callbacks armed */
> + 		if (!io_rw_should_retry(req)) {
> + 			kiocb->ki_flags &= ~IOCB_WAITQ;
> + 			return -EAGAIN;
> + 		}
> + 
> + 		/*
> + 		 * Now retry read with the IOCB_WAITQ parts set in the iocb. If
> + 		 * we get -EIOCBQUEUED, then we'll get a notification when the
> + 		 * desired page gets unlocked. We can also get a partial read
> + 		 * here, and if we do, then just retry at the new offset.
> + 		 */
> + 		ret = io_iter_do_read(req, iter);
> + 		if (ret == -EIOCBQUEUED)
> + 			return 0;
> + 		/* we got some bytes, but not all. retry. */
> + 		kiocb->ki_flags &= ~IOCB_WAITQ;
> + 		iov_iter_restore(iter, state);
> + 	} while (ret > 0);
> ++>>>>>>> cd65869512ab5 (io_uring: use iov_iter state save/restore helpers)
>   done:
>  -	kiocb_done(kiocb, ret, issue_flags);
>  +	kiocb_done(kiocb, ret, cs);
>  +	ret = 0;
>   out_free:
>  -	/* it's faster to check here then delegate to kfree */
>  +	/* it's reportedly faster than delegating the null check to kfree() */
>   	if (iovec)
>   		kfree(iovec);
>  -	return 0;
>  +	return ret;
>   }
>   
>   static int io_write_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
> @@@ -3545,16 -3578,24 +3662,37 @@@ static int io_write(struct io_kiocb *re
>   	struct kiocb *kiocb = &req->rw.kiocb;
>   	struct iov_iter __iter, *iter = &__iter;
>   	struct io_async_rw *rw = req->async_data;
> ++<<<<<<< HEAD
>  +	ssize_t ret, ret2, io_size;
> ++=======
> + 	bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
> + 	struct iov_iter_state __state, *state;
> + 	ssize_t ret, ret2;
> ++>>>>>>> cd65869512ab5 (io_uring: use iov_iter state save/restore helpers)
>   
>  -	if (rw) {
>  +	if (rw)
>   		iter = &rw->iter;
> ++<<<<<<< HEAD
>  +
>  +	ret = io_import_iovec(WRITE, req, &iovec, iter, !force_nonblock);
>  +	if (ret < 0)
>  +		return ret;
>  +	io_size = iov_iter_count(iter);
>  +	req->result = io_size;
> ++=======
> + 		state = &rw->iter_state;
> + 		iov_iter_restore(iter, state);
> + 		iovec = NULL;
> + 	} else {
> + 		ret = io_import_iovec(WRITE, req, &iovec, iter, !force_nonblock);
> + 		if (ret < 0)
> + 			return ret;
> + 		state = &__state;
> + 		iov_iter_save_state(iter, state);
> + 	}
> + 	req->result = iov_iter_count(iter);
> + 	ret2 = 0;
> ++>>>>>>> cd65869512ab5 (io_uring: use iov_iter state save/restore helpers)
>   
>   	/* Ensure we clear previously set non-block flag */
>   	if (!force_nonblock)
> @@@ -3610,14 -3656,14 +3748,20 @@@
>   		if ((req->ctx->flags & IORING_SETUP_IOPOLL) && ret2 == -EAGAIN)
>   			goto copy_iov;
>   done:
>  -		kiocb_done(kiocb, ret2, issue_flags);
>  +		kiocb_done(kiocb, ret2, cs);
>   	} else {
>   copy_iov:
> ++<<<<<<< HEAD
>  +		/* some cases will consume bytes even on error returns */
>  +		iov_iter_revert(iter, io_size - iov_iter_count(iter));
> ++=======
> + 		iov_iter_restore(iter, state);
> + 		if (ret2 > 0)
> + 			iov_iter_advance(iter, ret2);
> ++>>>>>>> cd65869512ab5 (io_uring: use iov_iter state save/restore helpers)
>   		ret = io_setup_async_rw(req, iovec, inline_vecs, iter, false);
>  -		return ret ?: -EAGAIN;
>  +		if (!ret)
>  +			return -EAGAIN;
>   	}
>   out_free:
>   	/* it's reportedly faster than delegating the null check to kfree() */
> 

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [syzbot] KASAN: stack-out-of-bounds Read in iov_iter_revert
  2021-12-15  8:06             ` Lee Jones
@ 2021-12-15 11:16               ` Pavel Begunkov
  2021-12-16 17:02                 ` Lee Jones
  2022-03-21 10:52                 ` Lee Jones
  0 siblings, 2 replies; 22+ messages in thread
From: Pavel Begunkov @ 2021-12-15 11:16 UTC (permalink / raw)
  To: Lee Jones, Jens Axboe
  Cc: syzbot, io-uring, linux-kernel, syzkaller-bugs, Greg Kroah-Hartman

On 12/15/21 08:06, Lee Jones wrote:
> On Tue, 09 Nov 2021, Lee Jones wrote:
> 
>> On Mon, 08 Nov 2021, Jens Axboe wrote:
>>> On 11/8/21 8:29 AM, Pavel Begunkov wrote:
>>>> On 11/3/21 17:01, Lee Jones wrote:
>>>>> Good afternoon Pavel,
>>>>>
>>>>>> syzbot has tested the proposed patch and the reproducer did not trigger any issue:
>>>>>>
>>>>>> Reported-and-tested-by: syzbot+9671693590ef5aad8953@syzkaller.appspotmail.com
>>>>>>
>>>>>> Tested on:
>>>>>>
>>>>>> commit:         bff2c168 io_uring: don't retry with truncated iter
>>>>>> git tree:       https://github.com/isilence/linux.git truncate
>>>>>> kernel config:  https://syzkaller.appspot.com/x/.config?x=730106bfb5bf8ace
>>>>>> dashboard link: https://syzkaller.appspot.com/bug?extid=9671693590ef5aad8953
>>>>>> compiler:       Debian clang version 11.0.1-2, GNU ld (GNU Binutils for Debian) 2.35.1
>>>>>>
>>>>>> Note: testing is done by a robot and is best-effort only.
>>>>>
>>>>> As you can see in the 'dashboard link' above this bug also affects
>>>>> android-5-10 which is currently based on v5.10.75.
>>>>>
>>>>> I see that the back-port of this patch failed in v5.10.y:
>>>>>
>>>>>     https://lore.kernel.org/stable/163152589512611@kroah.com/
>>>>>
>>>>> And after solving the build-error by back-porting both:
>>>>>
>>>>>     2112ff5ce0c11 iov_iter: track truncated size
>>>>>     89c2b3b749182 io_uring: reexpand under-reexpanded iters
>>>>>
>>>>> I now see execution tripping the WARN() in iov_iter_revert():
>>>>>
>>>>>     if (WARN_ON(unroll > MAX_RW_COUNT))
>>>>>         return
>>>>>
>>>>> Am I missing any additional patches required to fix stable/v5.10.y?
>>>>
>>>> Is it the same syz test? There was a couple more patches for
>>>> IORING_SETUP_IOPOLL, but strange if that's not the case.
>>>>
>>>>
>>>> fwiw, Jens decided to replace it with another mechanism shortly
>>>> after, so it may be a better idea to backport those. Jens,
>>>> what do you think?
>>>>
>>>>
>>>> commit 8fb0f47a9d7acf620d0fd97831b69da9bc5e22ed
>>>> Author: Jens Axboe <axboe@kernel.dk>
>>>> Date:   Fri Sep 10 11:18:36 2021 -0600
>>>>
>>>>       iov_iter: add helper to save iov_iter state
>>>>
>>>> commit cd65869512ab5668a5d16f789bc4da1319c435c4
>>>> Author: Jens Axboe <axboe@kernel.dk>
>>>> Date:   Fri Sep 10 11:19:14 2021 -0600
>>>>
>>>>       io_uring: use iov_iter state save/restore helpers
>>>
>>> Yes, I think backporting based on the save/restore setup is the
>>> sanest way by far.
>>
>> Would you be kind enough to attempt to send these patches to Stable?
>>
>> When I tried to back-port them, the second one gave me trouble.  And
>> without the in depth knowledge of the driver/subsystem that you guys
>> have, I found it almost impossible to resolve all of the conflicts:
> 
> Any movement on this chaps?
> 
> Not sure I am able to do this back-port without your help.

Apologies, slipped from my attention, we'll backport it,
and thanks for the reminder


-- 
Pavel Begunkov

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [syzbot] KASAN: stack-out-of-bounds Read in iov_iter_revert
  2021-12-15 11:16               ` Pavel Begunkov
@ 2021-12-16 17:02                 ` Lee Jones
  2022-02-22 16:48                   ` Lee Jones
  2022-03-21 10:52                 ` Lee Jones
  1 sibling, 1 reply; 22+ messages in thread
From: Lee Jones @ 2021-12-16 17:02 UTC (permalink / raw)
  To: Pavel Begunkov
  Cc: Jens Axboe, syzbot, io-uring, linux-kernel, syzkaller-bugs,
	Greg Kroah-Hartman

On Wed, 15 Dec 2021, Pavel Begunkov wrote:

> On 12/15/21 08:06, Lee Jones wrote:
> > On Tue, 09 Nov 2021, Lee Jones wrote:
> > 
> > > On Mon, 08 Nov 2021, Jens Axboe wrote:
> > > > On 11/8/21 8:29 AM, Pavel Begunkov wrote:
> > > > > On 11/3/21 17:01, Lee Jones wrote:
> > > > > > Good afternoon Pavel,
> > > > > > 
> > > > > > > syzbot has tested the proposed patch and the reproducer did not trigger any issue:
> > > > > > > 
> > > > > > > Reported-and-tested-by: syzbot+9671693590ef5aad8953@syzkaller.appspotmail.com
> > > > > > > 
> > > > > > > Tested on:
> > > > > > > 
> > > > > > > commit:         bff2c168 io_uring: don't retry with truncated iter
> > > > > > > git tree:       https://github.com/isilence/linux.git truncate
> > > > > > > kernel config:  https://syzkaller.appspot.com/x/.config?x=730106bfb5bf8ace
> > > > > > > dashboard link: https://syzkaller.appspot.com/bug?extid=9671693590ef5aad8953
> > > > > > > compiler:       Debian clang version 11.0.1-2, GNU ld (GNU Binutils for Debian) 2.35.1
> > > > > > > 
> > > > > > > Note: testing is done by a robot and is best-effort only.
> > > > > > 
> > > > > > As you can see in the 'dashboard link' above this bug also affects
> > > > > > android-5-10 which is currently based on v5.10.75.
> > > > > > 
> > > > > > I see that the back-port of this patch failed in v5.10.y:
> > > > > > 
> > > > > >     https://lore.kernel.org/stable/163152589512611@kroah.com/
> > > > > > 
> > > > > > And after solving the build-error by back-porting both:
> > > > > > 
> > > > > >     2112ff5ce0c11 iov_iter: track truncated size
> > > > > >     89c2b3b749182 io_uring: reexpand under-reexpanded iters
> > > > > > 
> > > > > > I now see execution tripping the WARN() in iov_iter_revert():
> > > > > > 
> > > > > >     if (WARN_ON(unroll > MAX_RW_COUNT))
> > > > > >         return
> > > > > > 
> > > > > > Am I missing any additional patches required to fix stable/v5.10.y?
> > > > > 
> > > > > Is it the same syz test? There was a couple more patches for
> > > > > IORING_SETUP_IOPOLL, but strange if that's not the case.
> > > > > 
> > > > > 
> > > > > fwiw, Jens decided to replace it with another mechanism shortly
> > > > > after, so it may be a better idea to backport those. Jens,
> > > > > what do you think?
> > > > > 
> > > > > 
> > > > > commit 8fb0f47a9d7acf620d0fd97831b69da9bc5e22ed
> > > > > Author: Jens Axboe <axboe@kernel.dk>
> > > > > Date:   Fri Sep 10 11:18:36 2021 -0600
> > > > > 
> > > > >       iov_iter: add helper to save iov_iter state
> > > > > 
> > > > > commit cd65869512ab5668a5d16f789bc4da1319c435c4
> > > > > Author: Jens Axboe <axboe@kernel.dk>
> > > > > Date:   Fri Sep 10 11:19:14 2021 -0600
> > > > > 
> > > > >       io_uring: use iov_iter state save/restore helpers
> > > > 
> > > > Yes, I think backporting based on the save/restore setup is the
> > > > sanest way by far.
> > > 
> > > Would you be kind enough to attempt to send these patches to Stable?
> > > 
> > > When I tried to back-port them, the second one gave me trouble.  And
> > > without the in depth knowledge of the driver/subsystem that you guys
> > > have, I found it almost impossible to resolve all of the conflicts:
> > 
> > Any movement on this chaps?
> > 
> > Not sure I am able to do this back-port without your help.
> 
> Apologies, slipped from my attention, we'll backport it,
> and thanks for the reminder

Excellent.  Thanks Pavel.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [syzbot] KASAN: stack-out-of-bounds Read in iov_iter_revert
  2021-12-16 17:02                 ` Lee Jones
@ 2022-02-22 16:48                   ` Lee Jones
  0 siblings, 0 replies; 22+ messages in thread
From: Lee Jones @ 2022-02-22 16:48 UTC (permalink / raw)
  To: Pavel Begunkov
  Cc: Jens Axboe, syzbot, io-uring, linux-kernel, syzkaller-bugs,
	Greg Kroah-Hartman

On Thu, 16 Dec 2021, Lee Jones wrote:

> On Wed, 15 Dec 2021, Pavel Begunkov wrote:
> 
> > On 12/15/21 08:06, Lee Jones wrote:
> > > On Tue, 09 Nov 2021, Lee Jones wrote:
> > > 
> > > > On Mon, 08 Nov 2021, Jens Axboe wrote:
> > > > > On 11/8/21 8:29 AM, Pavel Begunkov wrote:
> > > > > > On 11/3/21 17:01, Lee Jones wrote:
> > > > > > > Good afternoon Pavel,
> > > > > > > 
> > > > > > > > syzbot has tested the proposed patch and the reproducer did not trigger any issue:
> > > > > > > > 
> > > > > > > > Reported-and-tested-by: syzbot+9671693590ef5aad8953@syzkaller.appspotmail.com
> > > > > > > > 
> > > > > > > > Tested on:
> > > > > > > > 
> > > > > > > > commit:         bff2c168 io_uring: don't retry with truncated iter
> > > > > > > > git tree:       https://github.com/isilence/linux.git truncate
> > > > > > > > kernel config:  https://syzkaller.appspot.com/x/.config?x=730106bfb5bf8ace
> > > > > > > > dashboard link: https://syzkaller.appspot.com/bug?extid=9671693590ef5aad8953
> > > > > > > > compiler:       Debian clang version 11.0.1-2, GNU ld (GNU Binutils for Debian) 2.35.1
> > > > > > > > 
> > > > > > > > Note: testing is done by a robot and is best-effort only.
> > > > > > > 
> > > > > > > As you can see in the 'dashboard link' above this bug also affects
> > > > > > > android-5-10 which is currently based on v5.10.75.
> > > > > > > 
> > > > > > > I see that the back-port of this patch failed in v5.10.y:
> > > > > > > 
> > > > > > >     https://lore.kernel.org/stable/163152589512611@kroah.com/
> > > > > > > 
> > > > > > > And after solving the build-error by back-porting both:
> > > > > > > 
> > > > > > >     2112ff5ce0c11 iov_iter: track truncated size
> > > > > > >     89c2b3b749182 io_uring: reexpand under-reexpanded iters
> > > > > > > 
> > > > > > > I now see execution tripping the WARN() in iov_iter_revert():
> > > > > > > 
> > > > > > >     if (WARN_ON(unroll > MAX_RW_COUNT))
> > > > > > >         return
> > > > > > > 
> > > > > > > Am I missing any additional patches required to fix stable/v5.10.y?
> > > > > > 
> > > > > > Is it the same syz test? There was a couple more patches for
> > > > > > IORING_SETUP_IOPOLL, but strange if that's not the case.
> > > > > > 
> > > > > > 
> > > > > > fwiw, Jens decided to replace it with another mechanism shortly
> > > > > > after, so it may be a better idea to backport those. Jens,
> > > > > > what do you think?
> > > > > > 
> > > > > > 
> > > > > > commit 8fb0f47a9d7acf620d0fd97831b69da9bc5e22ed
> > > > > > Author: Jens Axboe <axboe@kernel.dk>
> > > > > > Date:   Fri Sep 10 11:18:36 2021 -0600
> > > > > > 
> > > > > >       iov_iter: add helper to save iov_iter state
> > > > > > 
> > > > > > commit cd65869512ab5668a5d16f789bc4da1319c435c4
> > > > > > Author: Jens Axboe <axboe@kernel.dk>
> > > > > > Date:   Fri Sep 10 11:19:14 2021 -0600
> > > > > > 
> > > > > >       io_uring: use iov_iter state save/restore helpers
> > > > > 
> > > > > Yes, I think backporting based on the save/restore setup is the
> > > > > sanest way by far.
> > > > 
> > > > Would you be kind enough to attempt to send these patches to Stable?
> > > > 
> > > > When I tried to back-port them, the second one gave me trouble.  And
> > > > without the in depth knowledge of the driver/subsystem that you guys
> > > > have, I found it almost impossible to resolve all of the conflicts:
> > > 
> > > Any movement on this chaps?
> > > 
> > > Not sure I am able to do this back-port without your help.
> > 
> > Apologies, slipped from my attention, we'll backport it,
> > and thanks for the reminder
> 
> Excellent.  Thanks Pavel.

Has this now been back-ported to Stable?

If so, would you be kind enough to provide the SHA1?

Thanks.

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [syzbot] KASAN: stack-out-of-bounds Read in iov_iter_revert
  2021-12-15 11:16               ` Pavel Begunkov
  2021-12-16 17:02                 ` Lee Jones
@ 2022-03-21 10:52                 ` Lee Jones
  1 sibling, 0 replies; 22+ messages in thread
From: Lee Jones @ 2022-03-21 10:52 UTC (permalink / raw)
  To: Pavel Begunkov
  Cc: Jens Axboe, syzbot, io-uring, linux-kernel, syzkaller-bugs,
	Greg Kroah-Hartman

On Wed, 15 Dec 2021, Pavel Begunkov wrote:

> On 12/15/21 08:06, Lee Jones wrote:
> > On Tue, 09 Nov 2021, Lee Jones wrote:
> > 
> > > On Mon, 08 Nov 2021, Jens Axboe wrote:
> > > > On 11/8/21 8:29 AM, Pavel Begunkov wrote:
> > > > > On 11/3/21 17:01, Lee Jones wrote:
> > > > > > Good afternoon Pavel,
> > > > > > 
> > > > > > > syzbot has tested the proposed patch and the reproducer did not trigger any issue:
> > > > > > > 
> > > > > > > Reported-and-tested-by: syzbot+9671693590ef5aad8953@syzkaller.appspotmail.com
> > > > > > > 
> > > > > > > Tested on:
> > > > > > > 
> > > > > > > commit:         bff2c168 io_uring: don't retry with truncated iter
> > > > > > > git tree:       https://github.com/isilence/linux.git truncate
> > > > > > > kernel config:  https://syzkaller.appspot.com/x/.config?x=730106bfb5bf8ace
> > > > > > > dashboard link: https://syzkaller.appspot.com/bug?extid=9671693590ef5aad8953
> > > > > > > compiler:       Debian clang version 11.0.1-2, GNU ld (GNU Binutils for Debian) 2.35.1
> > > > > > > 
> > > > > > > Note: testing is done by a robot and is best-effort only.
> > > > > > 
> > > > > > As you can see in the 'dashboard link' above this bug also affects
> > > > > > android-5-10 which is currently based on v5.10.75.
> > > > > > 
> > > > > > I see that the back-port of this patch failed in v5.10.y:
> > > > > > 
> > > > > >     https://lore.kernel.org/stable/163152589512611@kroah.com/
> > > > > > 
> > > > > > And after solving the build-error by back-porting both:
> > > > > > 
> > > > > >     2112ff5ce0c11 iov_iter: track truncated size
> > > > > >     89c2b3b749182 io_uring: reexpand under-reexpanded iters
> > > > > > 
> > > > > > I now see execution tripping the WARN() in iov_iter_revert():
> > > > > > 
> > > > > >     if (WARN_ON(unroll > MAX_RW_COUNT))
> > > > > >         return
> > > > > > 
> > > > > > Am I missing any additional patches required to fix stable/v5.10.y?
> > > > > 
> > > > > Is it the same syz test? There was a couple more patches for
> > > > > IORING_SETUP_IOPOLL, but strange if that's not the case.
> > > > > 
> > > > > 
> > > > > fwiw, Jens decided to replace it with another mechanism shortly
> > > > > after, so it may be a better idea to backport those. Jens,
> > > > > what do you think?
> > > > > 
> > > > > 
> > > > > commit 8fb0f47a9d7acf620d0fd97831b69da9bc5e22ed
> > > > > Author: Jens Axboe <axboe@kernel.dk>
> > > > > Date:   Fri Sep 10 11:18:36 2021 -0600
> > > > > 
> > > > >       iov_iter: add helper to save iov_iter state
> > > > > 
> > > > > commit cd65869512ab5668a5d16f789bc4da1319c435c4
> > > > > Author: Jens Axboe <axboe@kernel.dk>
> > > > > Date:   Fri Sep 10 11:19:14 2021 -0600
> > > > > 
> > > > >       io_uring: use iov_iter state save/restore helpers
> > > > 
> > > > Yes, I think backporting based on the save/restore setup is the
> > > > sanest way by far.
> > > 
> > > Would you be kind enough to attempt to send these patches to Stable?
> > > 
> > > When I tried to back-port them, the second one gave me trouble.  And
> > > without the in depth knowledge of the driver/subsystem that you guys
> > > have, I found it almost impossible to resolve all of the conflicts:
> > 
> > Any movement on this chaps?
> > 
> > Not sure I am able to do this back-port without your help.
> 
> Apologies, slipped from my attention, we'll backport it,
> and thanks for the reminder

Looks as though these are still missing from Stable.

Is this still your plan?

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 22+ messages in thread

* linux-stable-5.10-y CVE-2022-1508 of io_uring module
  2021-11-08 15:41         ` Jens Axboe
  2021-11-09 13:33           ` Lee Jones
@ 2022-05-05 14:11           ` Guo Xuenan
  2022-05-06  2:16             ` Jens Axboe
  1 sibling, 1 reply; 22+ messages in thread
From: Guo Xuenan @ 2022-05-05 14:11 UTC (permalink / raw)
  To: axboe, asml.silence
  Cc: lee.jones, linux-kernel, io-uring, guoxuenan, yi.zhang, houtao1

Hi, Pavel & Jens

CVE-2022-1508[1] contains an patch[2] of io_uring. As Jones reported,
it is not enough only apply [2] to stable-5.10. 
Io_uring is very valuable and active module of linux kernel.
I've tried to apply these two patches[3] [4] to my local 5.10 code, I
found my understanding of io_uring is not enough to resolve all conflicts.

Since 5.10 is an important stable branch of linux, we would appreciate
your help in soloving this problem.

[1] https://access.redhat.com/security/cve/cve-2022-1508
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=89c2b3b7491
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8fb0f47a9d7
[4] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=cd65869512a

Best regards
Xuenan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: linux-stable-5.10-y CVE-2022-1508 of io_uring module
  2022-05-05 14:11           ` linux-stable-5.10-y CVE-2022-1508 of io_uring module Guo Xuenan
@ 2022-05-06  2:16             ` Jens Axboe
  2022-05-06 15:57               ` Pavel Begunkov
  0 siblings, 1 reply; 22+ messages in thread
From: Jens Axboe @ 2022-05-06  2:16 UTC (permalink / raw)
  To: Guo Xuenan, asml.silence
  Cc: lee.jones, linux-kernel, io-uring, yi.zhang, houtao1

On 5/5/22 8:11 AM, Guo Xuenan wrote:
> Hi, Pavel & Jens
> 
> CVE-2022-1508[1] contains an patch[2] of io_uring. As Jones reported,
> it is not enough only apply [2] to stable-5.10. 
> Io_uring is very valuable and active module of linux kernel.
> I've tried to apply these two patches[3] [4] to my local 5.10 code, I
> found my understanding of io_uring is not enough to resolve all conflicts.
> 
> Since 5.10 is an important stable branch of linux, we would appreciate
> your help in solving this problem.

Yes, this really needs to get buttoned up for 5.10. I seem to recall
there was a reproducer for this that was somewhat saner than the
syzbot one (which doesn't do anything for me). Pavel, do you have one?

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: linux-stable-5.10-y CVE-2022-1508 of io_uring module
  2022-05-06  2:16             ` Jens Axboe
@ 2022-05-06 15:57               ` Pavel Begunkov
  2022-05-06 16:15                 ` Jens Axboe
  0 siblings, 1 reply; 22+ messages in thread
From: Pavel Begunkov @ 2022-05-06 15:57 UTC (permalink / raw)
  To: Jens Axboe, Guo Xuenan
  Cc: lee.jones, linux-kernel, io-uring, yi.zhang, houtao1

On 5/6/22 03:16, Jens Axboe wrote:
> On 5/5/22 8:11 AM, Guo Xuenan wrote:
>> Hi, Pavel & Jens
>>
>> CVE-2022-1508[1] contains an patch[2] of io_uring. As Jones reported,
>> it is not enough only apply [2] to stable-5.10.
>> Io_uring is very valuable and active module of linux kernel.
>> I've tried to apply these two patches[3] [4] to my local 5.10 code, I
>> found my understanding of io_uring is not enough to resolve all conflicts.
>>
>> Since 5.10 is an important stable branch of linux, we would appreciate
>> your help in solving this problem.
> 
> Yes, this really needs to get buttoned up for 5.10. I seem to recall
> there was a reproducer for this that was somewhat saner than the
> syzbot one (which doesn't do anything for me). Pavel, do you have one?

No, it was the only repro and was triggering the problem
just fine back then

-- 
Pavel Begunkov

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: linux-stable-5.10-y CVE-2022-1508 of io_uring module
  2022-05-06 15:57               ` Pavel Begunkov
@ 2022-05-06 16:15                 ` Jens Axboe
  2022-05-06 18:22                   ` Jens Axboe
  0 siblings, 1 reply; 22+ messages in thread
From: Jens Axboe @ 2022-05-06 16:15 UTC (permalink / raw)
  To: Pavel Begunkov, Guo Xuenan
  Cc: lee.jones, linux-kernel, io-uring, yi.zhang, houtao1

On 5/6/22 9:57 AM, Pavel Begunkov wrote:
> On 5/6/22 03:16, Jens Axboe wrote:
>> On 5/5/22 8:11 AM, Guo Xuenan wrote:
>>> Hi, Pavel & Jens
>>>
>>> CVE-2022-1508[1] contains an patch[2] of io_uring. As Jones reported,
>>> it is not enough only apply [2] to stable-5.10.
>>> Io_uring is very valuable and active module of linux kernel.
>>> I've tried to apply these two patches[3] [4] to my local 5.10 code, I
>>> found my understanding of io_uring is not enough to resolve all conflicts.
>>>
>>> Since 5.10 is an important stable branch of linux, we would appreciate
>>> your help in solving this problem.
>>
>> Yes, this really needs to get buttoned up for 5.10. I seem to recall
>> there was a reproducer for this that was somewhat saner than the
>> syzbot one (which doesn't do anything for me). Pavel, do you have one?
> 
> No, it was the only repro and was triggering the problem
> just fine back then

I modified it a bit and I can now trigger it.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: linux-stable-5.10-y CVE-2022-1508 of io_uring module
  2022-05-06 16:15                 ` Jens Axboe
@ 2022-05-06 18:22                   ` Jens Axboe
  2022-05-07  9:16                     ` Pavel Begunkov
  0 siblings, 1 reply; 22+ messages in thread
From: Jens Axboe @ 2022-05-06 18:22 UTC (permalink / raw)
  To: Pavel Begunkov, Guo Xuenan
  Cc: lee.jones, linux-kernel, io-uring, yi.zhang, houtao1

On 5/6/22 10:15 AM, Jens Axboe wrote:
> On 5/6/22 9:57 AM, Pavel Begunkov wrote:
>> On 5/6/22 03:16, Jens Axboe wrote:
>>> On 5/5/22 8:11 AM, Guo Xuenan wrote:
>>>> Hi, Pavel & Jens
>>>>
>>>> CVE-2022-1508[1] contains an patch[2] of io_uring. As Jones reported,
>>>> it is not enough only apply [2] to stable-5.10.
>>>> Io_uring is very valuable and active module of linux kernel.
>>>> I've tried to apply these two patches[3] [4] to my local 5.10 code, I
>>>> found my understanding of io_uring is not enough to resolve all conflicts.
>>>>
>>>> Since 5.10 is an important stable branch of linux, we would appreciate
>>>> your help in solving this problem.
>>>
>>> Yes, this really needs to get buttoned up for 5.10. I seem to recall
>>> there was a reproducer for this that was somewhat saner than the
>>> syzbot one (which doesn't do anything for me). Pavel, do you have one?
>>
>> No, it was the only repro and was triggering the problem
>> just fine back then
> 
> I modified it a bit and I can now trigger it.

Pavel, why don't we just keep it really simple and just always save the
iter state in read/write, and use the restore instead of the revert?

Then it's just a trivial backport of ff6165b2d7f6 and the trivial
io_uring patch after that.


diff --git a/fs/io_uring.c b/fs/io_uring.c
index ab9290ab4cae..138f204db72a 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -3429,6 +3429,7 @@ static int io_read(struct io_kiocb *req, bool force_nonblock,
 	struct kiocb *kiocb = &req->rw.kiocb;
 	struct iov_iter __iter, *iter = &__iter;
 	struct io_async_rw *rw = req->async_data;
+	struct iov_iter_state iter_state;
 	ssize_t io_size, ret, ret2;
 	bool no_async;
 
@@ -3458,6 +3459,7 @@ static int io_read(struct io_kiocb *req, bool force_nonblock,
 	if (unlikely(ret))
 		goto out_free;
 
+	iov_iter_save_state(iter, &iter_state);
 	ret = io_iter_do_read(req, iter);
 
 	if (!ret) {
@@ -3473,7 +3475,7 @@ static int io_read(struct io_kiocb *req, bool force_nonblock,
 		if (req->file->f_flags & O_NONBLOCK)
 			goto done;
 		/* some cases will consume bytes even on error returns */
-		iov_iter_revert(iter, io_size - iov_iter_count(iter));
+		iov_iter_restore(iter, &iter_state);
 		ret = 0;
 		goto copy_iov;
 	} else if (ret < 0) {
@@ -3557,6 +3559,7 @@ static int io_write(struct io_kiocb *req, bool force_nonblock,
 	struct kiocb *kiocb = &req->rw.kiocb;
 	struct iov_iter __iter, *iter = &__iter;
 	struct io_async_rw *rw = req->async_data;
+	struct iov_iter_state iter_state;
 	ssize_t ret, ret2, io_size;
 
 	if (rw)
@@ -3574,6 +3577,8 @@ static int io_write(struct io_kiocb *req, bool force_nonblock,
 	else
 		kiocb->ki_flags |= IOCB_NOWAIT;
 
+	iov_iter_save_state(iter, &iter_state);
+
 	/* If the file doesn't support async, just async punt */
 	if (force_nonblock && !io_file_supports_async(req->file, WRITE))
 		goto copy_iov;
@@ -3626,7 +3631,7 @@ static int io_write(struct io_kiocb *req, bool force_nonblock,
 	} else {
 copy_iov:
 		/* some cases will consume bytes even on error returns */
-		iov_iter_revert(iter, io_size - iov_iter_count(iter));
+		iov_iter_restore(iter, &iter_state);
 		ret = io_setup_async_rw(req, iovec, inline_vecs, iter, false);
 		if (!ret)
 			return -EAGAIN;
diff --git a/include/linux/uio.h b/include/linux/uio.h
index 27ff8eb786dc..cedb68e49e4f 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -26,6 +26,12 @@ enum iter_type {
 	ITER_DISCARD = 64,
 };
 
+struct iov_iter_state {
+	size_t iov_offset;
+	size_t count;
+	unsigned long nr_segs;
+};
+
 struct iov_iter {
 	/*
 	 * Bit 0 is the read/write bit, set if we're writing.
@@ -55,6 +61,14 @@ static inline enum iter_type iov_iter_type(const struct iov_iter *i)
 	return i->type & ~(READ | WRITE);
 }
 
+static inline void iov_iter_save_state(struct iov_iter *iter,
+				       struct iov_iter_state *state)
+{
+	state->iov_offset = iter->iov_offset;
+	state->count = iter->count;
+	state->nr_segs = iter->nr_segs;
+}
+
 static inline bool iter_is_iovec(const struct iov_iter *i)
 {
 	return iov_iter_type(i) == ITER_IOVEC;
@@ -226,6 +240,7 @@ ssize_t iov_iter_get_pages(struct iov_iter *i, struct page **pages,
 ssize_t iov_iter_get_pages_alloc(struct iov_iter *i, struct page ***pages,
 			size_t maxsize, size_t *start);
 int iov_iter_npages(const struct iov_iter *i, int maxpages);
+void iov_iter_restore(struct iov_iter *i, struct iov_iter_state *state);
 
 const void *dup_iter(struct iov_iter *new, struct iov_iter *old, gfp_t flags);
 
diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 1b0a349fbcd9..00a66229d182 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -1857,3 +1857,39 @@ int iov_iter_for_each_range(struct iov_iter *i, size_t bytes,
 	return err;
 }
 EXPORT_SYMBOL(iov_iter_for_each_range);
+
+/**
+ * iov_iter_restore() - Restore a &struct iov_iter to the same state as when
+ *     iov_iter_save_state() was called.
+ *
+ * @i: &struct iov_iter to restore
+ * @state: state to restore from
+ *
+ * Used after iov_iter_save_state() to bring restore @i, if operations may
+ * have advanced it.
+ *
+ * Note: only works on ITER_IOVEC, ITER_BVEC, and ITER_KVEC
+ */
+void iov_iter_restore(struct iov_iter *i, struct iov_iter_state *state)
+{
+	if (WARN_ON_ONCE(!iov_iter_is_bvec(i) && !iter_is_iovec(i)) &&
+			 !iov_iter_is_kvec(i))
+		return;
+	i->iov_offset = state->iov_offset;
+	i->count = state->count;
+	/*
+	 * For the *vec iters, nr_segs + iov is constant - if we increment
+	 * the vec, then we also decrement the nr_segs count. Hence we don't
+	 * need to track both of these, just one is enough and we can deduct
+	 * the other from that. ITER_KVEC and ITER_IOVEC are the same struct
+	 * size, so we can just increment the iov pointer as they are unionzed.
+	 * ITER_BVEC _may_ be the same size on some archs, but on others it is
+	 * not. Be safe and handle it separately.
+	 */
+	BUILD_BUG_ON(sizeof(struct iovec) != sizeof(struct kvec));
+	if (iov_iter_is_bvec(i))
+		i->bvec -= state->nr_segs - i->nr_segs;
+	else
+		i->iov -= state->nr_segs - i->nr_segs;
+	i->nr_segs = state->nr_segs;
+}

-- 
Jens Axboe


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: linux-stable-5.10-y CVE-2022-1508 of io_uring module
  2022-05-06 18:22                   ` Jens Axboe
@ 2022-05-07  9:16                     ` Pavel Begunkov
  2022-05-07 14:18                       ` Jens Axboe
  0 siblings, 1 reply; 22+ messages in thread
From: Pavel Begunkov @ 2022-05-07  9:16 UTC (permalink / raw)
  To: Jens Axboe, Guo Xuenan
  Cc: lee.jones, linux-kernel, io-uring, yi.zhang, houtao1

On 5/6/22 19:22, Jens Axboe wrote:
> On 5/6/22 10:15 AM, Jens Axboe wrote:
>> On 5/6/22 9:57 AM, Pavel Begunkov wrote:
>>> On 5/6/22 03:16, Jens Axboe wrote:
>>>> On 5/5/22 8:11 AM, Guo Xuenan wrote:
>>>>> Hi, Pavel & Jens
>>>>>
>>>>> CVE-2022-1508[1] contains an patch[2] of io_uring. As Jones reported,
>>>>> it is not enough only apply [2] to stable-5.10.
>>>>> Io_uring is very valuable and active module of linux kernel.
>>>>> I've tried to apply these two patches[3] [4] to my local 5.10 code, I
>>>>> found my understanding of io_uring is not enough to resolve all conflicts.
>>>>>
>>>>> Since 5.10 is an important stable branch of linux, we would appreciate
>>>>> your help in solving this problem.
>>>>
>>>> Yes, this really needs to get buttoned up for 5.10. I seem to recall
>>>> there was a reproducer for this that was somewhat saner than the
>>>> syzbot one (which doesn't do anything for me). Pavel, do you have one?
>>>
>>> No, it was the only repro and was triggering the problem
>>> just fine back then
>>
>> I modified it a bit and I can now trigger it.
> 
> Pavel, why don't we just keep it really simple and just always save the
> iter state in read/write, and use the restore instead of the revert?

The problem here is where we're doing revert. If it's done deep in
the stack and then while unwinding someone decides to revert it again,
e.g. blkdev_read_iter(), we're screwed.

The last attempt was backporting 20+ patches that would move revert
into io_read/io_write, i.e. REQ_F_REISSUE, back that failed some of
your tests back then. (was it read retry tests iirc?)


> Then it's just a trivial backport of ff6165b2d7f6 and the trivial
> io_uring patch after that.
> 
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index ab9290ab4cae..138f204db72a 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -3429,6 +3429,7 @@ static int io_read(struct io_kiocb *req, bool force_nonblock,
>   	struct kiocb *kiocb = &req->rw.kiocb;
>   	struct iov_iter __iter, *iter = &__iter;
>   	struct io_async_rw *rw = req->async_data;
> +	struct iov_iter_state iter_state;
>   	ssize_t io_size, ret, ret2;
>   	bool no_async;
>   
> @@ -3458,6 +3459,7 @@ static int io_read(struct io_kiocb *req, bool force_nonblock,
>   	if (unlikely(ret))
>   		goto out_free;
>   
> +	iov_iter_save_state(iter, &iter_state);
>   	ret = io_iter_do_read(req, iter);
>   
>   	if (!ret) {
> @@ -3473,7 +3475,7 @@ static int io_read(struct io_kiocb *req, bool force_nonblock,
>   		if (req->file->f_flags & O_NONBLOCK)
>   			goto done;
>   		/* some cases will consume bytes even on error returns */
> -		iov_iter_revert(iter, io_size - iov_iter_count(iter));
> +		iov_iter_restore(iter, &iter_state);
>   		ret = 0;
>   		goto copy_iov;
>   	} else if (ret < 0) {
> @@ -3557,6 +3559,7 @@ static int io_write(struct io_kiocb *req, bool force_nonblock,
>   	struct kiocb *kiocb = &req->rw.kiocb;
>   	struct iov_iter __iter, *iter = &__iter;
>   	struct io_async_rw *rw = req->async_data;
> +	struct iov_iter_state iter_state;
>   	ssize_t ret, ret2, io_size;
>   
>   	if (rw)
> @@ -3574,6 +3577,8 @@ static int io_write(struct io_kiocb *req, bool force_nonblock,
>   	else
>   		kiocb->ki_flags |= IOCB_NOWAIT;
>   
> +	iov_iter_save_state(iter, &iter_state);
> +
>   	/* If the file doesn't support async, just async punt */
>   	if (force_nonblock && !io_file_supports_async(req->file, WRITE))
>   		goto copy_iov;
> @@ -3626,7 +3631,7 @@ static int io_write(struct io_kiocb *req, bool force_nonblock,
>   	} else {
>   copy_iov:
>   		/* some cases will consume bytes even on error returns */
> -		iov_iter_revert(iter, io_size - iov_iter_count(iter));
> +		iov_iter_restore(iter, &iter_state);
>   		ret = io_setup_async_rw(req, iovec, inline_vecs, iter, false);
>   		if (!ret)
>   			return -EAGAIN;
> diff --git a/include/linux/uio.h b/include/linux/uio.h
> index 27ff8eb786dc..cedb68e49e4f 100644
> --- a/include/linux/uio.h
> +++ b/include/linux/uio.h
> @@ -26,6 +26,12 @@ enum iter_type {
>   	ITER_DISCARD = 64,
>   };
>   
> +struct iov_iter_state {
> +	size_t iov_offset;
> +	size_t count;
> +	unsigned long nr_segs;
> +};
> +
>   struct iov_iter {
>   	/*
>   	 * Bit 0 is the read/write bit, set if we're writing.
> @@ -55,6 +61,14 @@ static inline enum iter_type iov_iter_type(const struct iov_iter *i)
>   	return i->type & ~(READ | WRITE);
>   }
>   
> +static inline void iov_iter_save_state(struct iov_iter *iter,
> +				       struct iov_iter_state *state)
> +{
> +	state->iov_offset = iter->iov_offset;
> +	state->count = iter->count;
> +	state->nr_segs = iter->nr_segs;
> +}
> +
>   static inline bool iter_is_iovec(const struct iov_iter *i)
>   {
>   	return iov_iter_type(i) == ITER_IOVEC;
> @@ -226,6 +240,7 @@ ssize_t iov_iter_get_pages(struct iov_iter *i, struct page **pages,
>   ssize_t iov_iter_get_pages_alloc(struct iov_iter *i, struct page ***pages,
>   			size_t maxsize, size_t *start);
>   int iov_iter_npages(const struct iov_iter *i, int maxpages);
> +void iov_iter_restore(struct iov_iter *i, struct iov_iter_state *state);
>   
>   const void *dup_iter(struct iov_iter *new, struct iov_iter *old, gfp_t flags);
>   
> diff --git a/lib/iov_iter.c b/lib/iov_iter.c
> index 1b0a349fbcd9..00a66229d182 100644
> --- a/lib/iov_iter.c
> +++ b/lib/iov_iter.c
> @@ -1857,3 +1857,39 @@ int iov_iter_for_each_range(struct iov_iter *i, size_t bytes,
>   	return err;
>   }
>   EXPORT_SYMBOL(iov_iter_for_each_range);
> +
> +/**
> + * iov_iter_restore() - Restore a &struct iov_iter to the same state as when
> + *     iov_iter_save_state() was called.
> + *
> + * @i: &struct iov_iter to restore
> + * @state: state to restore from
> + *
> + * Used after iov_iter_save_state() to bring restore @i, if operations may
> + * have advanced it.
> + *
> + * Note: only works on ITER_IOVEC, ITER_BVEC, and ITER_KVEC
> + */
> +void iov_iter_restore(struct iov_iter *i, struct iov_iter_state *state)
> +{
> +	if (WARN_ON_ONCE(!iov_iter_is_bvec(i) && !iter_is_iovec(i)) &&
> +			 !iov_iter_is_kvec(i))
> +		return;
> +	i->iov_offset = state->iov_offset;
> +	i->count = state->count;
> +	/*
> +	 * For the *vec iters, nr_segs + iov is constant - if we increment
> +	 * the vec, then we also decrement the nr_segs count. Hence we don't
> +	 * need to track both of these, just one is enough and we can deduct
> +	 * the other from that. ITER_KVEC and ITER_IOVEC are the same struct
> +	 * size, so we can just increment the iov pointer as they are unionzed.
> +	 * ITER_BVEC _may_ be the same size on some archs, but on others it is
> +	 * not. Be safe and handle it separately.
> +	 */
> +	BUILD_BUG_ON(sizeof(struct iovec) != sizeof(struct kvec));
> +	if (iov_iter_is_bvec(i))
> +		i->bvec -= state->nr_segs - i->nr_segs;
> +	else
> +		i->iov -= state->nr_segs - i->nr_segs;
> +	i->nr_segs = state->nr_segs;
> +}
> 

-- 
Pavel Begunkov

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: linux-stable-5.10-y CVE-2022-1508 of io_uring module
  2022-05-07  9:16                     ` Pavel Begunkov
@ 2022-05-07 14:18                       ` Jens Axboe
  2022-05-08 11:43                         ` Pavel Begunkov
  0 siblings, 1 reply; 22+ messages in thread
From: Jens Axboe @ 2022-05-07 14:18 UTC (permalink / raw)
  To: Pavel Begunkov, Guo Xuenan
  Cc: lee.jones, linux-kernel, io-uring, yi.zhang, houtao1

On 5/7/22 3:16 AM, Pavel Begunkov wrote:
> On 5/6/22 19:22, Jens Axboe wrote:
>> On 5/6/22 10:15 AM, Jens Axboe wrote:
>>> On 5/6/22 9:57 AM, Pavel Begunkov wrote:
>>>> On 5/6/22 03:16, Jens Axboe wrote:
>>>>> On 5/5/22 8:11 AM, Guo Xuenan wrote:
>>>>>> Hi, Pavel & Jens
>>>>>>
>>>>>> CVE-2022-1508[1] contains an patch[2] of io_uring. As Jones reported,
>>>>>> it is not enough only apply [2] to stable-5.10.
>>>>>> Io_uring is very valuable and active module of linux kernel.
>>>>>> I've tried to apply these two patches[3] [4] to my local 5.10 code, I
>>>>>> found my understanding of io_uring is not enough to resolve all conflicts.
>>>>>>
>>>>>> Since 5.10 is an important stable branch of linux, we would appreciate
>>>>>> your help in solving this problem.
>>>>>
>>>>> Yes, this really needs to get buttoned up for 5.10. I seem to recall
>>>>> there was a reproducer for this that was somewhat saner than the
>>>>> syzbot one (which doesn't do anything for me). Pavel, do you have one?
>>>>
>>>> No, it was the only repro and was triggering the problem
>>>> just fine back then
>>>
>>> I modified it a bit and I can now trigger it.
>>
>> Pavel, why don't we just keep it really simple and just always save the
>> iter state in read/write, and use the restore instead of the revert?
> 
> The problem here is where we're doing revert. If it's done deep in
> the stack and then while unwinding someone decides to revert it again,
> e.g. blkdev_read_iter(), we're screwed.
> 
> The last attempt was backporting 20+ patches that would move revert
> into io_read/io_write, i.e. REQ_F_REISSUE, back that failed some of
> your tests back then. (was it read retry tests iirc?)

Do you still have that series? Yes, if I recall correctly, the series
had an issue with the resubmit. Which might just be minor, I don't
believe we really took a closer look at that.

Let's resurrect that series and see if we can pull it to completion,
would be nice to finally close the chapter on this issue for 5.10...

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: linux-stable-5.10-y CVE-2022-1508 of io_uring module
  2022-05-07 14:18                       ` Jens Axboe
@ 2022-05-08 11:43                         ` Pavel Begunkov
  0 siblings, 0 replies; 22+ messages in thread
From: Pavel Begunkov @ 2022-05-08 11:43 UTC (permalink / raw)
  To: Jens Axboe, Guo Xuenan
  Cc: lee.jones, linux-kernel, io-uring, yi.zhang, houtao1

On 5/7/22 15:18, Jens Axboe wrote:
> On 5/7/22 3:16 AM, Pavel Begunkov wrote:
>> On 5/6/22 19:22, Jens Axboe wrote:
>>> On 5/6/22 10:15 AM, Jens Axboe wrote:
>>>> On 5/6/22 9:57 AM, Pavel Begunkov wrote:
>>>>> On 5/6/22 03:16, Jens Axboe wrote:
>>>>>> On 5/5/22 8:11 AM, Guo Xuenan wrote:
>>>>>>> Hi, Pavel & Jens
>>>>>>>
>>>>>>> CVE-2022-1508[1] contains an patch[2] of io_uring. As Jones reported,
>>>>>>> it is not enough only apply [2] to stable-5.10.
>>>>>>> Io_uring is very valuable and active module of linux kernel.
>>>>>>> I've tried to apply these two patches[3] [4] to my local 5.10 code, I
>>>>>>> found my understanding of io_uring is not enough to resolve all conflicts.
>>>>>>>
>>>>>>> Since 5.10 is an important stable branch of linux, we would appreciate
>>>>>>> your help in solving this problem.
>>>>>>
>>>>>> Yes, this really needs to get buttoned up for 5.10. I seem to recall
>>>>>> there was a reproducer for this that was somewhat saner than the
>>>>>> syzbot one (which doesn't do anything for me). Pavel, do you have one?
>>>>>
>>>>> No, it was the only repro and was triggering the problem
>>>>> just fine back then
>>>>
>>>> I modified it a bit and I can now trigger it.
>>>
>>> Pavel, why don't we just keep it really simple and just always save the
>>> iter state in read/write, and use the restore instead of the revert?
>>
>> The problem here is where we're doing revert. If it's done deep in
>> the stack and then while unwinding someone decides to revert it again,
>> e.g. blkdev_read_iter(), we're screwed.
>>
>> The last attempt was backporting 20+ patches that would move revert
>> into io_read/io_write, i.e. REQ_F_REISSUE, back that failed some of
>> your tests back then. (was it read retry tests iirc?)
> 
> Do you still have that series? Yes, if I recall correctly, the series

Yep, still in the repo:

https://github.com/isilence/linux/tree/5.10_revert

> had an issue with the resubmit. Which might just be minor, I don't
> believe we really took a closer look at that.
> 
> Let's resurrect that series and see if we can pull it to completion,
> would be nice to finally close the chapter on this issue for 5.10...

We can try, but I'm not too comfortable with those backports, I had
to considerably rewrite last three patches or so. Another option
is to disable retries from the rw callback if the iter has been
truncated.

-- 
Pavel Begunkov

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2022-05-08 11:44 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-12 13:43 [syzbot] KASAN: stack-out-of-bounds Read in iov_iter_revert syzbot
2021-08-12 14:30 ` Pavel Begunkov
2021-08-12 20:36   ` syzbot
2021-11-03 17:01     ` Lee Jones
2021-11-08 15:29       ` Pavel Begunkov
2021-11-08 15:41         ` Jens Axboe
2021-11-09 13:33           ` Lee Jones
2021-12-15  8:06             ` Lee Jones
2021-12-15 11:16               ` Pavel Begunkov
2021-12-16 17:02                 ` Lee Jones
2022-02-22 16:48                   ` Lee Jones
2022-03-21 10:52                 ` Lee Jones
2022-05-05 14:11           ` linux-stable-5.10-y CVE-2022-1508 of io_uring module Guo Xuenan
2022-05-06  2:16             ` Jens Axboe
2022-05-06 15:57               ` Pavel Begunkov
2022-05-06 16:15                 ` Jens Axboe
2022-05-06 18:22                   ` Jens Axboe
2022-05-07  9:16                     ` Pavel Begunkov
2022-05-07 14:18                       ` Jens Axboe
2022-05-08 11:43                         ` Pavel Begunkov
2021-08-23  0:24 ` [syzbot] KASAN: stack-out-of-bounds Read in iov_iter_revert Pavel Begunkov
2021-08-23  0:44   ` syzbot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).