Re: [PATCHv3 0/6] tty: Hold write ldisc sem in tty_reopen()

2018-09-11 Thread Dmitry Safonov
On Tue, 2018-09-11 at 13:16 +0100, Mark Rutland wrote:
> On Tue, Sep 11, 2018 at 02:48:15AM +0100, Dmitry Safonov wrote:
> > Hi all,
> 
> Hi,
> 
> > Three fixes that worth to have in the @stable, as we've hit them on
> > v4.9
> > stable.
> > 
> > And for linux-next - adding lockdep asserts for line discipline
> > changing
> > code, verifying that write ldisc sem will be held forthwith.
> > 
> > The last patch is optional and probably, timeout can be dropped for
> > read_lock(). I'll do it if everyone agrees.
> > 
> > Rong Chen, could you kindly re-run this version to see if the
> > lockup
> > from v1 still happens? I wasn't able to reproduce it..
> 
> These patches seem to fix issues I've been seeing on arm64 for a
> while
> but hadn't managed to track down.
> 
> For patches 1, 3, and 5, feel free to add:
> 
> Tested-by: Mark Rutland 

Thanks, Mark!
Will add on the next version.

> 
> On vanilla v4.19-rc2, the below reproducer would fire in seconds,
> whereas with those patches applied, I have not seen issues after 10s
> of
> minutes of testing.
> 
> Thanks,
> Mark.
> 
> Syzkaller hit 'KASAN: user-memory-access Write in n_tty_set_termios'
> bug.
> 
> IPv6: ADDRCONF(NETDEV_UP): veth0: link is not ready
> ipV6: ADDRCONF(NETDEV_UP): veth1: link is not ready
> IPv6: ADDRCONF(NETDEV_CHANGE): veth1: link becomes ready
> IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
> ==
> BUG: KASAN: user-memory-access in memset include/linux/string.h:330
> [inline]
> BUG: KASAN: user-memory-access in bitmap_zero
> include/linux/bitmap.h:216 [inline]
> BUG: KASAN: user-memory-access in n_tty_set_termios+0xe4/0xd08
> drivers/tty/n_tty.c:1784
> Write of size 512 at addr 1060 by task syz-executor0/3007
> 
> CPU: 1 PID: 3007 Comm: syz-executor0 Not tainted 4.19.0-rc2-dirty #4
> Hardware name: linux,dummy-virt (DT)
> Call trace:
>  dump_backtrace+0x0/0x340 arch/arm64/include/asm/ptrace.h:270
>  show_stack+0x20/0x30 arch/arm64/kernel/traps.c:152
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0xec/0x150 lib/dump_stack.c:113
>  kasan_report_error mm/kasan/report.c:352 [inline]
>  kasan_report+0x228/0x360 mm/kasan/report.c:412
>  check_memory_region_inline mm/kasan/kasan.c:253 [inline]
>  check_memory_region+0x114/0x1c8 mm/kasan/kasan.c:267
>  memset+0x2c/0x50 mm/kasan/kasan.c:285
>  memset include/linux/string.h:330 [inline]
>  bitmap_zero include/linux/bitmap.h:216 [inline]
>  n_tty_set_termios+0xe4/0xd08 drivers/tty/n_tty.c:1784
>  tty_set_termios+0x538/0x760 drivers/tty/tty_ioctl.c:341
>  set_termios+0x348/0x968 drivers/tty/tty_ioctl.c:414
>  tty_mode_ioctl+0x8f0/0xc60 drivers/tty/tty_ioctl.c:779
>  n_tty_ioctl_helper+0x6c/0x390 drivers/tty/tty_ioctl.c:940
>  n_tty_ioctl+0x6c/0x490 drivers/tty/n_tty.c:2450
>  tty_ioctl+0x610/0x19a8 drivers/tty/tty_io.c:2655
>  vfs_ioctl fs/ioctl.c:46 [inline]
>  file_ioctl fs/ioctl.c:501 [inline]
>  do_vfs_ioctl+0x1bc/0x1618 fs/ioctl.c:685
>  ksys_ioctl+0xbc/0x108 fs/ioctl.c:702
>  __do_sys_ioctl fs/ioctl.c:709 [inline]
>  __se_sys_ioctl fs/ioctl.c:707 [inline]
>  __arm64_sys_ioctl+0x6c/0xa0 fs/ioctl.c:707
>  __invoke_syscall arch/arm64/kernel/syscall.c:36 [inline]
>  invoke_syscall arch/arm64/kernel/syscall.c:48 [inline]
>  el0_svc_common+0x150/0x288 arch/arm64/kernel/syscall.c:84
>  el0_svc_handler+0x54/0xf0 arch/arm64/kernel/syscall.c:130
>  el0_svc+0x8/0xc arch/arm64/kernel/entry.S:917
> ==
> 
> 
> Syzkaller reproducer:
> # {Threaded:true Collide:true Repeat:true RepeatTimes:0 Procs:1
> Sandbox:none Fault:false FaultCall:-1 FaultNth:0 EnableTun:true
> UseTmpDir:true EnableCgroups:true EnableNetdev:true ResetNet:true
> HandleSegv:true Repro:false Trace:false}
> r0 = openat$ptmx(0xff9c,
> &(0x7f00)='/dev/ptmx\x00', 0x0, 0x0)
> ioctl$TIOCGPTPEER(r0, 0x40045431, 0x6e)
> r1 = syz_open_pts(r0, 0x0)
> ioctl$TCXONC(r1, 0x5437, 0x0)
> ioctl$TIOCGSOFTCAR(r0, 0x5419, &(0x7fc0))
> r2 = semget(0x0, 0x1, 0x1a)
> semctl$IPC_INFO(r2, 0x0, 0x3, &(0x7f000100)=""/166)
> syz_open_pts(r0, 0x2)
> ioctl$TCSETAW(r0, 0x5407, &(0x7f80))
> 

