linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* KASAN: slab-out-of-bounds Write in tty_insert_flip_string_fixed_flag
@ 2018-04-19  8:09 DaeRyong Jeong
  2018-04-19  8:17 ` Greg KH
  0 siblings, 1 reply; 6+ messages in thread
From: DaeRyong Jeong @ 2018-04-19  8:09 UTC (permalink / raw)
  To: Byoungyoung Lee, Kyungtae Kim, LKML, gregkh

We report the crash:
KASAN: slab-out-of-bounds Write in tty_insert_flip_string_fixed_flag

This crash has been found in v4.16 using RaceFuzzer (a modified
version of Syzkaller), which we describe more at the end of this
report. Our analysis shows that the race occurs when invoking two
syscalls concurrently, ioctl$TCXONC(r0, 0x540a, 0x2) and
ioctl$TCXONC(r0, 0x540a, 0x1).

C repro code:
https://kiwi.cs.purdue.edu/static/race-fuzzer/tty_insert_flip_string_fixed_flag.c
kernel config v4.16:
https://kiwi.cs.purdue.edu/static/race-fuzzer/tty_insert_flip_string_fixed_flag.config

Analysis:
We think the concurrent execution of tty_insert_filp_string_fixed_flag
causes the crash.
Unsynchronized copying data to a buffer and updating an offset may
cause crashes slab-out-of-bounds write in
tty_insert_flip_string_fixed_flag or slab-out-of-bounds read in
flush_to_ldisc.

Call Sequence (v4.16):
CPU0 (ioctl$TCXONC(r0, 0x540a, 0x1))
n_tty_ioctl_helper
  __start_tty
    tty_wakeup
      n_hdlc_tty_wakeup
        n_hdlc_send_frames
          pty_write
            tty_insert_flip_string
              tty_insert_flip_string_fixed_flag

CPU1 (ioctl$TCXONC(r0, 0x540a, 0x2))
n_tty_ioctl_helper
  tty_send_xchar
    pty_write
      tty_insert_flip_string
        tty_insert_flip_string_fixed_flag

Crash log:
==================================================================
[  149.605804] BUG: KASAN: slab-out-of-bounds in
tty_insert_flip_string_fixed_flag+0xaf/0x130
[  149.606914] Write of size 1 at addr ffff8800b1373ae0 by task
a.out/3899
[  149.607801]
[  149.608033] CPU: 0 PID: 3899 Comm: a.out Not tainted 4.16.0 #1
[  149.608852] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
[  149.610040] Call Trace:
[  149.610379]  dump_stack+0x155/0x1f6
[  149.610847]  ? arch_local_irq_restore+0x3c/0x3c
[  149.611438]  ? show_regs_print_info+0x18/0x18
[  149.612012]  ? flush_to_ldisc+0x310/0x310
[  149.612548]  ? tty_insert_flip_string_fixed_flag+0xaf/0x130
[  149.613273]  print_address_description+0x73/0x250
[  149.613889]  ? tty_insert_flip_string_fixed_flag+0xaf/0x130
[  149.614609]  kasan_report+0x23f/0x360
[  149.615106]  check_memory_region+0x140/0x1a0
[  149.615669]  memcpy+0x37/0x50
[  149.616076]  tty_insert_flip_string_fixed_flag+0xaf/0x130
[  149.616807]  pty_write+0x7f/0xc0
[  149.617248]  tty_send_xchar+0x114/0x180
[  149.617765]  n_tty_ioctl_helper+0xec/0x1e0
[  149.618312]  n_hdlc_tty_ioctl+0x118/0x370
[  149.618848]  ? n_hdlc_tty_wakeup+0xa0/0xa0
[  149.619410]  ? ldsem_down_read+0x37/0x40
[  149.619923]  ? ldsem_down_read+0x37/0x40
[  149.620474]  tty_ioctl+0x292/0x1030
[  149.620914]  ? n_hdlc_tty_wakeup+0xa0/0xa0
[  149.621431]  ? tty_vhangup+0x30/0x30
[  149.621883]  ? rcu_is_watching+0x8f/0xd0
[  149.622378]  ? rcutorture_record_progress+0x10/0x10
[  149.622998]  ? __lock_is_held+0x30/0xc0
[  149.623505]  ? ___might_sleep+0x258/0x340
[  149.624010]  ? check_same_owner+0x240/0x240
[  149.624535]  ? __vfs_write+0xe5/0x480
[  149.624997]  ? rcu_note_context_switch+0x4e0/0x4e0
[  149.625594]  ? rcu_note_context_switch+0x4e0/0x4e0
[  149.626186]  ? kernel_read+0xa0/0xa0
[  149.626648]  ? tty_vhangup+0x30/0x30
[  149.627099]  do_vfs_ioctl+0x145/0xd00
[  149.627557]  ? _cond_resched+0x14/0x30
[  149.628039]  ? ioctl_preallocate+0x1c0/0x1c0
[  149.628583]  ? selinux_capable+0x40/0x40
[  149.629087]  ? SyS_futex+0x22b/0x2f0
[  149.629562]  ? security_file_ioctl+0x62/0x90
[  149.630094]  ? security_file_ioctl+0x6e/0x90
[  149.630633]  SyS_ioctl+0x8f/0xc0
[  149.631049]  ? do_vfs_ioctl+0xd00/0xd00
[  149.631534]  do_syscall_64+0x1f2/0x530
[  149.632017]  ? syscall_return_slowpath+0x360/0x360
[  149.632632]  ? syscall_return_slowpath+0x1fd/0x360
[  149.633204]  ? mark_held_locks+0x23/0xa0
[  149.633675]  ? entry_SYSCALL_64_after_hwframe+0x52/0xb7
[  149.634295]  ? trace_hardirqs_off_caller+0xb5/0x120
[  149.634874]  ? trace_hardirqs_off_thunk+0x1a/0x1c
[  149.635444]  entry_SYSCALL_64_after_hwframe+0x42/0xb7
[  149.636051] RIP: 0033:0x7f4079e59b79
[  149.636493] RSP: 002b:00007f4079d7ddd8 EFLAGS: 00000246 ORIG_RAX:
0000000000000010
[  149.637377] RAX: ffffffffffffffda RBX: 0000000000000001 RCX:
00007f4079e59b79
[  149.638194] RDX: 0000000000000002 RSI: 000000000000540a RDI:
0000000000000062
[  149.639014] RBP: 00007f4079d7de20 R08: 0000000000000000 R09:
0000000000000000
[  149.639839] R10: 0000000000000000 R11: 0000000000000246 R12:
000000000072fef0
[  149.640675] R13: 00007f4079d7e9c0 R14: 00007f407a548040 R15:
0000000000000003
[  149.641529]
[  149.641722] Allocated by task 3899:
[  149.642140]  save_stack+0x43/0xd0
[  149.642539]  kasan_kmalloc+0xae/0xe0
[  149.642968]  __kmalloc+0x162/0x760
[  149.643376]  __tty_buffer_request_room+0x1cb/0x400
[  149.643934]  tty_insert_flip_string_fixed_flag+0x67/0x130
[  149.644585]  pty_write+0x7f/0xc0
[  149.644959]  n_hdlc_send_frames+0x1a4/0x340
[  149.645432]  n_hdlc_tty_write+0x3f3/0x480
[  149.645887]  tty_write+0x320/0x510
[  149.646275]  __vfs_write+0xdd/0x480
[  149.646672]  vfs_write+0x12d/0x2d0
[  149.647059]  SyS_write+0xca/0x190
[  149.647438]  do_syscall_64+0x1f2/0x530
[  149.647863]  entry_SYSCALL_64_after_hwframe+0x42/0xb7
[  149.648434]
[  149.648624] Freed by task 0:
[  149.648937] (stack is not available)
[  149.649322]
[  149.649501] The buggy address belongs to the object at
ffff8800b1372cc0
[  149.649501]  which belongs to the cache kmalloc-4096 of size 4096
[  149.650820] The buggy address is located 3616 bytes inside of
[  149.650820]  4096-byte region [ffff8800b1372cc0, ffff8800b1373cc0)
[  149.652054] The buggy address belongs to the page:
[  149.652581] page:ffffea0002c4dc80 count:1 mapcount:0
mapping:ffff8800b1372cc0 index:0x0 compound_mapcount: 0
[  149.653629] flags: 0x1fffc0000008100(slab|head)
[  149.654122] raw: 01fffc0000008100 ffff8800b1372cc0 0000000000000000
0000000100000001
[  149.654941] raw: ffffea0002cb1c20 ffff8800b4801a48 ffff8800b4800dc0
0000000000000000
[  149.655748] page dumped because: kasan: bad access detected
[  149.656346]
[  149.656520] Memory state around the buggy address:
[  149.657031]  ffff8800b1373980: 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00
[  149.657805]  ffff8800b1373a00: 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00
[  149.658563] >ffff8800b1373a80: 00 00 00 00 00 00 00 00 00 00 00 00
fc fc fc fc
[  149.659320]
^
[  149.659989]  ffff8800b1373b00: fc fc fc fc fc fc fc fc fc fc fc fc
fc fc fc fc
[  149.660759]  ffff8800b1373b80: fc fc fc fc fc fc fc fc fc fc fc fc
fc fc fc fc
[  149.661535]
==================================================================
[  149.662297] Disabling lock debugging due to kernel taint



= About RaceFuzzer

RaceFuzzer is a customized version of Syzkaller, specifically tailored
to find race condition bugs in the Linux kernel. While we leverage
many different technique, the notable feature of RaceFuzzer is in
leveraging a custom hypervisor (QEMU/KVM) to interleave the
scheduling. In particular, we modified the hypervisor to intentionally
stall a per-core execution, which is similar to supporting per-core
breakpoint functionality. This allows RaceFuzzer to force the kernel
to deterministically trigger racy condition (which may rarely happen
in practice due to randomness in scheduling).

RaceFuzzer's C repro always pinpoints two racy syscalls. Since C
repro's scheduling synchronization should be performed at the user
space, its reproducibility is limited (reproduction may take from 1
second to 10 minutes (or even more), depending on a bug). This is
because, while RaceFuzzer precisely interleaves the scheduling at the
kernel's instruction level when finding this bug, C repro cannot fully
utilize such a feature. Please disregard all code related to
"should_hypercall" in the C repro, as this is only for our debugging
purposes using our own hypervisor.

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

* Re: KASAN: slab-out-of-bounds Write in tty_insert_flip_string_fixed_flag
  2018-04-19  8:09 KASAN: slab-out-of-bounds Write in tty_insert_flip_string_fixed_flag DaeRyong Jeong
@ 2018-04-19  8:17 ` Greg KH
  2018-04-19 12:25   ` DaeRyong Jeong
  0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2018-04-19  8:17 UTC (permalink / raw)
  To: DaeRyong Jeong; +Cc: Byoungyoung Lee, Kyungtae Kim, LKML

On Thu, Apr 19, 2018 at 05:09:16PM +0900, DaeRyong Jeong wrote:
> We report the crash:
> KASAN: slab-out-of-bounds Write in tty_insert_flip_string_fixed_flag
> 
> This crash has been found in v4.16 using RaceFuzzer (a modified
> version of Syzkaller), which we describe more at the end of this
> report. Our analysis shows that the race occurs when invoking two
> syscalls concurrently, ioctl$TCXONC(r0, 0x540a, 0x2) and
> ioctl$TCXONC(r0, 0x540a, 0x1).

Nice!

Do you have a kernel patch to resolve this issue?

thanks,

greg k-h

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

* Re: KASAN: slab-out-of-bounds Write in tty_insert_flip_string_fixed_flag
  2018-04-19  8:17 ` Greg KH
