Re: [PATCH] pty: cancel pty slave port buf's work in tty_release

[Greg KH]

On Wed, Dec 13, 2017 at 09:10:48AM +0400, kpark3469@xxxxxxxxx wrote:
> From: Sahara <keun-o.park@xxxxxxxxxxxxx>

I need a "Full" name here, I doubt you sign legal documents with just
the single name, right?

> 
> In case that CONFIG_SLUB_DEBUG is on and pty is used, races between
> release_one_tty and flush_to_ldisc work threads may happen and lead
> to use-after-free condition on tty->link->port. Because SLUB_DEBUG
> is turned on, freed tty->link->port is filled with POISON_FREE value.
> So far without SLUB_DEBUG, port was filled with zero and flush_to_ldisc
> could return without a problem by checking if tty is NULL.
> 
> CPU 0                                 CPU 1
> -----                                 -----
> release_tty                           pty_write
>    cancel_work_sync(tty)                 to = tty->link
>    tty_kref_put(tty->link)               tty_schedule_flip(to->port)
>       << workqueue >>                 ...
>       release_one_tty                 ...
>          pty_cleanup                  ...
>             kfree(tty->link->port)       << workqueue >>
>                                          flush_to_ldisc
>                                             tty = READ_ONCE(port->itty)
>                                             tty is 0x6b6b6b6b6b6b6b6b
>                                             !!PANIC!! access tty->ldisc
> 
>  Unable to handle kernel paging request at virtual address 6b6b6b6b6b6b6b93
>  pgd = ffffffc0eb1c3000
>  [6b6b6b6b6b6b6b93] *pgd=0000000000000000, *pud=0000000000000000
>  ------------[ cut here ]------------
>  Kernel BUG at ffffff800851154c [verbose debug info unavailable]
>  Internal error: Oops - BUG: 96000004 [#1] PREEMPT SMP
>  CPU: 3 PID: 265 Comm: kworker/u8:9 Tainted: G        W 3.18.31-g0a58eeb #1
>  Hardware name: Qualcomm Technologies, Inc. MSM 8996pro v1.1 + PMI8996 Carbide (DT)
>  Workqueue: events_unbound flush_to_ldisc
>  task: ffffffc0ed610ec0 ti: ffffffc0ed624000 task.ti: ffffffc0ed624000
>  PC is at ldsem_down_read_trylock+0x0/0x4c
>  LR is at tty_ldisc_ref+0x24/0x4c
>  pc : [<ffffff800851154c>] lr : [<ffffff800850f6c0>] pstate: 80400145
>  sp : ffffffc0ed627cd0
>  x29: ffffffc0ed627cd0 x28: 0000000000000000
>  x27: ffffff8009e05000 x26: ffffffc0d382cfa0
>  x25: 0000000000000000 x24: ffffff800a012f08
>  x23: 0000000000000000 x22: ffffffc0703fbc88
>  x21: 6b6b6b6b6b6b6b6b x20: 6b6b6b6b6b6b6b93
>  x19: 0000000000000000 x18: 0000000000000001
>  x17: 00e80000f80d6f53 x16: 0000000000000001
>  x15: 0000007f7d826fff x14: 00000000000000a0
>  x13: 0000000000000000 x12: 0000000000000109
>  x11: 0000000000000000 x10: 0000000000000000
>  x9 : ffffffc0ed624000 x8 : ffffffc0ed611580
>  x7 : 0000000000000000 x6 : ffffff800a42e000
>  x5 : 00000000000003fc x4 : 0000000003bd1201
>  x3 : 0000000000000001 x2 : 0000000000000001
>  x1 : ffffff800851004c x0 : 6b6b6b6b6b6b6b93
> 
> Signed-off-by: Sahara <keun-o.park@xxxxxxxxxxxxx>

Same here :)

> ---
>  drivers/tty/tty_io.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index dc60aee..a6ca634 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -1476,6 +1476,8 @@ static void release_tty(struct tty_struct *tty, int idx)
>  	if (tty->link)
>  		tty->link->port->itty = NULL;
>  	tty_buffer_cancel_work(tty->port);
> +	if (tty->link)
> +		tty_buffer_cancel_work(tty->link->port);

Your oops above is from 3.18, which is a _very_ old kernel version.  Are
you sure this isn't already fixed in latest kernel release?

thanks,

greg k-h