-- 
Thanks,
 Dmitry


Re: [PATCHv3 0/6] tty: Hold write ldisc sem in tty_reopen()

2018-09-11 Thread Mark Rutland
On Tue, Sep 11, 2018 at 02:48:15AM +0100, Dmitry Safonov wrote:
> Hi all,

Hi,

> Three fixes that worth to have in the @stable, as we've hit them on v4.9
> stable.
> 
> And for linux-next - adding lockdep asserts for line discipline changing
> code, verifying that write ldisc sem will be held forthwith.
> 
> The last patch is optional and probably, timeout can be dropped for
> read_lock(). I'll do it if everyone agrees.
> 
> Rong Chen, could you kindly re-run this version to see if the lockup
> from v1 still happens? I wasn't able to reproduce it..

These patches seem to fix issues I've been seeing on arm64 for a while
but hadn't managed to track down.

For patches 1, 3, and 5, feel free to add:

Tested-by: Mark Rutland 

On vanilla v4.19-rc2, the below reproducer would fire in seconds,
whereas with those patches applied, I have not seen issues after 10s of
minutes of testing.

Thanks,
Mark.

Syzkaller hit 'KASAN: user-memory-access Write in n_tty_set_termios' bug.

IPv6: ADDRCONF(NETDEV_UP): veth0: link is not ready
ipV6: ADDRCONF(NETDEV_UP): veth1: link is not ready
IPv6: ADDRCONF(NETDEV_CHANGE): veth1: link becomes ready
IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
==
BUG: KASAN: user-memory-access in memset include/linux/string.h:330 [inline]
BUG: KASAN: user-memory-access in bitmap_zero include/linux/bitmap.h:216 
[inline]
BUG: KASAN: user-memory-access in n_tty_set_termios+0xe4/0xd08 
drivers/tty/n_tty.c:1784
Write of size 512 at addr 1060 by task syz-executor0/3007

CPU: 1 PID: 3007 Comm: syz-executor0 Not tainted 4.19.0-rc2-dirty #4
Hardware name: linux,dummy-virt (DT)
Call trace:
 dump_backtrace+0x0/0x340 arch/arm64/include/asm/ptrace.h:270
 show_stack+0x20/0x30 arch/arm64/kernel/traps.c:152
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0xec/0x150 lib/dump_stack.c:113
 kasan_report_error mm/kasan/report.c:352 [inline]
 kasan_report+0x228/0x360 mm/kasan/report.c:412
 check_memory_region_inline mm/kasan/kasan.c:253 [inline]
 check_memory_region+0x114/0x1c8 mm/kasan/kasan.c:267
 memset+0x2c/0x50 mm/kasan/kasan.c:285
 memset include/linux/string.h:330 [inline]
 bitmap_zero include/linux/bitmap.h:216 [inline]
 n_tty_set_termios+0xe4/0xd08 drivers/tty/n_tty.c:1784
 tty_set_termios+0x538/0x760 drivers/tty/tty_ioctl.c:341
 set_termios+0x348/0x968 drivers/tty/tty_ioctl.c:414
 tty_mode_ioctl+0x8f0/0xc60 drivers/tty/tty_ioctl.c:779
 n_tty_ioctl_helper+0x6c/0x390 drivers/tty/tty_ioctl.c:940
 n_tty_ioctl+0x6c/0x490 drivers/tty/n_tty.c:2450
 tty_ioctl+0x610/0x19a8 drivers/tty/tty_io.c:2655
 vfs_ioctl fs/ioctl.c:46 [inline]
 file_ioctl fs/ioctl.c:501 [inline]
 do_vfs_ioctl+0x1bc/0x1618 fs/ioctl.c:685
 ksys_ioctl+0xbc/0x108 fs/ioctl.c:702
 __do_sys_ioctl fs/ioctl.c:709 [inline]
 __se_sys_ioctl fs/ioctl.c:707 [inline]
 __arm64_sys_ioctl+0x6c/0xa0 fs/ioctl.c:707
 __invoke_syscall arch/arm64/kernel/syscall.c:36 [inline]
 invoke_syscall arch/arm64/kernel/syscall.c:48 [inline]
 el0_svc_common+0x150/0x288 arch/arm64/kernel/syscall.c:84
 el0_svc_handler+0x54/0xf0 arch/arm64/kernel/syscall.c:130
 el0_svc+0x8/0xc arch/arm64/kernel/entry.S:917