@ 2018-04-19 12:25   ` DaeRyong Jeong
  2018-04-19 14:07     ` Greg KH
  2018-04-25 12:53     ` Greg KH
  0 siblings, 2 replies; 6+ messages in thread
From: DaeRyong Jeong @ 2018-04-19 12:25 UTC (permalink / raw)
  To: Greg KH; +Cc: Byoungyoung Lee, Kyungtae Kim, LKML, basavesh

The patch is attached at the end of this email and can be downloaded from here.
https://kiwi.cs.purdue.edu/static/race-fuzzer/tty_insert_flip_string_fixed_flag.patch

We applied the patch to v4.16 and tested our reproducer. we can't see the
crash any longer.

Our rationale is
  - Since tty_wakeup() called by __start_tty() sends frames, call
    tty_write_lock() before __start_tty().
  - Since tty_write_lock() might sleep, locate tty_write_lock() before
    spin_lock_irq(&tty->flow_lock).
  - Since wake_up_interruptible_poll() is called by both tty_write_unlock()
    and __start_tty(), Prevent calling wake_up_interruptible_poll() twice by
    adding the wakeup flag to tty_write_unlock()

If there is something that we are missing or we are wrong, please let us know.


Thank you.

Best regards,
Daeryong Jeong



diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 63114ea..09c76d3 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -873,13 +873,15 @@ static ssize_t tty_read(struct file *file, char
__user *buf, size_t count,
  return i;
 }

-static void tty_write_unlock(struct tty_struct *tty)
+void tty_write_unlock(struct tty_struct *tty, int wakeup)
 {
  mutex_unlock(&tty->atomic_write_lock);
- wake_up_interruptible_poll(&tty->write_wait, EPOLLOUT);
+ if (wakeup) {
+ wake_up_interruptible_poll(&tty->write_wait, EPOLLOUT);
+ }
 }

-static int tty_write_lock(struct tty_struct *tty, int ndelay)
+int tty_write_lock(struct tty_struct *tty, int ndelay)
 {
  if (!mutex_trylock(&tty->atomic_write_lock)) {
  if (ndelay)
@@ -973,7 +975,7 @@ static inline ssize_t do_tty_write(
  ret = written;
  }
 out:
- tty_write_unlock(tty);
+ tty_write_unlock(tty, 1);
  return ret;
 }

@@ -997,7 +999,7 @@ void tty_write_message(struct tty_struct *tty, char *msg)
  if (tty->ops->write && tty->count > 0)
  tty->ops->write(tty, msg, strlen(msg));
  tty_unlock(tty);
- tty_write_unlock(tty);
+ tty_write_unlock(tty, 1);
  }
  return;
 }
@@ -1092,7 +1094,7 @@ int tty_send_xchar(struct tty_struct *tty, char ch)
  if (was_stopped)
  stop_tty(tty);
  up_read(&tty->termios_rwsem);
- tty_write_unlock(tty);
+ tty_write_unlock(tty, 1);
  return 0;
 }

@@ -2395,7 +2397,7 @@ static int send_break(struct tty_struct *tty,
unsigned int duration)
  msleep_interruptible(duration);
  retval = tty->ops->break_ctl(tty, 0);
 out:
- tty_write_unlock(tty);
+ tty_write_unlock(tty, 1);
  if (signal_pending(current))
  retval = -EINTR;
  }
diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
index d9b561d..f331e99 100644
--- a/drivers/tty/tty_ioctl.c
+++ b/drivers/tty/tty_ioctl.c
@@ -911,12 +911,17 @@ int n_tty_ioctl_helper(struct tty_struct *tty,
struct file *file,
  spin_unlock_irq(&tty->flow_lock);
  break;
  case TCOON:
+ if (tty_write_lock(tty, 0) < 0)
+ return -ERESTARTSYS;
+
  spin_lock_irq(&tty->flow_lock);
  if (tty->flow_stopped) {
  tty->flow_stopped = 0;
  __start_tty(tty);
  }
  spin_unlock_irq(&tty->flow_lock);
+
+ tty_write_unlock(tty, 0);
  break;
  case TCIOFF:
  if (STOP_CHAR(tty) != __DISABLED_CHAR)
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 47f8af2..5bdf928 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -458,6 +458,8 @@ static inline struct tty_struct
*tty_kref_get(struct tty_struct *tty)
  return tty;
 }

+extern void tty_write_unlock(struct tty_struct *tty, int wakeup);
+extern int tty_write_lock(struct tty_struct *tty, int ndelay);
 extern const char *tty_driver_name(const struct tty_struct *tty);
 extern void tty_wait_until_sent(struct tty_struct *tty, long timeout);
 extern int __tty_check_change(struct tty_struct *tty, int sig);

On Thu, Apr 19, 2018 at 5:17 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, Apr 19, 2018 at 05:09:16PM +0900, DaeRyong Jeong wrote:
>> We report the crash:
>> KASAN: slab-out-of-bounds Write in tty_insert_flip_string_fixed_flag
>>
>> This crash has been found in v4.16 using RaceFuzzer (a modified
>> version of Syzkaller), which we describe more at the end of this
>> report. Our analysis shows that the race occurs when invoking two
>> syscalls concurrently, ioctl$TCXONC(r0, 0x540a, 0x2) and
>> ioctl$TCXONC(r0, 0x540a, 0x1).
>
> Nice!
>
> Do you have a kernel patch to resolve this issue?
>
> thanks,
>
> greg k-h

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

* Re: KASAN: slab-out-of-bounds Write in tty_insert_flip_string_fixed_flag
  2018-04-19 12:25   ` DaeRyong Jeong
