Messages in this thread | | | Date | Thu, 19 Apr 2018 16:07:51 +0200 | From | Greg KH <> | Subject | Re: KASAN: slab-out-of-bounds Write in tty_insert_flip_string_fixed_flag |
| |
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
| |