==


Syzkaller reproducer:
# {Threaded:true Collide:true Repeat:true RepeatTimes:0 Procs:1 Sandbox:none 
Fault:false FaultCall:-1 FaultNth:0 EnableTun:true UseTmpDir:true 
EnableCgroups:true EnableNetdev:true ResetNet:true HandleSegv:true Repro:false 
Trace:false}
r0 = openat$ptmx(0xff9c, &(0x7f00)='/dev/ptmx\x00', 0x0, 
0x0)
ioctl$TIOCGPTPEER(r0, 0x40045431, 0x6e)
r1 = syz_open_pts(r0, 0x0)
ioctl$TCXONC(r1, 0x5437, 0x0)
ioctl$TIOCGSOFTCAR(r0, 0x5419, &(0x7fc0))
r2 = semget(0x0, 0x1, 0x1a)
semctl$IPC_INFO(r2, 0x0, 0x3, &(0x7f000100)=""/166)
syz_open_pts(r0, 0x2)
ioctl$TCSETAW(r0, 0x5407, &(0x7f80))



[PATCHv3 0/6] tty: Hold write ldisc sem in tty_reopen()

2018-09-10 Thread Dmitry Safonov
Hi all,

Three fixes that worth to have in the @stable, as we've hit them on v4.9
stable.

And for linux-next - adding lockdep asserts for line discipline changing
code, verifying that write ldisc sem will be held forthwith.

The last patch is optional and probably, timeout can be dropped for
read_lock(). I'll do it if everyone agrees.

Rong Chen, could you kindly re-run this version to see if the lockup
from v1 still happens? I wasn't able to reproduce it..

Thanks,
Dima

Changes since v2:
- Added reviewed-by tags
- Hopefully, fixed reported by 0-day issue.
- Added optional fix for wait_readers decrement

Changes since v1:
- Added tested-by/reported-by tags
- Dropped 3/4 (locking tty pair for lockdep sake),
  Because of that - not adding lockdep_assert_held() in tty_ldisc_open()
- Added 4/4 cleanup to inc tty->count only on success of
  tty_ldisc_reinit()
- lock ldisc without (5*HZ) timeout in tty_reopen()

v1 link: 
lkml.kernel.org/r/<20180829022353.23568-1-d...@arista.com>

Huuge cc list:
Cc: Daniel Axtens 
Cc: Dmitry Vyukov 
Cc: Michael Neuling 
Cc: Mikulas Patocka 
Cc: Nathan March 
Cc: Pasi Kärkkäinen 
Cc: Peter Hurley 
Cc: "Rong, Chen" 
Cc: Sergey Senozhatsky 
Cc: Tan Xiaojun 
Cc: Tetsuo Handa 
(please, ignore if I Cc'ed you mistakenly)

Dmitry Safonov (6):
  tty: Drop tty->count on tty_reopen() failure
  tty/ldsem: Update waiter->task before waking up reader
  tty: Hold tty_ldisc_lock() during tty_reopen()
  tty/lockdep: Add ldisc_sem asserts
  tty: Simplify tty->count math in tty_reopen()
  tty/ldsem: Decrement wait_readers on timeouted down_read()

 drivers/tty/tty_io.c| 12 
 drivers/tty/tty_ldisc.c |  5 +
 drivers/tty/tty_ldsem.c |  5 -
 3 files changed, 17 insertions(+), 5 deletions(-)

-- 
2.13.6