@ 2018-04-19 14:07     ` Greg KH
  2018-04-25 12:53     ` Greg KH
  1 sibling, 0 replies; 6+ messages in thread
From: Greg KH @ 2018-04-19 14:07 UTC (permalink / raw)
  To: DaeRyong Jeong; +Cc: Byoungyoung Lee, Kyungtae Kim, LKML, basavesh

On Thu, Apr 19, 2018 at 09:25:08PM +0900, DaeRyong Jeong wrote:
> The patch is attached at the end of this email and can be downloaded from here.
> https://kiwi.cs.purdue.edu/static/race-fuzzer/tty_insert_flip_string_fixed_flag.patch
> 
> We applied the patch to v4.16 and tested our reproducer. we can't see the
> crash any longer.
> 
> Our rationale is
>   - Since tty_wakeup() called by __start_tty() sends frames, call
>     tty_write_lock() before __start_tty().
>   - Since tty_write_lock() might sleep, locate tty_write_lock() before
>     spin_lock_irq(&tty->flow_lock).
>   - Since wake_up_interruptible_poll() is called by both tty_write_unlock()
>     and __start_tty(), Prevent calling wake_up_interruptible_poll() twice by
>     adding the wakeup flag to tty_write_unlock()
> 
> If there is something that we are missing or we are wrong, please let us know.
> 
> 
> Thank you.
> 
> Best regards,
> Daeryong Jeong
> 
> 
> 
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index 63114ea..09c76d3 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -873,13 +873,15 @@ static ssize_t tty_read(struct file *file, char
> __user *buf, size_t count,
>   return i;
>  }
> 
> -static void tty_write_unlock(struct tty_struct *tty)
> +void tty_write_unlock(struct tty_struct *tty, int wakeup)
>  {
>   mutex_unlock(&tty->atomic_write_lock);
> - wake_up_interruptible_poll(&tty->write_wait, EPOLLOUT);
> + if (wakeup) {
> + wake_up_interruptible_poll(&tty->write_wait, EPOLLOUT);
> + }
>  }
> 
> -static int tty_write_lock(struct tty_struct *tty, int ndelay)
> +int tty_write_lock(struct tty_struct *tty, int ndelay)
>  {
>   if (!mutex_trylock(&tty->atomic_write_lock)) {
>   if (ndelay)
> @@ -973,7 +975,7 @@ static inline ssize_t do_tty_write(
>   ret = written;
>   }
>  out:
> - tty_write_unlock(tty);
> + tty_write_unlock(tty, 1);
>   return ret;
>  }
> 
> @@ -997,7 +999,7 @@ void tty_write_message(struct tty_struct *tty, char *msg)
>   if (tty->ops->write && tty->count > 0)
>   tty->ops->write(tty, msg, strlen(msg));
>   tty_unlock(tty);
> - tty_write_unlock(tty);
> + tty_write_unlock(tty, 1);
>   }
>   return;
>  }
> @@ -1092,7 +1094,7 @@ int tty_send_xchar(struct tty_struct *tty, char ch)
>   if (was_stopped)
>   stop_tty(tty);
>   up_read(&tty->termios_rwsem);
> - tty_write_unlock(tty);
> + tty_write_unlock(tty, 1);
>   return 0;
>  }
> 
> @@ -2395,7 +2397,7 @@ static int send_break(struct tty_struct *tty,
> unsigned int duration)
>   msleep_interruptible(duration);
>   retval = tty->ops->break_ctl(tty, 0);
>  out:
> - tty_write_unlock(tty);
> + tty_write_unlock(tty, 1);
>   if (signal_pending(current))
>   retval = -EINTR;
>   }
> diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c
> index d9b561d..f331e99 100644
> --- a/drivers/tty/tty_ioctl.c
> +++ b/drivers/tty/tty_ioctl.c
> @@ -911,12 +911,17 @@ int n_tty_ioctl_helper(struct tty_struct *tty,
> struct file *file,
>   spin_unlock_irq(&tty->flow_lock);
>   break;
>   case TCOON:
> + if (tty_write_lock(tty, 0) < 0)
> + return -ERESTARTSYS;
> +
>   spin_lock_irq(&tty->flow_lock);
>   if (tty->flow_stopped) {
>   tty->flow_stopped = 0;
>   __start_tty(tty);
>   }
>   spin_unlock_irq(&tty->flow_lock);
> +
> + tty_write_unlock(tty, 0);
>   break;
>   case TCIOFF:
>   if (STOP_CHAR(tty) != __DISABLED_CHAR)
> diff --git a/include/linux/tty.h b/include/linux/tty.h
> index 47f8af2..5bdf928 100644
> --- a/include/linux/tty.h
> +++ b/include/linux/tty.h
> @@ -458,6 +458,8 @@ static inline struct tty_struct
> *tty_kref_get(struct tty_struct *tty)
>   return tty;
>  }
> 
> +extern void tty_write_unlock(struct tty_struct *tty, int wakeup);
> +extern int tty_write_lock(struct tty_struct *tty, int ndelay);
>  extern const char *tty_driver_name(const struct tty_struct *tty);
>  extern void tty_wait_until_sent(struct tty_struct *tty, long timeout);
>  extern int __tty_check_change(struct tty_struct *tty, int sig);
> 

