BUG: unable to handle kernel NULL pointer dereference in mem_serial_out

BUG: unable to handle kernel NULL pointer dereference in mem_serial_out

[syzbot]

Hello,

syzbot found the following crash on:

HEAD commit:    7ada90eb Merge tag 'drm-next-2019-12-06' of git://anongit...
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=123ec282e00000
kernel config:  https://syzkaller.appspot.com/x/.config?x=f07a23020fd7d21a
dashboard link: https://syzkaller.appspot.com/bug?extid=f4f1e871965064ae689e
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=17ab090ee00000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=17f127f2e00000

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

BUG: kernel NULL pointer dereference, address: 0000000000000002
#PF: supervisor write access in kernel mode
#PF: error_code(0x0002) - not-present page
PGD 9764a067 P4D 9764a067 PUD 9f995067 PMD 0
Oops: 0002 [#1] PREEMPT SMP KASAN
CPU: 0 PID: 9687 Comm: syz-executor433 Not tainted 5.4.0-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
RIP: 0010:writeb arch/x86/include/asm/io.h:65 [inline]
RIP: 0010:mem_serial_out+0x70/0x90 drivers/tty/serial/8250/8250_port.c:408
Code: e9 00 00 00 49 8d 7c 24 40 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48  
c1 ea 03 d3 e3 80 3c 02 00 75 19 48 63 db 49 03 5c 24 40 <44> 88 2b 5b 41  
5c 41 5d 5d c3 e8 81 ed cf fd eb c0 e8 da ed cf fd
RSP: 0018:ffffc90001de78e8 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: 0000000000000002 RCX: 0000000000000000
RDX: 1ffffffff181f40e RSI: ffffffff83e28776 RDI: ffffffff8c0fa070
RBP: ffffc90001de7900 R08: ffff8880919dc340 R09: ffffed10431ee1c6
R10: ffffed10431ee1c5 R11: ffff888218f70e2b R12: ffffffff8c0fa030
R13: 0000000000000001 R14: ffffc90001de7a40 R15: ffffffff8c0fa188
FS:  0000000001060880(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000002 CR3: 000000009e6b8000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
  serial_out drivers/tty/serial/8250/8250.h:118 [inline]
  serial8250_clear_fifos.part.0+0x3a/0xb0  
drivers/tty/serial/8250/8250_port.c:557
  serial8250_clear_fifos drivers/tty/serial/8250/8250_port.c:556 [inline]
  serial8250_do_startup+0x426/0x1cf0 drivers/tty/serial/8250/8250_port.c:2121
  serial8250_startup+0x62/0x80 drivers/tty/serial/8250/8250_port.c:2329
  uart_port_startup drivers/tty/serial/serial_core.c:219 [inline]
  uart_startup drivers/tty/serial/serial_core.c:258 [inline]
  uart_startup+0x452/0x980 drivers/tty/serial/serial_core.c:249
  uart_set_info drivers/tty/serial/serial_core.c:998 [inline]
  uart_set_info_user+0x13b4/0x1cf0 drivers/tty/serial/serial_core.c:1023
  tty_tiocsserial drivers/tty/tty_io.c:2506 [inline]
  tty_ioctl+0xf60/0x14f0 drivers/tty/tty_io.c:2648
  vfs_ioctl fs/ioctl.c:47 [inline]
  file_ioctl fs/ioctl.c:545 [inline]
  do_vfs_ioctl+0x977/0x14e0 fs/ioctl.c:732
  ksys_ioctl+0xab/0xd0 fs/ioctl.c:749
  __do_sys_ioctl fs/ioctl.c:756 [inline]
  __se_sys_ioctl fs/ioctl.c:754 [inline]
  __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:754
  do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x440219
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 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 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffced648c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000440219
RDX: 0000000020000240 RSI: 000000000000541f RDI: 0000000000000003
RBP: 00000000006ca018 R08: 0000000000000000 R09: 00000000004002c8
R10: 0000000000401b30 R11: 0000000000000246 R12: 0000000000401aa0
R13: 0000000000401b30 R14: 0000000000000000 R15: 0000000000000000
Modules linked in:
CR2: 0000000000000002
---[ end trace eaa11ffe82f3a763 ]---
RIP: 0010:writeb arch/x86/include/asm/io.h:65 [inline]
RIP: 0010:mem_serial_out+0x70/0x90 drivers/tty/serial/8250/8250_port.c:408
Code: e9 00 00 00 49 8d 7c 24 40 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48  
c1 ea 03 d3 e3 80 3c 02 00 75 19 48 63 db 49 03 5c 24 40 <44> 88 2b 5b 41  
5c 41 5d 5d c3 e8 81 ed cf fd eb c0 e8 da ed cf fd
RSP: 0018:ffffc90001de78e8 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: 0000000000000002 RCX: 0000000000000000
RDX: 1ffffffff181f40e RSI: ffffffff83e28776 RDI: ffffffff8c0fa070
RBP: ffffc90001de7900 R08: ffff8880919dc340 R09: ffffed10431ee1c6
R10: ffffed10431ee1c5 R11: ffff888218f70e2b R12: ffffffff8c0fa030
R13: 0000000000000001 R14: ffffc90001de7a40 R15: ffffffff8c0fa188
FS:  0000000001060880(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000002 CR3: 000000009e6b8000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400


---
This bug 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 bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

Re: BUG: unable to handle kernel NULL pointer dereference in mem_serial_out

[Greg KH]

On Fri, Dec 06, 2019 at 10:25:08PM -0800, syzbot wrote:
> Hello,
> 
> syzbot found the following crash on:
> 
> HEAD commit:    7ada90eb Merge tag 'drm-next-2019-12-06' of git://anongit...
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=123ec282e00000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=f07a23020fd7d21a
> dashboard link: https://syzkaller.appspot.com/bug?extid=f4f1e871965064ae689e
> compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=17ab090ee00000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=17f127f2e00000
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+f4f1e871965064ae689e@syzkaller.appspotmail.com
> 
> BUG: kernel NULL pointer dereference, address: 0000000000000002
> #PF: supervisor write access in kernel mode
> #PF: error_code(0x0002) - not-present page
> PGD 9764a067 P4D 9764a067 PUD 9f995067 PMD 0
> Oops: 0002 [#1] PREEMPT SMP KASAN
> CPU: 0 PID: 9687 Comm: syz-executor433 Not tainted 5.4.0-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> RIP: 0010:writeb arch/x86/include/asm/io.h:65 [inline]
> RIP: 0010:mem_serial_out+0x70/0x90 drivers/tty/serial/8250/8250_port.c:408
> Code: e9 00 00 00 49 8d 7c 24 40 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48
> c1 ea 03 d3 e3 80 3c 02 00 75 19 48 63 db 49 03 5c 24 40 <44> 88 2b 5b 41 5c
> 41 5d 5d c3 e8 81 ed cf fd eb c0 e8 da ed cf fd
> RSP: 0018:ffffc90001de78e8 EFLAGS: 00010202
> RAX: dffffc0000000000 RBX: 0000000000000002 RCX: 0000000000000000
> RDX: 1ffffffff181f40e RSI: ffffffff83e28776 RDI: ffffffff8c0fa070
> RBP: ffffc90001de7900 R08: ffff8880919dc340 R09: ffffed10431ee1c6
> R10: ffffed10431ee1c5 R11: ffff888218f70e2b R12: ffffffff8c0fa030
> R13: 0000000000000001 R14: ffffc90001de7a40 R15: ffffffff8c0fa188
> FS:  0000000001060880(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000002 CR3: 000000009e6b8000 CR4: 00000000001406f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
>  serial_out drivers/tty/serial/8250/8250.h:118 [inline]
>  serial8250_clear_fifos.part.0+0x3a/0xb0
> drivers/tty/serial/8250/8250_port.c:557
>  serial8250_clear_fifos drivers/tty/serial/8250/8250_port.c:556 [inline]
>  serial8250_do_startup+0x426/0x1cf0 drivers/tty/serial/8250/8250_port.c:2121
>  serial8250_startup+0x62/0x80 drivers/tty/serial/8250/8250_port.c:2329
>  uart_port_startup drivers/tty/serial/serial_core.c:219 [inline]
>  uart_startup drivers/tty/serial/serial_core.c:258 [inline]
>  uart_startup+0x452/0x980 drivers/tty/serial/serial_core.c:249
>  uart_set_info drivers/tty/serial/serial_core.c:998 [inline]
>  uart_set_info_user+0x13b4/0x1cf0 drivers/tty/serial/serial_core.c:1023
>  tty_tiocsserial drivers/tty/tty_io.c:2506 [inline]
>  tty_ioctl+0xf60/0x14f0 drivers/tty/tty_io.c:2648
>  vfs_ioctl fs/ioctl.c:47 [inline]
>  file_ioctl fs/ioctl.c:545 [inline]
>  do_vfs_ioctl+0x977/0x14e0 fs/ioctl.c:732
>  ksys_ioctl+0xab/0xd0 fs/ioctl.c:749
>  __do_sys_ioctl fs/ioctl.c:756 [inline]
>  __se_sys_ioctl fs/ioctl.c:754 [inline]
>  __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:754
>  do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x440219
> Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 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
> 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007ffced648c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000440219
> RDX: 0000000020000240 RSI: 000000000000541f RDI: 0000000000000003
> RBP: 00000000006ca018 R08: 0000000000000000 R09: 00000000004002c8
> R10: 0000000000401b30 R11: 0000000000000246 R12: 0000000000401aa0
> R13: 0000000000401b30 R14: 0000000000000000 R15: 0000000000000000
> Modules linked in:
> CR2: 0000000000000002
> ---[ end trace eaa11ffe82f3a763 ]---
> RIP: 0010:writeb arch/x86/include/asm/io.h:65 [inline]
> RIP: 0010:mem_serial_out+0x70/0x90 drivers/tty/serial/8250/8250_port.c:408
> Code: e9 00 00 00 49 8d 7c 24 40 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48
> c1 ea 03 d3 e3 80 3c 02 00 75 19 48 63 db 49 03 5c 24 40 <44> 88 2b 5b 41 5c
> 41 5d 5d c3 e8 81 ed cf fd eb c0 e8 da ed cf fd
> RSP: 0018:ffffc90001de78e8 EFLAGS: 00010202
> RAX: dffffc0000000000 RBX: 0000000000000002 RCX: 0000000000000000
> RDX: 1ffffffff181f40e RSI: ffffffff83e28776 RDI: ffffffff8c0fa070
> RBP: ffffc90001de7900 R08: ffff8880919dc340 R09: ffffed10431ee1c6
> R10: ffffed10431ee1c5 R11: ffff888218f70e2b R12: ffffffff8c0fa030
> R13: 0000000000000001 R14: ffffc90001de7a40 R15: ffffffff8c0fa188
> FS:  0000000001060880(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000002 CR3: 000000009e6b8000 CR4: 00000000001406f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> 

You set up a dubious memory base for your uart and then get upset when
you write to that location.

I don't know what to really do about this, this is a root-only operation
and you are expected to know what you are doing when you attempt this.

thanks,

greg k-h

Re: BUG: unable to handle kernel NULL pointer dereference in mem_serial_out

[Dmitry Vyukov]

On Thu, Dec 12, 2019 at 11:57 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Fri, Dec 06, 2019 at 10:25:08PM -0800, syzbot wrote:
> > Hello,
> >
> > syzbot found the following crash on:
> >
> > HEAD commit:    7ada90eb Merge tag 'drm-next-2019-12-06' of git://anongit...
> > git tree:       upstream
> > console output: https://syzkaller.appspot.com/x/log.txt?x=123ec282e00000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=f07a23020fd7d21a
> > dashboard link: https://syzkaller.appspot.com/bug?extid=f4f1e871965064ae689e
> > compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=17ab090ee00000
> > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=17f127f2e00000
> >
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: syzbot+f4f1e871965064ae689e@syzkaller.appspotmail.com
> >
> > BUG: kernel NULL pointer dereference, address: 0000000000000002
> > #PF: supervisor write access in kernel mode
> > #PF: error_code(0x0002) - not-present page
> > PGD 9764a067 P4D 9764a067 PUD 9f995067 PMD 0
> > Oops: 0002 [#1] PREEMPT SMP KASAN
> > CPU: 0 PID: 9687 Comm: syz-executor433 Not tainted 5.4.0-syzkaller #0
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > Google 01/01/2011
> > RIP: 0010:writeb arch/x86/include/asm/io.h:65 [inline]
> > RIP: 0010:mem_serial_out+0x70/0x90 drivers/tty/serial/8250/8250_port.c:408
> > Code: e9 00 00 00 49 8d 7c 24 40 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48
> > c1 ea 03 d3 e3 80 3c 02 00 75 19 48 63 db 49 03 5c 24 40 <44> 88 2b 5b 41 5c
> > 41 5d 5d c3 e8 81 ed cf fd eb c0 e8 da ed cf fd
> > RSP: 0018:ffffc90001de78e8 EFLAGS: 00010202
> > RAX: dffffc0000000000 RBX: 0000000000000002 RCX: 0000000000000000
> > RDX: 1ffffffff181f40e RSI: ffffffff83e28776 RDI: ffffffff8c0fa070
> > RBP: ffffc90001de7900 R08: ffff8880919dc340 R09: ffffed10431ee1c6
> > R10: ffffed10431ee1c5 R11: ffff888218f70e2b R12: ffffffff8c0fa030
> > R13: 0000000000000001 R14: ffffc90001de7a40 R15: ffffffff8c0fa188
> > FS:  0000000001060880(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
> > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 0000000000000002 CR3: 000000009e6b8000 CR4: 00000000001406f0
> > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > Call Trace:
> >  serial_out drivers/tty/serial/8250/8250.h:118 [inline]
> >  serial8250_clear_fifos.part.0+0x3a/0xb0
> > drivers/tty/serial/8250/8250_port.c:557
> >  serial8250_clear_fifos drivers/tty/serial/8250/8250_port.c:556 [inline]
> >  serial8250_do_startup+0x426/0x1cf0 drivers/tty/serial/8250/8250_port.c:2121
> >  serial8250_startup+0x62/0x80 drivers/tty/serial/8250/8250_port.c:2329
> >  uart_port_startup drivers/tty/serial/serial_core.c:219 [inline]
> >  uart_startup drivers/tty/serial/serial_core.c:258 [inline]
> >  uart_startup+0x452/0x980 drivers/tty/serial/serial_core.c:249
> >  uart_set_info drivers/tty/serial/serial_core.c:998 [inline]
> >  uart_set_info_user+0x13b4/0x1cf0 drivers/tty/serial/serial_core.c:1023
> >  tty_tiocsserial drivers/tty/tty_io.c:2506 [inline]
> >  tty_ioctl+0xf60/0x14f0 drivers/tty/tty_io.c:2648
> >  vfs_ioctl fs/ioctl.c:47 [inline]
> >  file_ioctl fs/ioctl.c:545 [inline]
> >  do_vfs_ioctl+0x977/0x14e0 fs/ioctl.c:732
> >  ksys_ioctl+0xab/0xd0 fs/ioctl.c:749
> >  __do_sys_ioctl fs/ioctl.c:756 [inline]
> >  __se_sys_ioctl fs/ioctl.c:754 [inline]
> >  __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:754
> >  do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294
> >  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > RIP: 0033:0x440219
> > Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 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
> > 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
> > RSP: 002b:00007ffced648c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> > RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000440219
> > RDX: 0000000020000240 RSI: 000000000000541f RDI: 0000000000000003
> > RBP: 00000000006ca018 R08: 0000000000000000 R09: 00000000004002c8
> > R10: 0000000000401b30 R11: 0000000000000246 R12: 0000000000401aa0
> > R13: 0000000000401b30 R14: 0000000000000000 R15: 0000000000000000
> > Modules linked in:
> > CR2: 0000000000000002
> > ---[ end trace eaa11ffe82f3a763 ]---
> > RIP: 0010:writeb arch/x86/include/asm/io.h:65 [inline]
> > RIP: 0010:mem_serial_out+0x70/0x90 drivers/tty/serial/8250/8250_port.c:408
> > Code: e9 00 00 00 49 8d 7c 24 40 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48
> > c1 ea 03 d3 e3 80 3c 02 00 75 19 48 63 db 49 03 5c 24 40 <44> 88 2b 5b 41 5c
> > 41 5d 5d c3 e8 81 ed cf fd eb c0 e8 da ed cf fd
> > RSP: 0018:ffffc90001de78e8 EFLAGS: 00010202
> > RAX: dffffc0000000000 RBX: 0000000000000002 RCX: 0000000000000000
> > RDX: 1ffffffff181f40e RSI: ffffffff83e28776 RDI: ffffffff8c0fa070
> > RBP: ffffc90001de7900 R08: ffff8880919dc340 R09: ffffed10431ee1c6
> > R10: ffffed10431ee1c5 R11: ffff888218f70e2b R12: ffffffff8c0fa030
> > R13: 0000000000000001 R14: ffffc90001de7a40 R15: ffffffff8c0fa188
> > FS:  0000000001060880(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
> > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 0000000000000002 CR3: 000000009e6b8000 CR4: 00000000001406f0
> > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> >
>
> You set up a dubious memory base for your uart and then get upset when
> you write to that location.
>
> I don't know what to really do about this, this is a root-only operation
> and you are expected to know what you are doing when you attempt this.

Hi Greg,

Thanks for looking into this!
Should we restrict the fuzzer from accessing /dev/ttyS* entirely? Or
only restrict TIOCSSERIAL on them? Something else?

Re: BUG: unable to handle kernel NULL pointer dereference in mem_serial_out

[Greg KH]

On Fri, Dec 13, 2019 at 10:02:33AM +0100, Dmitry Vyukov wrote:
> On Thu, Dec 12, 2019 at 11:57 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, Dec 06, 2019 at 10:25:08PM -0800, syzbot wrote:
> > > Hello,
> > >
> > > syzbot found the following crash on:
> > >
> > > HEAD commit:    7ada90eb Merge tag 'drm-next-2019-12-06' of git://anongit...
> > > git tree:       upstream
> > > console output: https://syzkaller.appspot.com/x/log.txt?x=123ec282e00000
> > > kernel config:  https://syzkaller.appspot.com/x/.config?x=f07a23020fd7d21a
> > > dashboard link: https://syzkaller.appspot.com/bug?extid=f4f1e871965064ae689e
> > > compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> > > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=17ab090ee00000
> > > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=17f127f2e00000
> > >
> > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > Reported-by: syzbot+f4f1e871965064ae689e@syzkaller.appspotmail.com
> > >
> > > BUG: kernel NULL pointer dereference, address: 0000000000000002
> > > #PF: supervisor write access in kernel mode
> > > #PF: error_code(0x0002) - not-present page
> > > PGD 9764a067 P4D 9764a067 PUD 9f995067 PMD 0
> > > Oops: 0002 [#1] PREEMPT SMP KASAN
> > > CPU: 0 PID: 9687 Comm: syz-executor433 Not tainted 5.4.0-syzkaller #0
> > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > > Google 01/01/2011
> > > RIP: 0010:writeb arch/x86/include/asm/io.h:65 [inline]
> > > RIP: 0010:mem_serial_out+0x70/0x90 drivers/tty/serial/8250/8250_port.c:408
> > > Code: e9 00 00 00 49 8d 7c 24 40 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48
> > > c1 ea 03 d3 e3 80 3c 02 00 75 19 48 63 db 49 03 5c 24 40 <44> 88 2b 5b 41 5c
> > > 41 5d 5d c3 e8 81 ed cf fd eb c0 e8 da ed cf fd
> > > RSP: 0018:ffffc90001de78e8 EFLAGS: 00010202
> > > RAX: dffffc0000000000 RBX: 0000000000000002 RCX: 0000000000000000
> > > RDX: 1ffffffff181f40e RSI: ffffffff83e28776 RDI: ffffffff8c0fa070
> > > RBP: ffffc90001de7900 R08: ffff8880919dc340 R09: ffffed10431ee1c6
> > > R10: ffffed10431ee1c5 R11: ffff888218f70e2b R12: ffffffff8c0fa030
> > > R13: 0000000000000001 R14: ffffc90001de7a40 R15: ffffffff8c0fa188
> > > FS:  0000000001060880(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
> > > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > CR2: 0000000000000002 CR3: 000000009e6b8000 CR4: 00000000001406f0
> > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > Call Trace:
> > >  serial_out drivers/tty/serial/8250/8250.h:118 [inline]
> > >  serial8250_clear_fifos.part.0+0x3a/0xb0
> > > drivers/tty/serial/8250/8250_port.c:557
> > >  serial8250_clear_fifos drivers/tty/serial/8250/8250_port.c:556 [inline]
> > >  serial8250_do_startup+0x426/0x1cf0 drivers/tty/serial/8250/8250_port.c:2121
> > >  serial8250_startup+0x62/0x80 drivers/tty/serial/8250/8250_port.c:2329
> > >  uart_port_startup drivers/tty/serial/serial_core.c:219 [inline]
> > >  uart_startup drivers/tty/serial/serial_core.c:258 [inline]
> > >  uart_startup+0x452/0x980 drivers/tty/serial/serial_core.c:249
> > >  uart_set_info drivers/tty/serial/serial_core.c:998 [inline]
> > >  uart_set_info_user+0x13b4/0x1cf0 drivers/tty/serial/serial_core.c:1023
> > >  tty_tiocsserial drivers/tty/tty_io.c:2506 [inline]
> > >  tty_ioctl+0xf60/0x14f0 drivers/tty/tty_io.c:2648
> > >  vfs_ioctl fs/ioctl.c:47 [inline]
> > >  file_ioctl fs/ioctl.c:545 [inline]
> > >  do_vfs_ioctl+0x977/0x14e0 fs/ioctl.c:732
> > >  ksys_ioctl+0xab/0xd0 fs/ioctl.c:749
> > >  __do_sys_ioctl fs/ioctl.c:756 [inline]
> > >  __se_sys_ioctl fs/ioctl.c:754 [inline]
> > >  __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:754
> > >  do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294
> > >  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > > RIP: 0033:0x440219
> > > Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 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
> > > 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
> > > RSP: 002b:00007ffced648c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> > > RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000440219
> > > RDX: 0000000020000240 RSI: 000000000000541f RDI: 0000000000000003
> > > RBP: 00000000006ca018 R08: 0000000000000000 R09: 00000000004002c8
> > > R10: 0000000000401b30 R11: 0000000000000246 R12: 0000000000401aa0
> > > R13: 0000000000401b30 R14: 0000000000000000 R15: 0000000000000000
> > > Modules linked in:
> > > CR2: 0000000000000002
> > > ---[ end trace eaa11ffe82f3a763 ]---
> > > RIP: 0010:writeb arch/x86/include/asm/io.h:65 [inline]
> > > RIP: 0010:mem_serial_out+0x70/0x90 drivers/tty/serial/8250/8250_port.c:408
> > > Code: e9 00 00 00 49 8d 7c 24 40 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48
> > > c1 ea 03 d3 e3 80 3c 02 00 75 19 48 63 db 49 03 5c 24 40 <44> 88 2b 5b 41 5c
> > > 41 5d 5d c3 e8 81 ed cf fd eb c0 e8 da ed cf fd
> > > RSP: 0018:ffffc90001de78e8 EFLAGS: 00010202
> > > RAX: dffffc0000000000 RBX: 0000000000000002 RCX: 0000000000000000
> > > RDX: 1ffffffff181f40e RSI: ffffffff83e28776 RDI: ffffffff8c0fa070
> > > RBP: ffffc90001de7900 R08: ffff8880919dc340 R09: ffffed10431ee1c6
> > > R10: ffffed10431ee1c5 R11: ffff888218f70e2b R12: ffffffff8c0fa030
> > > R13: 0000000000000001 R14: ffffc90001de7a40 R15: ffffffff8c0fa188
> > > FS:  0000000001060880(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
> > > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > CR2: 0000000000000002 CR3: 000000009e6b8000 CR4: 00000000001406f0
> > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > >
> >
> > You set up a dubious memory base for your uart and then get upset when
> > you write to that location.
> >
> > I don't know what to really do about this, this is a root-only operation
> > and you are expected to know what you are doing when you attempt this.
> 
> Hi Greg,
> 
> Thanks for looking into this!
> Should we restrict the fuzzer from accessing /dev/ttyS* entirely?

No, not at all.

> Or only restrict TIOCSSERIAL on them? Something else?

Try running not as root.  if you have CAP_SYS_ADMIN you can do a lot of
pretty bad things with tty ports, as you see here.  There's a reason the
LOCKDOWN_TIOCSSERIAL "security lockdown" check was added :)

The TIOCSSERIAL ioctl is a nice one for a lot of things that are able to
be done as a normal user (baud rate changes, etc.), but there are also
things like setting io port memory locations that can cause random
hardware accesses and kernel crashes, as you instantly found out here :)

So restrict the fuzzer to only run as a "normal" user of the serial
port, and if you find problems there, I'll be glad to look at them.

thanks,

greg k-h

Re: BUG: unable to handle kernel NULL pointer dereference in mem_serial_out

[Dmitry Vyukov]

On Fri, Dec 13, 2019 at 10:34 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Fri, Dec 13, 2019 at 10:02:33AM +0100, Dmitry Vyukov wrote:
> > On Thu, Dec 12, 2019 at 11:57 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Fri, Dec 06, 2019 at 10:25:08PM -0800, syzbot wrote:
> > > > Hello,
> > > >
> > > > syzbot found the following crash on:
> > > >
> > > > HEAD commit:    7ada90eb Merge tag 'drm-next-2019-12-06' of git://anongit...
> > > > git tree:       upstream
> > > > console output: https://syzkaller.appspot.com/x/log.txt?x=123ec282e00000
> > > > kernel config:  https://syzkaller.appspot.com/x/.config?x=f07a23020fd7d21a
> > > > dashboard link: https://syzkaller.appspot.com/bug?extid=f4f1e871965064ae689e
> > > > compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> > > > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=17ab090ee00000
> > > > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=17f127f2e00000
> > > >
> > > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > > Reported-by: syzbot+f4f1e871965064ae689e@syzkaller.appspotmail.com
> > > >
> > > > BUG: kernel NULL pointer dereference, address: 0000000000000002
> > > > #PF: supervisor write access in kernel mode
> > > > #PF: error_code(0x0002) - not-present page
> > > > PGD 9764a067 P4D 9764a067 PUD 9f995067 PMD 0
> > > > Oops: 0002 [#1] PREEMPT SMP KASAN
> > > > CPU: 0 PID: 9687 Comm: syz-executor433 Not tainted 5.4.0-syzkaller #0
> > > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > > > Google 01/01/2011
> > > > RIP: 0010:writeb arch/x86/include/asm/io.h:65 [inline]
> > > > RIP: 0010:mem_serial_out+0x70/0x90 drivers/tty/serial/8250/8250_port.c:408
> > > > Code: e9 00 00 00 49 8d 7c 24 40 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48
> > > > c1 ea 03 d3 e3 80 3c 02 00 75 19 48 63 db 49 03 5c 24 40 <44> 88 2b 5b 41 5c
> > > > 41 5d 5d c3 e8 81 ed cf fd eb c0 e8 da ed cf fd
> > > > RSP: 0018:ffffc90001de78e8 EFLAGS: 00010202
> > > > RAX: dffffc0000000000 RBX: 0000000000000002 RCX: 0000000000000000
> > > > RDX: 1ffffffff181f40e RSI: ffffffff83e28776 RDI: ffffffff8c0fa070
> > > > RBP: ffffc90001de7900 R08: ffff8880919dc340 R09: ffffed10431ee1c6
> > > > R10: ffffed10431ee1c5 R11: ffff888218f70e2b R12: ffffffff8c0fa030
> > > > R13: 0000000000000001 R14: ffffc90001de7a40 R15: ffffffff8c0fa188
> > > > FS:  0000000001060880(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
> > > > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > CR2: 0000000000000002 CR3: 000000009e6b8000 CR4: 00000000001406f0
> > > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > > Call Trace:
> > > >  serial_out drivers/tty/serial/8250/8250.h:118 [inline]
> > > >  serial8250_clear_fifos.part.0+0x3a/0xb0
> > > > drivers/tty/serial/8250/8250_port.c:557
> > > >  serial8250_clear_fifos drivers/tty/serial/8250/8250_port.c:556 [inline]
> > > >  serial8250_do_startup+0x426/0x1cf0 drivers/tty/serial/8250/8250_port.c:2121
> > > >  serial8250_startup+0x62/0x80 drivers/tty/serial/8250/8250_port.c:2329
> > > >  uart_port_startup drivers/tty/serial/serial_core.c:219 [inline]
> > > >  uart_startup drivers/tty/serial/serial_core.c:258 [inline]
> > > >  uart_startup+0x452/0x980 drivers/tty/serial/serial_core.c:249
> > > >  uart_set_info drivers/tty/serial/serial_core.c:998 [inline]
> > > >  uart_set_info_user+0x13b4/0x1cf0 drivers/tty/serial/serial_core.c:1023
> > > >  tty_tiocsserial drivers/tty/tty_io.c:2506 [inline]
> > > >  tty_ioctl+0xf60/0x14f0 drivers/tty/tty_io.c:2648
> > > >  vfs_ioctl fs/ioctl.c:47 [inline]
> > > >  file_ioctl fs/ioctl.c:545 [inline]
> > > >  do_vfs_ioctl+0x977/0x14e0 fs/ioctl.c:732
> > > >  ksys_ioctl+0xab/0xd0 fs/ioctl.c:749
> > > >  __do_sys_ioctl fs/ioctl.c:756 [inline]
> > > >  __se_sys_ioctl fs/ioctl.c:754 [inline]
> > > >  __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:754
> > > >  do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294
> > > >  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > > > RIP: 0033:0x440219
> > > > Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 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
> > > > 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
> > > > RSP: 002b:00007ffced648c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> > > > RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000440219
> > > > RDX: 0000000020000240 RSI: 000000000000541f RDI: 0000000000000003
> > > > RBP: 00000000006ca018 R08: 0000000000000000 R09: 00000000004002c8
> > > > R10: 0000000000401b30 R11: 0000000000000246 R12: 0000000000401aa0
> > > > R13: 0000000000401b30 R14: 0000000000000000 R15: 0000000000000000
> > > > Modules linked in:
> > > > CR2: 0000000000000002
> > > > ---[ end trace eaa11ffe82f3a763 ]---
> > > > RIP: 0010:writeb arch/x86/include/asm/io.h:65 [inline]
> > > > RIP: 0010:mem_serial_out+0x70/0x90 drivers/tty/serial/8250/8250_port.c:408
> > > > Code: e9 00 00 00 49 8d 7c 24 40 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48
> > > > c1 ea 03 d3 e3 80 3c 02 00 75 19 48 63 db 49 03 5c 24 40 <44> 88 2b 5b 41 5c
> > > > 41 5d 5d c3 e8 81 ed cf fd eb c0 e8 da ed cf fd
> > > > RSP: 0018:ffffc90001de78e8 EFLAGS: 00010202
> > > > RAX: dffffc0000000000 RBX: 0000000000000002 RCX: 0000000000000000
> > > > RDX: 1ffffffff181f40e RSI: ffffffff83e28776 RDI: ffffffff8c0fa070
> > > > RBP: ffffc90001de7900 R08: ffff8880919dc340 R09: ffffed10431ee1c6
> > > > R10: ffffed10431ee1c5 R11: ffff888218f70e2b R12: ffffffff8c0fa030
> > > > R13: 0000000000000001 R14: ffffc90001de7a40 R15: ffffffff8c0fa188
> > > > FS:  0000000001060880(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
> > > > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > CR2: 0000000000000002 CR3: 000000009e6b8000 CR4: 00000000001406f0
> > > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > >
> > >
> > > You set up a dubious memory base for your uart and then get upset when
> > > you write to that location.
> > >
> > > I don't know what to really do about this, this is a root-only operation
> > > and you are expected to know what you are doing when you attempt this.
> >
> > Hi Greg,
> >
> > Thanks for looking into this!
> > Should we restrict the fuzzer from accessing /dev/ttyS* entirely?
>
> No, not at all.
>
> > Or only restrict TIOCSSERIAL on them? Something else?
>
> Try running not as root.  if you have CAP_SYS_ADMIN you can do a lot of
> pretty bad things with tty ports, as you see here.  There's a reason the
> LOCKDOWN_TIOCSSERIAL "security lockdown" check was added :)
>
> The TIOCSSERIAL ioctl is a nice one for a lot of things that are able to
> be done as a normal user (baud rate changes, etc.), but there are also
> things like setting io port memory locations that can cause random
> hardware accesses and kernel crashes, as you instantly found out here :)
>
> So restrict the fuzzer to only run as a "normal" user of the serial
> port, and if you find problems there, I'll be glad to look at them.

Easier said than done. "normal user of the serial port" is not really
a thing in Linux, right? You either have CAP_SYS_ADMIN or not, that's
not per-device...
As far as I remember +Tetsuo proposed a config along the lines of
"restrict only things that legitimately cause damage under a fuzzer
workload", e.g. freezing filesystems, disabling console output, etc.
This may be another candidate. But I can't find where that proposal is
now.

A simpler option that I see is as follows. syzkaller has several
sandboxing modes, one of them is "namespace" which uses a user ns, in
that more fuzzer is still uid=0 in the init namespace, so has access
to all /dev nodes, but it does not have CAP_SYS_ADMIN in the init
namespace. We could enable /dev/ttyS* only on instance that use
sandbox=namesace, and disable on the rest. Does it make sense?

Re: BUG: unable to handle kernel NULL pointer dereference in mem_serial_out

[Greg KH]

On Fri, Dec 13, 2019 at 11:00:35AM +0100, Dmitry Vyukov wrote:
> On Fri, Dec 13, 2019 at 10:34 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, Dec 13, 2019 at 10:02:33AM +0100, Dmitry Vyukov wrote:
> > > On Thu, Dec 12, 2019 at 11:57 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Fri, Dec 06, 2019 at 10:25:08PM -0800, syzbot wrote:
> > > > > Hello,
> > > > >
> > > > > syzbot found the following crash on:
> > > > >
> > > > > HEAD commit:    7ada90eb Merge tag 'drm-next-2019-12-06' of git://anongit...
> > > > > git tree:       upstream
> > > > > console output: https://syzkaller.appspot.com/x/log.txt?x=123ec282e00000
> > > > > kernel config:  https://syzkaller.appspot.com/x/.config?x=f07a23020fd7d21a
> > > > > dashboard link: https://syzkaller.appspot.com/bug?extid=f4f1e871965064ae689e
> > > > > compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> > > > > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=17ab090ee00000
> > > > > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=17f127f2e00000
> > > > >
> > > > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > > > Reported-by: syzbot+f4f1e871965064ae689e@syzkaller.appspotmail.com
> > > > >
> > > > > BUG: kernel NULL pointer dereference, address: 0000000000000002
> > > > > #PF: supervisor write access in kernel mode
> > > > > #PF: error_code(0x0002) - not-present page
> > > > > PGD 9764a067 P4D 9764a067 PUD 9f995067 PMD 0
> > > > > Oops: 0002 [#1] PREEMPT SMP KASAN
> > > > > CPU: 0 PID: 9687 Comm: syz-executor433 Not tainted 5.4.0-syzkaller #0
> > > > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > > > > Google 01/01/2011
> > > > > RIP: 0010:writeb arch/x86/include/asm/io.h:65 [inline]
> > > > > RIP: 0010:mem_serial_out+0x70/0x90 drivers/tty/serial/8250/8250_port.c:408
> > > > > Code: e9 00 00 00 49 8d 7c 24 40 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48
> > > > > c1 ea 03 d3 e3 80 3c 02 00 75 19 48 63 db 49 03 5c 24 40 <44> 88 2b 5b 41 5c
> > > > > 41 5d 5d c3 e8 81 ed cf fd eb c0 e8 da ed cf fd
> > > > > RSP: 0018:ffffc90001de78e8 EFLAGS: 00010202
> > > > > RAX: dffffc0000000000 RBX: 0000000000000002 RCX: 0000000000000000
> > > > > RDX: 1ffffffff181f40e RSI: ffffffff83e28776 RDI: ffffffff8c0fa070
> > > > > RBP: ffffc90001de7900 R08: ffff8880919dc340 R09: ffffed10431ee1c6
> > > > > R10: ffffed10431ee1c5 R11: ffff888218f70e2b R12: ffffffff8c0fa030
> > > > > R13: 0000000000000001 R14: ffffc90001de7a40 R15: ffffffff8c0fa188
> > > > > FS:  0000000001060880(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
> > > > > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > > CR2: 0000000000000002 CR3: 000000009e6b8000 CR4: 00000000001406f0
> > > > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > > > Call Trace:
> > > > >  serial_out drivers/tty/serial/8250/8250.h:118 [inline]
> > > > >  serial8250_clear_fifos.part.0+0x3a/0xb0
> > > > > drivers/tty/serial/8250/8250_port.c:557
> > > > >  serial8250_clear_fifos drivers/tty/serial/8250/8250_port.c:556 [inline]
> > > > >  serial8250_do_startup+0x426/0x1cf0 drivers/tty/serial/8250/8250_port.c:2121
> > > > >  serial8250_startup+0x62/0x80 drivers/tty/serial/8250/8250_port.c:2329
> > > > >  uart_port_startup drivers/tty/serial/serial_core.c:219 [inline]
> > > > >  uart_startup drivers/tty/serial/serial_core.c:258 [inline]
> > > > >  uart_startup+0x452/0x980 drivers/tty/serial/serial_core.c:249
> > > > >  uart_set_info drivers/tty/serial/serial_core.c:998 [inline]
> > > > >  uart_set_info_user+0x13b4/0x1cf0 drivers/tty/serial/serial_core.c:1023
> > > > >  tty_tiocsserial drivers/tty/tty_io.c:2506 [inline]
> > > > >  tty_ioctl+0xf60/0x14f0 drivers/tty/tty_io.c:2648
> > > > >  vfs_ioctl fs/ioctl.c:47 [inline]
> > > > >  file_ioctl fs/ioctl.c:545 [inline]
> > > > >  do_vfs_ioctl+0x977/0x14e0 fs/ioctl.c:732
> > > > >  ksys_ioctl+0xab/0xd0 fs/ioctl.c:749
> > > > >  __do_sys_ioctl fs/ioctl.c:756 [inline]
> > > > >  __se_sys_ioctl fs/ioctl.c:754 [inline]
> > > > >  __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:754
> > > > >  do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294
> > > > >  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > > > > RIP: 0033:0x440219
> > > > > Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 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
> > > > > 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
> > > > > RSP: 002b:00007ffced648c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> > > > > RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000440219
> > > > > RDX: 0000000020000240 RSI: 000000000000541f RDI: 0000000000000003
> > > > > RBP: 00000000006ca018 R08: 0000000000000000 R09: 00000000004002c8
> > > > > R10: 0000000000401b30 R11: 0000000000000246 R12: 0000000000401aa0
> > > > > R13: 0000000000401b30 R14: 0000000000000000 R15: 0000000000000000
> > > > > Modules linked in:
> > > > > CR2: 0000000000000002
> > > > > ---[ end trace eaa11ffe82f3a763 ]---
> > > > > RIP: 0010:writeb arch/x86/include/asm/io.h:65 [inline]
> > > > > RIP: 0010:mem_serial_out+0x70/0x90 drivers/tty/serial/8250/8250_port.c:408
> > > > > Code: e9 00 00 00 49 8d 7c 24 40 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48
> > > > > c1 ea 03 d3 e3 80 3c 02 00 75 19 48 63 db 49 03 5c 24 40 <44> 88 2b 5b 41 5c
> > > > > 41 5d 5d c3 e8 81 ed cf fd eb c0 e8 da ed cf fd
> > > > > RSP: 0018:ffffc90001de78e8 EFLAGS: 00010202
> > > > > RAX: dffffc0000000000 RBX: 0000000000000002 RCX: 0000000000000000
> > > > > RDX: 1ffffffff181f40e RSI: ffffffff83e28776 RDI: ffffffff8c0fa070
> > > > > RBP: ffffc90001de7900 R08: ffff8880919dc340 R09: ffffed10431ee1c6
> > > > > R10: ffffed10431ee1c5 R11: ffff888218f70e2b R12: ffffffff8c0fa030
> > > > > R13: 0000000000000001 R14: ffffc90001de7a40 R15: ffffffff8c0fa188
> > > > > FS:  0000000001060880(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
> > > > > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > > CR2: 0000000000000002 CR3: 000000009e6b8000 CR4: 00000000001406f0
> > > > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > > >
> > > >
> > > > You set up a dubious memory base for your uart and then get upset when
> > > > you write to that location.
> > > >
> > > > I don't know what to really do about this, this is a root-only operation
> > > > and you are expected to know what you are doing when you attempt this.
> > >
> > > Hi Greg,
> > >
> > > Thanks for looking into this!
> > > Should we restrict the fuzzer from accessing /dev/ttyS* entirely?
> >
> > No, not at all.
> >
> > > Or only restrict TIOCSSERIAL on them? Something else?
> >
> > Try running not as root.  if you have CAP_SYS_ADMIN you can do a lot of
> > pretty bad things with tty ports, as you see here.  There's a reason the
> > LOCKDOWN_TIOCSSERIAL "security lockdown" check was added :)
> >
> > The TIOCSSERIAL ioctl is a nice one for a lot of things that are able to
> > be done as a normal user (baud rate changes, etc.), but there are also
> > things like setting io port memory locations that can cause random
> > hardware accesses and kernel crashes, as you instantly found out here :)
> >
> > So restrict the fuzzer to only run as a "normal" user of the serial
> > port, and if you find problems there, I'll be glad to look at them.
> 
> Easier said than done. "normal user of the serial port" is not really
> a thing in Linux, right? You either have CAP_SYS_ADMIN or not, that's
> not per-device...

Not true, there's lots of users of serial port devices that do not have
CAP_SYS_ADMIN set.  That's why we have groups :)

You can change the baud rate of your usb-serial device without root
permissions, right?  That's a "normal" user right there.

> As far as I remember +Tetsuo proposed a config along the lines of
> "restrict only things that legitimately cause damage under a fuzzer
> workload", e.g. freezing filesystems, disabling console output, etc.
> This may be another candidate. But I can't find where that proposal is
> now.
> 
> A simpler option that I see is as follows. syzkaller has several
> sandboxing modes, one of them is "namespace" which uses a user ns, in
> that more fuzzer is still uid=0 in the init namespace, so has access
> to all /dev nodes, but it does not have CAP_SYS_ADMIN in the init
> namespace. We could enable /dev/ttyS* only on instance that use
> sandbox=namesace, and disable on the rest. Does it make sense?

Maybe, I don't know.  Why do you have to run the fuzzer with uid=0 in
the first place?

thanks,

greg k-h

Re: BUG: unable to handle kernel NULL pointer dereference in mem_serial_out

[Dmitry Vyukov]

On Fri, Dec 13, 2019 at 11:10 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Fri, Dec 13, 2019 at 11:00:35AM +0100, Dmitry Vyukov wrote:
> > On Fri, Dec 13, 2019 at 10:34 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Fri, Dec 13, 2019 at 10:02:33AM +0100, Dmitry Vyukov wrote:
> > > > On Thu, Dec 12, 2019 at 11:57 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > >
> > > > > On Fri, Dec 06, 2019 at 10:25:08PM -0800, syzbot wrote:
> > > > > > Hello,
> > > > > >
> > > > > > syzbot found the following crash on:
> > > > > >
> > > > > > HEAD commit:    7ada90eb Merge tag 'drm-next-2019-12-06' of git://anongit...
> > > > > > git tree:       upstream
> > > > > > console output: https://syzkaller.appspot.com/x/log.txt?x=123ec282e00000
> > > > > > kernel config:  https://syzkaller.appspot.com/x/.config?x=f07a23020fd7d21a
> > > > > > dashboard link: https://syzkaller.appspot.com/bug?extid=f4f1e871965064ae689e
> > > > > > compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> > > > > > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=17ab090ee00000
> > > > > > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=17f127f2e00000
> > > > > >
> > > > > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > > > > Reported-by: syzbot+f4f1e871965064ae689e@syzkaller.appspotmail.com
> > > > > >
> > > > > > BUG: kernel NULL pointer dereference, address: 0000000000000002
> > > > > > #PF: supervisor write access in kernel mode
> > > > > > #PF: error_code(0x0002) - not-present page
> > > > > > PGD 9764a067 P4D 9764a067 PUD 9f995067 PMD 0
> > > > > > Oops: 0002 [#1] PREEMPT SMP KASAN
> > > > > > CPU: 0 PID: 9687 Comm: syz-executor433 Not tainted 5.4.0-syzkaller #0
> > > > > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > > > > > Google 01/01/2011
> > > > > > RIP: 0010:writeb arch/x86/include/asm/io.h:65 [inline]
> > > > > > RIP: 0010:mem_serial_out+0x70/0x90 drivers/tty/serial/8250/8250_port.c:408
> > > > > > Code: e9 00 00 00 49 8d 7c 24 40 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48
> > > > > > c1 ea 03 d3 e3 80 3c 02 00 75 19 48 63 db 49 03 5c 24 40 <44> 88 2b 5b 41 5c
> > > > > > 41 5d 5d c3 e8 81 ed cf fd eb c0 e8 da ed cf fd
> > > > > > RSP: 0018:ffffc90001de78e8 EFLAGS: 00010202
> > > > > > RAX: dffffc0000000000 RBX: 0000000000000002 RCX: 0000000000000000
> > > > > > RDX: 1ffffffff181f40e RSI: ffffffff83e28776 RDI: ffffffff8c0fa070
> > > > > > RBP: ffffc90001de7900 R08: ffff8880919dc340 R09: ffffed10431ee1c6
> > > > > > R10: ffffed10431ee1c5 R11: ffff888218f70e2b R12: ffffffff8c0fa030
> > > > > > R13: 0000000000000001 R14: ffffc90001de7a40 R15: ffffffff8c0fa188
> > > > > > FS:  0000000001060880(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
> > > > > > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > > > CR2: 0000000000000002 CR3: 000000009e6b8000 CR4: 00000000001406f0
> > > > > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > > > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > > > > Call Trace:
> > > > > >  serial_out drivers/tty/serial/8250/8250.h:118 [inline]
> > > > > >  serial8250_clear_fifos.part.0+0x3a/0xb0
> > > > > > drivers/tty/serial/8250/8250_port.c:557
> > > > > >  serial8250_clear_fifos drivers/tty/serial/8250/8250_port.c:556 [inline]
> > > > > >  serial8250_do_startup+0x426/0x1cf0 drivers/tty/serial/8250/8250_port.c:2121
> > > > > >  serial8250_startup+0x62/0x80 drivers/tty/serial/8250/8250_port.c:2329
> > > > > >  uart_port_startup drivers/tty/serial/serial_core.c:219 [inline]
> > > > > >  uart_startup drivers/tty/serial/serial_core.c:258 [inline]
> > > > > >  uart_startup+0x452/0x980 drivers/tty/serial/serial_core.c:249
> > > > > >  uart_set_info drivers/tty/serial/serial_core.c:998 [inline]
> > > > > >  uart_set_info_user+0x13b4/0x1cf0 drivers/tty/serial/serial_core.c:1023
> > > > > >  tty_tiocsserial drivers/tty/tty_io.c:2506 [inline]
> > > > > >  tty_ioctl+0xf60/0x14f0 drivers/tty/tty_io.c:2648
> > > > > >  vfs_ioctl fs/ioctl.c:47 [inline]
> > > > > >  file_ioctl fs/ioctl.c:545 [inline]
> > > > > >  do_vfs_ioctl+0x977/0x14e0 fs/ioctl.c:732
> > > > > >  ksys_ioctl+0xab/0xd0 fs/ioctl.c:749
> > > > > >  __do_sys_ioctl fs/ioctl.c:756 [inline]
> > > > > >  __se_sys_ioctl fs/ioctl.c:754 [inline]
> > > > > >  __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:754
> > > > > >  do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294
> > > > > >  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > > > > > RIP: 0033:0x440219
> > > > > > Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 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
> > > > > > 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
> > > > > > RSP: 002b:00007ffced648c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> > > > > > RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000440219
> > > > > > RDX: 0000000020000240 RSI: 000000000000541f RDI: 0000000000000003
> > > > > > RBP: 00000000006ca018 R08: 0000000000000000 R09: 00000000004002c8
> > > > > > R10: 0000000000401b30 R11: 0000000000000246 R12: 0000000000401aa0
> > > > > > R13: 0000000000401b30 R14: 0000000000000000 R15: 0000000000000000
> > > > > > Modules linked in:
> > > > > > CR2: 0000000000000002
> > > > > > ---[ end trace eaa11ffe82f3a763 ]---
> > > > > > RIP: 0010:writeb arch/x86/include/asm/io.h:65 [inline]
> > > > > > RIP: 0010:mem_serial_out+0x70/0x90 drivers/tty/serial/8250/8250_port.c:408
> > > > > > Code: e9 00 00 00 49 8d 7c 24 40 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48
> > > > > > c1 ea 03 d3 e3 80 3c 02 00 75 19 48 63 db 49 03 5c 24 40 <44> 88 2b 5b 41 5c
> > > > > > 41 5d 5d c3 e8 81 ed cf fd eb c0 e8 da ed cf fd
> > > > > > RSP: 0018:ffffc90001de78e8 EFLAGS: 00010202
> > > > > > RAX: dffffc0000000000 RBX: 0000000000000002 RCX: 0000000000000000
> > > > > > RDX: 1ffffffff181f40e RSI: ffffffff83e28776 RDI: ffffffff8c0fa070
> > > > > > RBP: ffffc90001de7900 R08: ffff8880919dc340 R09: ffffed10431ee1c6
> > > > > > R10: ffffed10431ee1c5 R11: ffff888218f70e2b R12: ffffffff8c0fa030
> > > > > > R13: 0000000000000001 R14: ffffc90001de7a40 R15: ffffffff8c0fa188
> > > > > > FS:  0000000001060880(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
> > > > > > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > > > CR2: 0000000000000002 CR3: 000000009e6b8000 CR4: 00000000001406f0
> > > > > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > > > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > > > >
> > > > >
> > > > > You set up a dubious memory base for your uart and then get upset when
> > > > > you write to that location.
> > > > >
> > > > > I don't know what to really do about this, this is a root-only operation
> > > > > and you are expected to know what you are doing when you attempt this.
> > > >
> > > > Hi Greg,
> > > >
> > > > Thanks for looking into this!
> > > > Should we restrict the fuzzer from accessing /dev/ttyS* entirely?
> > >
> > > No, not at all.
> > >
> > > > Or only restrict TIOCSSERIAL on them? Something else?
> > >
> > > Try running not as root.  if you have CAP_SYS_ADMIN you can do a lot of
> > > pretty bad things with tty ports, as you see here.  There's a reason the
> > > LOCKDOWN_TIOCSSERIAL "security lockdown" check was added :)
> > >
> > > The TIOCSSERIAL ioctl is a nice one for a lot of things that are able to
> > > be done as a normal user (baud rate changes, etc.), but there are also
> > > things like setting io port memory locations that can cause random
> > > hardware accesses and kernel crashes, as you instantly found out here :)
> > >
> > > So restrict the fuzzer to only run as a "normal" user of the serial
> > > port, and if you find problems there, I'll be glad to look at them.
> >
> > Easier said than done. "normal user of the serial port" is not really
> > a thing in Linux, right? You either have CAP_SYS_ADMIN or not, that's
> > not per-device...
>
> Not true, there's lots of users of serial port devices that do not have
> CAP_SYS_ADMIN set.  That's why we have groups :)
>
> You can change the baud rate of your usb-serial device without root
> permissions, right?  That's a "normal" user right there.

Yes, but this requires dropping CAP_SYS_ADMIN. And one can't drop
CAP_SYS_ADMIN only for ttyS. If it would be a separate capability, we
could drop just that, but not CAP_SYS_ADMIN.

> > As far as I remember +Tetsuo proposed a config along the lines of
> > "restrict only things that legitimately cause damage under a fuzzer
> > workload", e.g. freezing filesystems, disabling console output, etc.
> > This may be another candidate. But I can't find where that proposal is
> > now.
> >
> > A simpler option that I see is as follows. syzkaller has several
> > sandboxing modes, one of them is "namespace" which uses a user ns, in
> > that more fuzzer is still uid=0 in the init namespace, so has access
> > to all /dev nodes, but it does not have CAP_SYS_ADMIN in the init
> > namespace. We could enable /dev/ttyS* only on instance that use
> > sandbox=namesace, and disable on the rest. Does it make sense?
>
> Maybe, I don't know.  Why do you have to run the fuzzer with uid=0 in
> the first place?

There is a number of reasons.
1. Lots of kernel functionalities are protected by CAP_SYS_ADMIN for
unclear reasons. Changing global resources would be reasonable to
protect by root (e.g. changing hostname or eth0). But say if I am
mounting local image file in my local directory, or applying bpf
filter to my socket that should be nobody's business because it does
not affect anybody else. But these things require CAP_SYS_ADMIN in
Linux. As a result users use sudo left and right, which leads to a
mismatch between what users expect and what kernel provides. If you
mount a USB stick or image downloaded from internet, there is no way
you can verify each and every byte of it manually (and it's not work
for humans, it's work for machines), so you just sudo mount it
assuming kernel will handle any bad data. But kernel assumes it's
protected by root, so no particular guarantees, all responsibility is
on you. The only way to shake out bugs in mount/bpf/etc is to test
them, but it requires CAP_SYS_ADMIN.
2. Kernel should not crash on invalid inputs in most cases even if
they are provided by root. Root can make mistakes, programs that root
uses can contain bugs. Useful diagnostics are always better than
silently corrupting memory.
3. On Android/with lockdown/security modules root is not necessary
completely trusted entity. Using a memory corruption root can e.g.
unlock lockdown.
4. Last but not least, using root functionalities may be required to
find critical non-root provoked bugs. Consider you (root) setup
bpf/mount something/setup a socket in a particular, but totally normal
way, but then a memory corruption is caused by a malicious packet
arriving on network (but it requires that root-only setup done on the
host). But giving the fuzzer root we can find such bugs.

We get this far with uid=0. Things were worse initially, then we did
lots of sandbox tuning (e.g. we still do new net ns now even if under
root), some syscall refinements (e.g. no FIFREEZE), disabled
/dev/mem/kmem/ports. There are no major problems now, except for very
rare and isolated cases like this one. I would very much not like to
abandon uid=0 wholesale at this point.

Re: BUG: unable to handle kernel NULL pointer dereference in mem_serial_out

[Greg KH]

On Fri, Dec 13, 2019 at 11:39:54AM +0100, Dmitry Vyukov wrote:
> On Fri, Dec 13, 2019 at 11:10 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, Dec 13, 2019 at 11:00:35AM +0100, Dmitry Vyukov wrote:
> > > On Fri, Dec 13, 2019 at 10:34 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Fri, Dec 13, 2019 at 10:02:33AM +0100, Dmitry Vyukov wrote:
> > > > > On Thu, Dec 12, 2019 at 11:57 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > > >
> > > > > > On Fri, Dec 06, 2019 at 10:25:08PM -0800, syzbot wrote:
> > > > > > > Hello,
> > > > > > >
> > > > > > > syzbot found the following crash on:
> > > > > > >
> > > > > > > HEAD commit:    7ada90eb Merge tag 'drm-next-2019-12-06' of git://anongit...
> > > > > > > git tree:       upstream
> > > > > > > console output: https://syzkaller.appspot.com/x/log.txt?x=123ec282e00000
> > > > > > > kernel config:  https://syzkaller.appspot.com/x/.config?x=f07a23020fd7d21a
> > > > > > > dashboard link: https://syzkaller.appspot.com/bug?extid=f4f1e871965064ae689e
> > > > > > > compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> > > > > > > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=17ab090ee00000
> > > > > > > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=17f127f2e00000
> > > > > > >
> > > > > > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > > > > > Reported-by: syzbot+f4f1e871965064ae689e@syzkaller.appspotmail.com
> > > > > > >
> > > > > > > BUG: kernel NULL pointer dereference, address: 0000000000000002
> > > > > > > #PF: supervisor write access in kernel mode
> > > > > > > #PF: error_code(0x0002) - not-present page
> > > > > > > PGD 9764a067 P4D 9764a067 PUD 9f995067 PMD 0
> > > > > > > Oops: 0002 [#1] PREEMPT SMP KASAN
> > > > > > > CPU: 0 PID: 9687 Comm: syz-executor433 Not tainted 5.4.0-syzkaller #0
> > > > > > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > > > > > > Google 01/01/2011
> > > > > > > RIP: 0010:writeb arch/x86/include/asm/io.h:65 [inline]
> > > > > > > RIP: 0010:mem_serial_out+0x70/0x90 drivers/tty/serial/8250/8250_port.c:408
> > > > > > > Code: e9 00 00 00 49 8d 7c 24 40 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48
> > > > > > > c1 ea 03 d3 e3 80 3c 02 00 75 19 48 63 db 49 03 5c 24 40 <44> 88 2b 5b 41 5c
> > > > > > > 41 5d 5d c3 e8 81 ed cf fd eb c0 e8 da ed cf fd
> > > > > > > RSP: 0018:ffffc90001de78e8 EFLAGS: 00010202
> > > > > > > RAX: dffffc0000000000 RBX: 0000000000000002 RCX: 0000000000000000
> > > > > > > RDX: 1ffffffff181f40e RSI: ffffffff83e28776 RDI: ffffffff8c0fa070
> > > > > > > RBP: ffffc90001de7900 R08: ffff8880919dc340 R09: ffffed10431ee1c6
> > > > > > > R10: ffffed10431ee1c5 R11: ffff888218f70e2b R12: ffffffff8c0fa030
> > > > > > > R13: 0000000000000001 R14: ffffc90001de7a40 R15: ffffffff8c0fa188
> > > > > > > FS:  0000000001060880(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
> > > > > > > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > > > > CR2: 0000000000000002 CR3: 000000009e6b8000 CR4: 00000000001406f0
> > > > > > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > > > > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > > > > > Call Trace:
> > > > > > >  serial_out drivers/tty/serial/8250/8250.h:118 [inline]
> > > > > > >  serial8250_clear_fifos.part.0+0x3a/0xb0
> > > > > > > drivers/tty/serial/8250/8250_port.c:557
> > > > > > >  serial8250_clear_fifos drivers/tty/serial/8250/8250_port.c:556 [inline]
> > > > > > >  serial8250_do_startup+0x426/0x1cf0 drivers/tty/serial/8250/8250_port.c:2121
> > > > > > >  serial8250_startup+0x62/0x80 drivers/tty/serial/8250/8250_port.c:2329
> > > > > > >  uart_port_startup drivers/tty/serial/serial_core.c:219 [inline]
> > > > > > >  uart_startup drivers/tty/serial/serial_core.c:258 [inline]
> > > > > > >  uart_startup+0x452/0x980 drivers/tty/serial/serial_core.c:249
> > > > > > >  uart_set_info drivers/tty/serial/serial_core.c:998 [inline]
> > > > > > >  uart_set_info_user+0x13b4/0x1cf0 drivers/tty/serial/serial_core.c:1023
> > > > > > >  tty_tiocsserial drivers/tty/tty_io.c:2506 [inline]
> > > > > > >  tty_ioctl+0xf60/0x14f0 drivers/tty/tty_io.c:2648
> > > > > > >  vfs_ioctl fs/ioctl.c:47 [inline]
> > > > > > >  file_ioctl fs/ioctl.c:545 [inline]
> > > > > > >  do_vfs_ioctl+0x977/0x14e0 fs/ioctl.c:732
> > > > > > >  ksys_ioctl+0xab/0xd0 fs/ioctl.c:749
> > > > > > >  __do_sys_ioctl fs/ioctl.c:756 [inline]
> > > > > > >  __se_sys_ioctl fs/ioctl.c:754 [inline]
> > > > > > >  __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:754
> > > > > > >  do_syscall_64+0xfa/0x790 arch/x86/entry/common.c:294
> > > > > > >  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > > > > > > RIP: 0033:0x440219
> > > > > > > Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 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
> > > > > > > 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
> > > > > > > RSP: 002b:00007ffced648c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> > > > > > > RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000440219
> > > > > > > RDX: 0000000020000240 RSI: 000000000000541f RDI: 0000000000000003
> > > > > > > RBP: 00000000006ca018 R08: 0000000000000000 R09: 00000000004002c8
> > > > > > > R10: 0000000000401b30 R11: 0000000000000246 R12: 0000000000401aa0
> > > > > > > R13: 0000000000401b30 R14: 0000000000000000 R15: 0000000000000000
> > > > > > > Modules linked in:
> > > > > > > CR2: 0000000000000002
> > > > > > > ---[ end trace eaa11ffe82f3a763 ]---
> > > > > > > RIP: 0010:writeb arch/x86/include/asm/io.h:65 [inline]
> > > > > > > RIP: 0010:mem_serial_out+0x70/0x90 drivers/tty/serial/8250/8250_port.c:408
> > > > > > > Code: e9 00 00 00 49 8d 7c 24 40 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48
> > > > > > > c1 ea 03 d3 e3 80 3c 02 00 75 19 48 63 db 49 03 5c 24 40 <44> 88 2b 5b 41 5c
> > > > > > > 41 5d 5d c3 e8 81 ed cf fd eb c0 e8 da ed cf fd
> > > > > > > RSP: 0018:ffffc90001de78e8 EFLAGS: 00010202
> > > > > > > RAX: dffffc0000000000 RBX: 0000000000000002 RCX: 0000000000000000
> > > > > > > RDX: 1ffffffff181f40e RSI: ffffffff83e28776 RDI: ffffffff8c0fa070
> > > > > > > RBP: ffffc90001de7900 R08: ffff8880919dc340 R09: ffffed10431ee1c6
> > > > > > > R10: ffffed10431ee1c5 R11: ffff888218f70e2b R12: ffffffff8c0fa030
> > > > > > > R13: 0000000000000001 R14: ffffc90001de7a40 R15: ffffffff8c0fa188
> > > > > > > FS:  0000000001060880(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
> > > > > > > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > > > > CR2: 0000000000000002 CR3: 000000009e6b8000 CR4: 00000000001406f0
> > > > > > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > > > > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > > > > >
> > > > > >
> > > > > > You set up a dubious memory base for your uart and then get upset when
> > > > > > you write to that location.
> > > > > >
> > > > > > I don't know what to really do about this, this is a root-only operation
> > > > > > and you are expected to know what you are doing when you attempt this.
> > > > >
> > > > > Hi Greg,
> > > > >
> > > > > Thanks for looking into this!
> > > > > Should we restrict the fuzzer from accessing /dev/ttyS* entirely?
> > > >
> > > > No, not at all.
> > > >
> > > > > Or only restrict TIOCSSERIAL on them? Something else?
> > > >
> > > > Try running not as root.  if you have CAP_SYS_ADMIN you can do a lot of
> > > > pretty bad things with tty ports, as you see here.  There's a reason the
> > > > LOCKDOWN_TIOCSSERIAL "security lockdown" check was added :)
> > > >
> > > > The TIOCSSERIAL ioctl is a nice one for a lot of things that are able to
> > > > be done as a normal user (baud rate changes, etc.), but there are also
> > > > things like setting io port memory locations that can cause random
> > > > hardware accesses and kernel crashes, as you instantly found out here :)
> > > >
> > > > So restrict the fuzzer to only run as a "normal" user of the serial
> > > > port, and if you find problems there, I'll be glad to look at them.
> > >
> > > Easier said than done. "normal user of the serial port" is not really
> > > a thing in Linux, right? You either have CAP_SYS_ADMIN or not, that's
> > > not per-device...
> >
> > Not true, there's lots of users of serial port devices that do not have
> > CAP_SYS_ADMIN set.  That's why we have groups :)
> >
> > You can change the baud rate of your usb-serial device without root
> > permissions, right?  That's a "normal" user right there.
> 
> Yes, but this requires dropping CAP_SYS_ADMIN. And one can't drop
> CAP_SYS_ADMIN only for ttyS. If it would be a separate capability, we
> could drop just that, but not CAP_SYS_ADMIN.

Ok, I think we are talking past each other here.  I am saying that it is
fine to talk to a serial port without CAP_SYS_ADMIN.  That should be
"safe" and not cause bad things to happen.

But if you do have CAP_SYS_ADMIN, you can do a lot more "bad" things
with a serial port (like setting memory addresses).

Your tool always has this capability, which is fine, but does not mean
that serial port accesses by everyone has to have that capability, which
is what I thought you were saying.

> > > As far as I remember +Tetsuo proposed a config along the lines of
> > > "restrict only things that legitimately cause damage under a fuzzer
> > > workload", e.g. freezing filesystems, disabling console output, etc.
> > > This may be another candidate. But I can't find where that proposal is
> > > now.
> > >
> > > A simpler option that I see is as follows. syzkaller has several
> > > sandboxing modes, one of them is "namespace" which uses a user ns, in
> > > that more fuzzer is still uid=0 in the init namespace, so has access
> > > to all /dev nodes, but it does not have CAP_SYS_ADMIN in the init
> > > namespace. We could enable /dev/ttyS* only on instance that use
> > > sandbox=namesace, and disable on the rest. Does it make sense?
> >
> > Maybe, I don't know.  Why do you have to run the fuzzer with uid=0 in
> > the first place?
> 
> There is a number of reasons.
> 1. Lots of kernel functionalities are protected by CAP_SYS_ADMIN for
> unclear reasons. Changing global resources would be reasonable to
> protect by root (e.g. changing hostname or eth0). But say if I am
> mounting local image file in my local directory, or applying bpf
> filter to my socket that should be nobody's business because it does
> not affect anybody else. But these things require CAP_SYS_ADMIN in
> Linux. As a result users use sudo left and right, which leads to a
> mismatch between what users expect and what kernel provides. If you
> mount a USB stick or image downloaded from internet, there is no way
> you can verify each and every byte of it manually (and it's not work
> for humans, it's work for machines), so you just sudo mount it
> assuming kernel will handle any bad data. But kernel assumes it's
> protected by root, so no particular guarantees, all responsibility is
> on you. The only way to shake out bugs in mount/bpf/etc is to test
> them, but it requires CAP_SYS_ADMIN.

Ok, that's fine, but don't test serial ports with that setting and
expect to have a "safe" system :)

> 2. Kernel should not crash on invalid inputs in most cases even if
> they are provided by root. Root can make mistakes, programs that root
> uses can contain bugs. Useful diagnostics are always better than
> silently corrupting memory.

Fine, but we have legacy apis that we have to support, like setting
memory locations of ports, and those we protect with that capability.
We can't change this, sorry, and so if you try to fuzz it, maybe just do
not change those "known to be able to do bad things" settings.

> 3. On Android/with lockdown/security modules root is not necessary
> completely trusted entity. Using a memory corruption root can e.g.
> unlock lockdown.

Fine, but again, we can't remove these old apis.

> 4. Last but not least, using root functionalities may be required to
> find critical non-root provoked bugs. Consider you (root) setup
> bpf/mount something/setup a socket in a particular, but totally normal
> way, but then a memory corruption is caused by a malicious packet
> arriving on network (but it requires that root-only setup done on the
> host). But giving the fuzzer root we can find such bugs.
> 
> We get this far with uid=0. Things were worse initially, then we did
> lots of sandbox tuning (e.g. we still do new net ns now even if under
> root), some syscall refinements (e.g. no FIFREEZE), disabled
> /dev/mem/kmem/ports. There are no major problems now, except for very
> rare and isolated cases like this one. I would very much not like to
> abandon uid=0 wholesale at this point.

Ok, then don't abandon it, just be more judicious when fuzzing the
serial layer with what you do, and do not, change.

thanks,

greg k-h

Re: BUG: unable to handle kernel NULL pointer dereference in mem_serial_out

[Dmitry Vyukov]

On Fri, Dec 13, 2019 at 10:48 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Fri, Dec 13, 2019 at 11:39:54AM +0100, Dmitry Vyukov wrote:
> > On Fri, Dec 13, 2019 at 11:10 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > > > > You set up a dubious memory base for your uart and then get upset when
> > > > > > > you write to that location.
> > > > > > >
> > > > > > > I don't know what to really do about this, this is a root-only operation
> > > > > > > and you are expected to know what you are doing when you attempt this.
> > > > > >
> > > > > > Hi Greg,
> > > > > >
> > > > > > Thanks for looking into this!
> > > > > > Should we restrict the fuzzer from accessing /dev/ttyS* entirely?
> > > > >
> > > > > No, not at all.
> > > > >
> > > > > > Or only restrict TIOCSSERIAL on them? Something else?
> > > > >
> > > > > Try running not as root.  if you have CAP_SYS_ADMIN you can do a lot of
> > > > > pretty bad things with tty ports, as you see here.  There's a reason the
> > > > > LOCKDOWN_TIOCSSERIAL "security lockdown" check was added :)
> > > > >
> > > > > The TIOCSSERIAL ioctl is a nice one for a lot of things that are able to
> > > > > be done as a normal user (baud rate changes, etc.), but there are also
> > > > > things like setting io port memory locations that can cause random
> > > > > hardware accesses and kernel crashes, as you instantly found out here :)
> > > > >
> > > > > So restrict the fuzzer to only run as a "normal" user of the serial
> > > > > port, and if you find problems there, I'll be glad to look at them.
> > > >
> > > > Easier said than done. "normal user of the serial port" is not really
> > > > a thing in Linux, right? You either have CAP_SYS_ADMIN or not, that's
> > > > not per-device...
> > >
> > > Not true, there's lots of users of serial port devices that do not have
> > > CAP_SYS_ADMIN set.  That's why we have groups :)
> > >
> > > You can change the baud rate of your usb-serial device without root
> > > permissions, right?  That's a "normal" user right there.
> >
> > Yes, but this requires dropping CAP_SYS_ADMIN. And one can't drop
> > CAP_SYS_ADMIN only for ttyS. If it would be a separate capability, we
> > could drop just that, but not CAP_SYS_ADMIN.
>
> Ok, I think we are talking past each other here.  I am saying that it is
> fine to talk to a serial port without CAP_SYS_ADMIN.  That should be
> "safe" and not cause bad things to happen.
>
> But if you do have CAP_SYS_ADMIN, you can do a lot more "bad" things
> with a serial port (like setting memory addresses).
>
> Your tool always has this capability, which is fine, but does not mean
> that serial port accesses by everyone has to have that capability, which
> is what I thought you were saying.

I mean that I don't see a realistic way to apply your "Try running not
as root" suggestion.
We can drop root, but that will dramatic effect on lots of other
things that has nothing to do with serial console.



> > > > As far as I remember +Tetsuo proposed a config along the lines of
> > > > "restrict only things that legitimately cause damage under a fuzzer
> > > > workload", e.g. freezing filesystems, disabling console output, etc.
> > > > This may be another candidate. But I can't find where that proposal is
> > > > now.
> > > >
> > > > A simpler option that I see is as follows. syzkaller has several
> > > > sandboxing modes, one of them is "namespace" which uses a user ns, in
> > > > that more fuzzer is still uid=0 in the init namespace, so has access
> > > > to all /dev nodes, but it does not have CAP_SYS_ADMIN in the init
> > > > namespace. We could enable /dev/ttyS* only on instance that use
> > > > sandbox=namesace, and disable on the rest. Does it make sense?
> > >
> > > Maybe, I don't know.  Why do you have to run the fuzzer with uid=0 in
> > > the first place?
> >
> > There is a number of reasons.
> > 1. Lots of kernel functionalities are protected by CAP_SYS_ADMIN for
> > unclear reasons. Changing global resources would be reasonable to
> > protect by root (e.g. changing hostname or eth0). But say if I am
> > mounting local image file in my local directory, or applying bpf
> > filter to my socket that should be nobody's business because it does
> > not affect anybody else. But these things require CAP_SYS_ADMIN in
> > Linux. As a result users use sudo left and right, which leads to a
> > mismatch between what users expect and what kernel provides. If you
> > mount a USB stick or image downloaded from internet, there is no way
> > you can verify each and every byte of it manually (and it's not work
> > for humans, it's work for machines), so you just sudo mount it
> > assuming kernel will handle any bad data. But kernel assumes it's
> > protected by root, so no particular guarantees, all responsibility is
> > on you. The only way to shake out bugs in mount/bpf/etc is to test
> > them, but it requires CAP_SYS_ADMIN.
>
> Ok, that's fine, but don't test serial ports with that setting and
> expect to have a "safe" system :)
>
> > 2. Kernel should not crash on invalid inputs in most cases even if
> > they are provided by root. Root can make mistakes, programs that root
> > uses can contain bugs. Useful diagnostics are always better than
> > silently corrupting memory.
>
> Fine, but we have legacy apis that we have to support, like setting
> memory locations of ports, and those we protect with that capability.
> We can't change this, sorry, and so if you try to fuzz it, maybe just do
> not change those "known to be able to do bad things" settings.

Precisely controlling what's being done is only possible in unit
testing setting. It's not that simple in a fuzzer setting. Precisely
restricting behavior of random programs is both (1) extremely complex,
(2) will over-restrict, read, make fuzzer less efficient, (3) still
most likely will be unreliable with lots of corner cases.
So I am looking for a more practical solution.


> > 3. On Android/with lockdown/security modules root is not necessary
> > completely trusted entity. Using a memory corruption root can e.g.
> > unlock lockdown.
>
> Fine, but again, we can't remove these old apis.
>
> > 4. Last but not least, using root functionalities may be required to
> > find critical non-root provoked bugs. Consider you (root) setup
> > bpf/mount something/setup a socket in a particular, but totally normal
> > way, but then a memory corruption is caused by a malicious packet
> > arriving on network (but it requires that root-only setup done on the
> > host). But giving the fuzzer root we can find such bugs.
> >
> > We get this far with uid=0. Things were worse initially, then we did
> > lots of sandbox tuning (e.g. we still do new net ns now even if under
> > root), some syscall refinements (e.g. no FIFREEZE), disabled
> > /dev/mem/kmem/ports. There are no major problems now, except for very
> > rare and isolated cases like this one. I would very much not like to
> > abandon uid=0 wholesale at this point.
>
> Ok, then don't abandon it, just be more judicious when fuzzing the
> serial layer with what you do, and do not, change.
>
> thanks,
>
> greg k-h

Re: BUG: unable to handle kernel NULL pointer dereference in mem_serial_out

[Dmitry Vyukov]

On Tue, Dec 17, 2019 at 11:48 AM Dmitry Vyukov <dvyukov@google.com> wrote:
>
> On Fri, Dec 13, 2019 at 10:48 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, Dec 13, 2019 at 11:39:54AM +0100, Dmitry Vyukov wrote:
> > > On Fri, Dec 13, 2019 at 11:10 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > > > > > You set up a dubious memory base for your uart and then get upset when
> > > > > > > > you write to that location.
> > > > > > > >
> > > > > > > > I don't know what to really do about this, this is a root-only operation
> > > > > > > > and you are expected to know what you are doing when you attempt this.
> > > > > > >
> > > > > > > Hi Greg,
> > > > > > >
> > > > > > > Thanks for looking into this!
> > > > > > > Should we restrict the fuzzer from accessing /dev/ttyS* entirely?
> > > > > >
> > > > > > No, not at all.
> > > > > >
> > > > > > > Or only restrict TIOCSSERIAL on them? Something else?
> > > > > >
> > > > > > Try running not as root.  if you have CAP_SYS_ADMIN you can do a lot of
> > > > > > pretty bad things with tty ports, as you see here.  There's a reason the
> > > > > > LOCKDOWN_TIOCSSERIAL "security lockdown" check was added :)
> > > > > >
> > > > > > The TIOCSSERIAL ioctl is a nice one for a lot of things that are able to
> > > > > > be done as a normal user (baud rate changes, etc.), but there are also
> > > > > > things like setting io port memory locations that can cause random
> > > > > > hardware accesses and kernel crashes, as you instantly found out here :)
> > > > > >
> > > > > > So restrict the fuzzer to only run as a "normal" user of the serial
> > > > > > port, and if you find problems there, I'll be glad to look at them.
> > > > >
> > > > > Easier said than done. "normal user of the serial port" is not really
> > > > > a thing in Linux, right? You either have CAP_SYS_ADMIN or not, that's
> > > > > not per-device...
> > > >
> > > > Not true, there's lots of users of serial port devices that do not have
> > > > CAP_SYS_ADMIN set.  That's why we have groups :)
> > > >
> > > > You can change the baud rate of your usb-serial device without root
> > > > permissions, right?  That's a "normal" user right there.
> > >
> > > Yes, but this requires dropping CAP_SYS_ADMIN. And one can't drop
> > > CAP_SYS_ADMIN only for ttyS. If it would be a separate capability, we
> > > could drop just that, but not CAP_SYS_ADMIN.
> >
> > Ok, I think we are talking past each other here.  I am saying that it is
> > fine to talk to a serial port without CAP_SYS_ADMIN.  That should be
> > "safe" and not cause bad things to happen.
> >
> > But if you do have CAP_SYS_ADMIN, you can do a lot more "bad" things
> > with a serial port (like setting memory addresses).
> >
> > Your tool always has this capability, which is fine, but does not mean
> > that serial port accesses by everyone has to have that capability, which
> > is what I thought you were saying.
>
> I mean that I don't see a realistic way to apply your "Try running not
> as root" suggestion.
> We can drop root, but that will dramatic effect on lots of other
> things that has nothing to do with serial console.

I've disabled testing of TIOCSSERIAL in syzkaller:
https://github.com/google/syzkaller/commit/af9047c60a3db32d5e43c29321f8f531db051a63

#syz invalid

Re: BUG: unable to handle kernel NULL pointer dereference in mem_serial_out

[Tetsuo Handa]

On 2019/12/13 19:00, Dmitry Vyukov wrote:
> Easier said than done. "normal user of the serial port" is not really
> a thing in Linux, right? You either have CAP_SYS_ADMIN or not, that's
> not per-device...
> As far as I remember +Tetsuo proposed a config along the lines of
> "restrict only things that legitimately cause damage under a fuzzer
> workload", e.g. freezing filesystems, disabling console output, etc.
> This may be another candidate. But I can't find where that proposal is
> now.

That suggestion got no response for two months.

  https://lkml.kernel.org/r/3e4e2b6b-7828-54ab-cf28-db1a396d7e20@i-love.sakura.ne.jp

Unless we add such kernel config option to upstream kernels, it will become
a whack-a-mole game.

Re: BUG: unable to handle kernel NULL pointer dereference in mem_serial_out

[Greg KH]

On Fri, Dec 13, 2019 at 11:31:08PM +0900, Tetsuo Handa wrote:
> On 2019/12/13 19:00, Dmitry Vyukov wrote:
> > Easier said than done. "normal user of the serial port" is not really
> > a thing in Linux, right? You either have CAP_SYS_ADMIN or not, that's
> > not per-device...
> > As far as I remember +Tetsuo proposed a config along the lines of
> > "restrict only things that legitimately cause damage under a fuzzer
> > workload", e.g. freezing filesystems, disabling console output, etc.
> > This may be another candidate. But I can't find where that proposal is
> > now.
> 
> That suggestion got no response for two months.
> 
>   https://lkml.kernel.org/r/3e4e2b6b-7828-54ab-cf28-db1a396d7e20@i-love.sakura.ne.jp
> 
> Unless we add such kernel config option to upstream kernels, it will become
> a whack-a-mole game.

It will be a whack-a-mole game no matter what.

Yes, /dev/mem/ makes no sense to fuzz.  Neither does other things (like
serial port memory addresses.)

You just will have a list of things that you "do not fuzz as these are
dangerous".  Nothing new here, any os will have that.

thanks,

greg k-h

Re: BUG: unable to handle kernel NULL pointer dereference in mem_serial_out

[Tetsuo Handa]

On 2019/12/14 1:07, Greg KH wrote:
> On Fri, Dec 13, 2019 at 11:31:08PM +0900, Tetsuo Handa wrote:
>> On 2019/12/13 19:00, Dmitry Vyukov wrote:
>>> Easier said than done. "normal user of the serial port" is not really
>>> a thing in Linux, right? You either have CAP_SYS_ADMIN or not, that's
>>> not per-device...
>>> As far as I remember +Tetsuo proposed a config along the lines of
>>> "restrict only things that legitimately cause damage under a fuzzer
>>> workload", e.g. freezing filesystems, disabling console output, etc.
>>> This may be another candidate. But I can't find where that proposal is
>>> now.
>>
>> That suggestion got no response for two months.
>>
>>   https://lkml.kernel.org/r/3e4e2b6b-7828-54ab-cf28-db1a396d7e20@i-love.sakura.ne.jp
>>
>> Unless we add such kernel config option to upstream kernels, it will become
>> a whack-a-mole game.
> 
> It will be a whack-a-mole game no matter what.
> 
> Yes, /dev/mem/ makes no sense to fuzz.  Neither does other things (like
> serial port memory addresses.)

/dev/mem makes sense to fuzz. Ditto for other things.

> 
> You just will have a list of things that you "do not fuzz as these are
> dangerous".  Nothing new here, any os will have that.

The list of kernel config options will become too complicated to maintain.
If we can have one kernel config option, we can avoid maintaining
the list of kernel config options (which keeps changing over time).

Re: BUG: unable to handle kernel NULL pointer dereference in mem_serial_out

[Greg KH]

On Sat, Dec 14, 2019 at 09:48:29AM +0900, Tetsuo Handa wrote:
> On 2019/12/14 1:07, Greg KH wrote:
> > On Fri, Dec 13, 2019 at 11:31:08PM +0900, Tetsuo Handa wrote:
> >> On 2019/12/13 19:00, Dmitry Vyukov wrote:
> >>> Easier said than done. "normal user of the serial port" is not really
> >>> a thing in Linux, right? You either have CAP_SYS_ADMIN or not, that's
> >>> not per-device...
> >>> As far as I remember +Tetsuo proposed a config along the lines of
> >>> "restrict only things that legitimately cause damage under a fuzzer
> >>> workload", e.g. freezing filesystems, disabling console output, etc.
> >>> This may be another candidate. But I can't find where that proposal is
> >>> now.
> >>
> >> That suggestion got no response for two months.
> >>
> >>   https://lkml.kernel.org/r/3e4e2b6b-7828-54ab-cf28-db1a396d7e20@i-love.sakura.ne.jp
> >>
> >> Unless we add such kernel config option to upstream kernels, it will become
> >> a whack-a-mole game.
> > 
> > It will be a whack-a-mole game no matter what.
> > 
> > Yes, /dev/mem/ makes no sense to fuzz.  Neither does other things (like
> > serial port memory addresses.)
> 
> /dev/mem makes sense to fuzz. Ditto for other things.

What?  What are you going to find if you randomly start to write to
/dev/mem?  How are we supposed to "fix" that?

> > You just will have a list of things that you "do not fuzz as these are
> > dangerous".  Nothing new here, any os will have that.
> 
> The list of kernel config options will become too complicated to maintain.
> If we can have one kernel config option, we can avoid maintaining
> the list of kernel config options (which keeps changing over time).

Use the newly added security_locked_down() call, that gives you a great
indication that root can cause problems for those things.

And it's not a config thing, it's a functionality thing within features,
as is explicitly shown by this very thread for the serial port memory
location.

thanks,

greg k-h

Re: BUG: unable to handle kernel NULL pointer dereference in mem_serial_out

[Tetsuo Handa]

On 2019/12/14 16:55, Greg KH wrote:
>>>> That suggestion got no response for two months.
>>>>
>>>>   https://lkml.kernel.org/r/3e4e2b6b-7828-54ab-cf28-db1a396d7e20@i-love.sakura.ne.jp
>>>>
>>>> Unless we add such kernel config option to upstream kernels, it will become
>>>> a whack-a-mole game.
>>>
>>> It will be a whack-a-mole game no matter what.
>>>
>>> Yes, /dev/mem/ makes no sense to fuzz.  Neither does other things (like
>>> serial port memory addresses.)
>>
>> /dev/mem makes sense to fuzz. Ditto for other things.
> 
> What?  What are you going to find if you randomly start to write to
> /dev/mem?  How are we supposed to "fix" that?
> 

When did I say "writing to random addresses" ? If you saw my suggestion, you
will find that "fuzzer will be able to test reading from random addresses,
writing to safe addresses (in order to find new lock dependency which would
otherwise be unnoticed)".

Disabling everything using kernel config option is overkill. What you are saying
is "never try to fuzz USB drivers" by excluding USB drivers from fuzz targets.
There is no need to disable whole tests. We need to exclude only stupid operations
(e.g. forever repeating SysRq-t) from fuzz targets.

>>> You just will have a list of things that you "do not fuzz as these are
>>> dangerous".  Nothing new here, any os will have that.
>>
>> The list of kernel config options will become too complicated to maintain.
>> If we can have one kernel config option, we can avoid maintaining
>> the list of kernel config options (which keeps changing over time).
> 
> Use the newly added security_locked_down() call, that gives you a great
> indication that root can cause problems for those things.
> 

No. security_locked_down() is not for fuzz kernels but for real kernels.

"enum lockdown_reason" is overkill, it just keeps fuzzers away from finding bugs.
For example, if ftrace code has bugs but ftrace_event_open() prevents from
fuzzing due to security_locked_down(LOCKDOWN_TRACEFS) ? Fuzz kernels should not
count on security_locked_down() returning errors. That is no different from
disabling everything using kernel config options.

> And it's not a config thing, it's a functionality thing within features,
> as is explicitly shown by this very thread for the serial port memory
> location.

My suggestion is not for real kernels but for fuzz kernels.

Re: BUG: unable to handle kernel NULL pointer dereference in mem_serial_out

[Greg KH]

On Sat, Dec 14, 2019 at 05:39:02PM +0900, Tetsuo Handa wrote:
> On 2019/12/14 16:55, Greg KH wrote:
> >>>> That suggestion got no response for two months.
> >>>>
> >>>>   https://lkml.kernel.org/r/3e4e2b6b-7828-54ab-cf28-db1a396d7e20@i-love.sakura.ne.jp
> >>>>
> >>>> Unless we add such kernel config option to upstream kernels, it will become
> >>>> a whack-a-mole game.
> >>>
> >>> It will be a whack-a-mole game no matter what.
> >>>
> >>> Yes, /dev/mem/ makes no sense to fuzz.  Neither does other things (like
> >>> serial port memory addresses.)
> >>
> >> /dev/mem makes sense to fuzz. Ditto for other things.
> > 
> > What?  What are you going to find if you randomly start to write to
> > /dev/mem?  How are we supposed to "fix" that?
> > 
> 
> When did I say "writing to random addresses" ? If you saw my suggestion, you
> will find that "fuzzer will be able to test reading from random addresses,
> writing to safe addresses (in order to find new lock dependency which would
> otherwise be unnoticed)".

I don't remember the suggestion specifically, sorry.  But how would you
figure out what those "safe addresses" really are?  They will change on
every single platform.

And why would this even help anything?  What lock dependency?

> Disabling everything using kernel config option is overkill. What you are saying
> is "never try to fuzz USB drivers" by excluding USB drivers from fuzz targets.

I never said that.

> There is no need to disable whole tests. We need to exclude only stupid operations
> (e.g. forever repeating SysRq-t) from fuzz targets.

Ok, like I said, configuring serial ports as root is a stupid operation
to think it is totally safe :)

> >>> You just will have a list of things that you "do not fuzz as these are
> >>> dangerous".  Nothing new here, any os will have that.
> >>
> >> The list of kernel config options will become too complicated to maintain.
> >> If we can have one kernel config option, we can avoid maintaining
> >> the list of kernel config options (which keeps changing over time).
> > 
> > Use the newly added security_locked_down() call, that gives you a great
> > indication that root can cause problems for those things.
> > 
> 
> No. security_locked_down() is not for fuzz kernels but for real kernels.

Yes, but it gives you a huge hint that this is something that you could
do as root that is "bad" and can harm the system.

> "enum lockdown_reason" is overkill, it just keeps fuzzers away from finding bugs.
> For example, if ftrace code has bugs but ftrace_event_open() prevents from
> fuzzing due to security_locked_down(LOCKDOWN_TRACEFS) ? Fuzz kernels should not
> count on security_locked_down() returning errors. That is no different from
> disabling everything using kernel config options.

They all might not be correct, but again they provide a hint.

> > And it's not a config thing, it's a functionality thing within features,
> > as is explicitly shown by this very thread for the serial port memory
> > location.
> 
> My suggestion is not for real kernels but for fuzz kernels.

So fuzz kernels are not real?  :)

Re: BUG: unable to handle kernel NULL pointer dereference in mem_serial_out

[Tetsuo Handa]

On 2019/12/14 18:09, Greg KH wrote:
> On Sat, Dec 14, 2019 at 05:39:02PM +0900, Tetsuo Handa wrote:
>> On 2019/12/14 16:55, Greg KH wrote:
>>>>>> That suggestion got no response for two months.
>>>>>>
>>>>>>   https://lkml.kernel.org/r/3e4e2b6b-7828-54ab-cf28-db1a396d7e20@i-love.sakura.ne.jp
>>>>>>
>>>>>> Unless we add such kernel config option to upstream kernels, it will become
>>>>>> a whack-a-mole game.
>>>>>
>>>>> It will be a whack-a-mole game no matter what.
>>>>>
>>>>> Yes, /dev/mem/ makes no sense to fuzz.  Neither does other things (like
>>>>> serial port memory addresses.)
>>>>
>>>> /dev/mem makes sense to fuzz. Ditto for other things.
>>>
>>> What?  What are you going to find if you randomly start to write to
>>> /dev/mem?  How are we supposed to "fix" that?
>>>
>>
>> When did I say "writing to random addresses" ? If you saw my suggestion, you
>> will find that "fuzzer will be able to test reading from random addresses,
>> writing to safe addresses (in order to find new lock dependency which would
>> otherwise be unnoticed)".
> 
> I don't remember the suggestion specifically, sorry.  But how would you
> figure out what those "safe addresses" really are?  They will change on
> every single platform.

----------
+#ifdef CONFIG_KERNEL_BUILT_FOR_FUZZ_TESTING
+static char dummybuf[PAGE_SIZE];
+#endif
----------

----------
                        ptr = xlate_dev_mem_ptr(p);
                        if (!ptr) {
                                if (written)
                                        break;
                                return -EFAULT;
                        }
+#ifndef CONFIG_KERNEL_BUILT_FOR_FUZZ_TESTING
                        copied = copy_from_user(ptr, buf, sz);
+#else
+                       copied = copy_from_user(dummybuf, buf, min(sizeof(dummybuf), sz));
+#endif
                        unxlate_dev_mem_ptr(p, ptr);
----------

How dummybuf cannot be "safe address" ?

> 
> And why would this even help anything?  What lock dependency?
> 

copy_from_user() can trigger page fault which involves memory allocation.
And direct reclaim which is performed within memory allocation operation
is full of subtle dependency bugs. :-(

Re: BUG: unable to handle kernel NULL pointer dereference in mem_serial_out

[Greg KH]

On Sat, Dec 14, 2019 at 07:28:02PM +0900, Tetsuo Handa wrote:
> On 2019/12/14 18:09, Greg KH wrote:
> > On Sat, Dec 14, 2019 at 05:39:02PM +0900, Tetsuo Handa wrote:
> >> On 2019/12/14 16:55, Greg KH wrote:
> >>>>>> That suggestion got no response for two months.
> >>>>>>
> >>>>>>   https://lkml.kernel.org/r/3e4e2b6b-7828-54ab-cf28-db1a396d7e20@i-love.sakura.ne.jp
> >>>>>>
> >>>>>> Unless we add such kernel config option to upstream kernels, it will become
> >>>>>> a whack-a-mole game.
> >>>>>
> >>>>> It will be a whack-a-mole game no matter what.
> >>>>>
> >>>>> Yes, /dev/mem/ makes no sense to fuzz.  Neither does other things (like
> >>>>> serial port memory addresses.)
> >>>>
> >>>> /dev/mem makes sense to fuzz. Ditto for other things.
> >>>
> >>> What?  What are you going to find if you randomly start to write to
> >>> /dev/mem?  How are we supposed to "fix" that?
> >>>
> >>
> >> When did I say "writing to random addresses" ? If you saw my suggestion, you
> >> will find that "fuzzer will be able to test reading from random addresses,
> >> writing to safe addresses (in order to find new lock dependency which would
> >> otherwise be unnoticed)".
> > 
> > I don't remember the suggestion specifically, sorry.  But how would you
> > figure out what those "safe addresses" really are?  They will change on
> > every single platform.
> 
> ----------
> +#ifdef CONFIG_KERNEL_BUILT_FOR_FUZZ_TESTING
> +static char dummybuf[PAGE_SIZE];
> +#endif
> ----------
> 
> ----------
>                         ptr = xlate_dev_mem_ptr(p);
>                         if (!ptr) {
>                                 if (written)
>                                         break;
>                                 return -EFAULT;
>                         }
> +#ifndef CONFIG_KERNEL_BUILT_FOR_FUZZ_TESTING
>                         copied = copy_from_user(ptr, buf, sz);
> +#else
> +                       copied = copy_from_user(dummybuf, buf, min(sizeof(dummybuf), sz));
> +#endif

Ick ick ick, we don't like #ifdefs in .c files for a reason :)

>                         unxlate_dev_mem_ptr(p, ptr);
> ----------
> 
> How dummybuf cannot be "safe address" ?

Sure, that's safe, but don't try to change what I originally said about
writing to random /dev/mem addresses please.

> > And why would this even help anything?  What lock dependency?
> > 
> 
> copy_from_user() can trigger page fault which involves memory allocation.
> And direct reclaim which is performed within memory allocation operation
> is full of subtle dependency bugs. :-(

It's as if you want a memory/platform type to be a "fuzz target", right?
You aren't going to go around and try to put the above crazyness into
each and every spot in the kernel that wants to write to a
user-specified address.

Fuzzing the kernel is great, but remember it's a means to an end, that
of trying to ensure that the kernel has less bugs than before.  Forcing
the kernel to adapt a memory model that is not what it "normally" uses
kind of goes against your main goal here, right?

And have you all really run out of apis that are all now bug free that
you are going after these types of ones?

thanks,

greg k-h

Re: BUG: unable to handle kernel NULL pointer dereference in mem_serial_out

[Tetsuo Handa]

Hmm, this is a surprising bug. syzbot provided a C reproducer, but the definition
of "struct serial_struct" used in that reproducer is wrong. As a result, syzbot was
reporting crash caused by passing wrong arguments. ;-)

close_delay field used in the C reproducer is sizeof(unsigned int) bytes rather than
sizeof(unsigned short) bytes, thus fields after close_delay field are incorrectly
interpreted.

----------------------------------------
#include <stdio.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <sys/ioctl.h>
#include <linux/serial.h>

struct bad_serial_struct {
        int     type;
        int     line;
        unsigned int    port;
        int     irq;
        int     flags;
        int     xmit_fifo_size;
        int     custom_divisor;
        int     baud_base;
        unsigned int    close_delay; /* Correct type is "unsigned short". */
        char    io_type;
        char    reserved_char[1];
        int     hub6;
        unsigned short  closing_wait;
        unsigned short  closing_wait2;
        unsigned char   *iomem_base;
        unsigned short  iomem_reg_shift;
        unsigned int    port_high;
        unsigned long   iomap_base;
};

int main(int argc, char *argv[])
{
        struct bad_serial_struct ss = { };
        int fd = open("/dev/ttyS3", O_RDONLY);
        ss.type = 0xa;
        ss.line = 0x400000;
        ss.port = 0x100;
        ss.irq = 0;
        ss.flags = 0x400000;
        ss.xmit_fifo_size = 0;
        ss.custom_divisor = 0;
        ss.baud_base = 0x80000;
        ss.close_delay = 0x200ff;
        ss.io_type = 0;
        ss.reserved_char[0] = 0x41;
        ss.hub6 = 3;
        ss.closing_wait = 0;
        ss.closing_wait2 = 0x7c5;
        ss.iomem_base = NULL;
        ss.iomem_reg_shift = 0;
        ss.port_high = 0;
        ss.iomap_base = 0;
        ioctl(fd, TIOCSSERIAL, &ss);
        return 0;
}
----------------------------------------

Re: BUG: unable to handle kernel NULL pointer dereference in mem_serial_out

[Dmitry Vyukov]

On Wed, Dec 18, 2019 at 1:56 AM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> Hmm, this is a surprising bug. syzbot provided a C reproducer, but the definition
> of "struct serial_struct" used in that reproducer is wrong. As a result, syzbot was
> reporting crash caused by passing wrong arguments. ;-)

We are on it:
https://github.com/google/syzkaller/blob/master/sys/linux/dev_ptmx.txt.warn#L20-L25

> close_delay field used in the C reproducer is sizeof(unsigned int) bytes rather than
> sizeof(unsigned short) bytes, thus fields after close_delay field are incorrectly
> interpreted.
>
> ----------------------------------------
> #include <stdio.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <fcntl.h>
> #include <sys/ioctl.h>
> #include <linux/serial.h>
>
> struct bad_serial_struct {
>         int     type;
>         int     line;
>         unsigned int    port;
>         int     irq;
>         int     flags;
>         int     xmit_fifo_size;
>         int     custom_divisor;
>         int     baud_base;
>         unsigned int    close_delay; /* Correct type is "unsigned short". */
>         char    io_type;
>         char    reserved_char[1];
>         int     hub6;
>         unsigned short  closing_wait;
>         unsigned short  closing_wait2;
>         unsigned char   *iomem_base;
>         unsigned short  iomem_reg_shift;
>         unsigned int    port_high;
>         unsigned long   iomap_base;
> };
>
> int main(int argc, char *argv[])
> {
>         struct bad_serial_struct ss = { };
>         int fd = open("/dev/ttyS3", O_RDONLY);
>         ss.type = 0xa;
>         ss.line = 0x400000;
>         ss.port = 0x100;
>         ss.irq = 0;
>         ss.flags = 0x400000;
>         ss.xmit_fifo_size = 0;
>         ss.custom_divisor = 0;
>         ss.baud_base = 0x80000;
>         ss.close_delay = 0x200ff;
>         ss.io_type = 0;
>         ss.reserved_char[0] = 0x41;
>         ss.hub6 = 3;
>         ss.closing_wait = 0;
>         ss.closing_wait2 = 0x7c5;
>         ss.iomem_base = NULL;
>         ss.iomem_reg_shift = 0;
>         ss.port_high = 0;
>         ss.iomap_base = 0;
>         ioctl(fd, TIOCSSERIAL, &ss);
>         return 0;
> }
> ----------------------------------------