Can you please submit this in a form that I can properly review it and
apply it if needed?  Documentation/SubmittingPatches has all of the
details.

thanks,

greg k-h

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

* Re: KASAN: slab-out-of-bounds Write in tty_insert_flip_string_fixed_flag
  2018-04-19 12:25   ` DaeRyong Jeong
  2018-04-19 14:07     ` Greg KH
@ 2018-04-25 12:53     ` Greg KH
  2018-04-25 13:02       ` DaeRyong Jeong
  1 sibling, 1 reply; 6+ messages in thread
From: Greg KH @ 2018-04-25 12:53 UTC (permalink / raw)
  To: DaeRyong Jeong; +Cc: Byoungyoung Lee, Kyungtae Kim, LKML, basavesh

On Thu, Apr 19, 2018 at 09:25:08PM +0900, DaeRyong Jeong wrote:
> The patch is attached at the end of this email and can be downloaded from here.
> https://kiwi.cs.purdue.edu/static/race-fuzzer/tty_insert_flip_string_fixed_flag.patch
> 
> We applied the patch to v4.16 and tested our reproducer. we can't see the
> crash any longer.
> 
> Our rationale is
>   - Since tty_wakeup() called by __start_tty() sends frames, call
>     tty_write_lock() before __start_tty().
>   - Since tty_write_lock() might sleep, locate tty_write_lock() before
>     spin_lock_irq(&tty->flow_lock).
>   - Since wake_up_interruptible_poll() is called by both tty_write_unlock()
>     and __start_tty(), Prevent calling wake_up_interruptible_poll() twice by
>     adding the wakeup flag to tty_write_unlock()
> 
> If there is something that we are missing or we are wrong, please let us know.
> 
> 
> Thank you.
> 
> Best regards,
> Daeryong Jeong
> 
> 
> 
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index 63114ea..09c76d3 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -873,13 +873,15 @@ static ssize_t tty_read(struct file *file, char
> __user *buf, size_t count,
>   return i;
>  }
> 
> -static void tty_write_unlock(struct tty_struct *tty)
> +void tty_write_unlock(struct tty_struct *tty, int wakeup)
>  {
>   mutex_unlock(&tty->atomic_write_lock);
> - wake_up_interruptible_poll(&tty->write_wait, EPOLLOUT);
> + if (wakeup) {
> + wake_up_interruptible_poll(&tty->write_wait, EPOLLOUT);
> + }
>  }
> 

<snip>

What ever happened to this patch, did you end up resending it in a
non-corrupted way?

thanks,

greg k-h

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

* Re: KASAN: slab-out-of-bounds Write in tty_insert_flip_string_fixed_flag
  2018-04-25 12:53     ` Greg KH
@ 2018-04-25 13:02       ` DaeRyong Jeong
  0 siblings, 0 replies; 6+ messages in thread
From: DaeRyong Jeong @ 2018-04-25 13:02 UTC (permalink / raw)
  To: Greg KH; +Cc: Byoungyoung Lee, Kyungtae Kim, LKML, basavesh

I'm really sorry for you to wait.
Since I made a few mistakes previously, I really don't want to make a
mistake again.
Therefore I have been reading documents more carefully and learning
the approved email client (i.e., mutt).
It is almost done. I will send a patch very soon.

Again, I'm really sorry.

Best regards,
Daeryong Jeong.

On Wed, Apr 25, 2018 at 9:53 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, Apr 19, 2018 at 09:25:08PM +0900, DaeRyong Jeong wrote:
>> The patch is attached at the end of this email and can be downloaded from here.
>> https://kiwi.cs.purdue.edu/static/race-fuzzer/tty_insert_flip_string_fixed_flag.patch
>>
>> We applied the patch to v4.16 and tested our reproducer. we can't see the
>> crash any longer.
>>
>> Our rationale is
>>   - Since tty_wakeup() called by __start_tty() sends frames, call
>>     tty_write_lock() before __start_tty().
>>   - Since tty_write_lock() might sleep, locate tty_write_lock() before
>>     spin_lock_irq(&tty->flow_lock).
>>   - Since wake_up_interruptible_poll() is called by both tty_write_unlock()
>>     and __start_tty(), Prevent calling wake_up_interruptible_poll() twice by
>>     adding the wakeup flag to tty_write_unlock()
>>
>> If there is something that we are missing or we are wrong, please let us know.
>>
>>
>> Thank you.
>>
>> Best regards,
>> Daeryong Jeong
>>
>>
>>
>> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
>> index 63114ea..09c76d3 100644
>> --- a/drivers/tty/tty_io.c
>> +++ b/drivers/tty/tty_io.c
>> @@ -873,13 +873,15 @@ static ssize_t tty_read(struct file *file, char
>> __user *buf, size_t count,
>>   return i;
>>  }
>>
>> -static void tty_write_unlock(struct tty_struct *tty)
>> +void tty_write_unlock(struct tty_struct *tty, int wakeup)
>>  {
>>   mutex_unlock(&tty->atomic_write_lock);
>> - wake_up_interruptible_poll(&tty->write_wait, EPOLLOUT);
>> + if (wakeup) {
>> + wake_up_interruptible_poll(&tty->write_wait, EPOLLOUT);
>> + }
>>  }
>>
>
> <snip>
>
> What ever happened to this patch, did you end up resending it in a
> non-corrupted way?
>
> thanks,
>
> greg k-h

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

end of thread, other threads:[~2018-04-25 13:02 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-19  8:09 KASAN: slab-out-of-bounds Write in tty_insert_flip_string_fixed_flag DaeRyong Jeong
2018-04-19  8:17 ` Greg KH
2018-04-19 12:25   ` DaeRyong Jeong
2018-04-19 14:07     ` Greg KH
2018-04-25 12:53     ` Greg KH
2018-04-25 13:02       ` DaeRyong Jeong

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