* [PATCH] pty: cancel pty slave port buf's work in tty_release
@ 2017-12-13 5:10 kpark3469
2017-12-13 8:23 ` Greg KH
2017-12-13 13:22 ` Alan Cox
0 siblings, 2 replies; 4+ messages in thread
From: kpark3469 @ 2017-12-13 5:10 UTC (permalink / raw)
To: linux-kernel; +Cc: gregkh, jslaby, keun-o.park
From: Sahara <keun-o.park@darkmatter.ae>
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@darkmatter.ae>
---
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);
tty_kref_put(tty->link);
tty_kref_put(tty);
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] pty: cancel pty slave port buf's work in tty_release
2017-12-13 5:10 [PATCH] pty: cancel pty slave port buf's work in tty_release kpark3469
@ 2017-12-13 8:23 ` Greg KH
2017-12-13 12:36 ` Keun-O Park
2017-12-13 13:22 ` Alan Cox
1 sibling, 1 reply; 4+ messages in thread
From: Greg KH @ 2017-12-13 8:23 UTC (permalink / raw)
To: kpark3469; +Cc: linux-kernel, jslaby, keun-o.park
On Wed, Dec 13, 2017 at 09:10:48AM +0400, kpark3469@gmail.com wrote:
> From: Sahara <keun-o.park@darkmatter.ae>
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@darkmatter.ae>
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] pty: cancel pty slave port buf's work in tty_release
2017-12-13 8:23 ` Greg KH
@ 2017-12-13 12:36 ` Keun-O Park
0 siblings, 0 replies; 4+ messages in thread
From: Keun-O Park @ 2017-12-13 12:36 UTC (permalink / raw)
To: Greg KH; +Cc: linux-kernel, jslaby, keun-o.park
On Wed, Dec 13, 2017 at 12:23 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Wed, Dec 13, 2017 at 09:10:48AM +0400, kpark3469@gmail.com wrote:
>> From: Sahara <keun-o.park@darkmatter.ae>
>
> I need a "Full" name here, I doubt you sign legal documents with just
> the single name, right?
For a long time, I have been using this single name as my signature in
my company, ex-company, and open communities.
If you don't mind, I want to use this single name in this life.
>
>>
>> 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@darkmatter.ae>
>
> Same here :)
Same comment. I hope you don't mind this.
>
>> ---
>> 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?
Right. Unfortunately I am not sure if this is fixed in latest kernel
with other newly introduced routines.
But, logically it seems the latest kernel also has the same chance to
meet this issue by reading the code.
And, frankly I thought with your broad insight about tty/pty this
could be easily adopted or abandoned.
If not, then probably need more efforts to reproduce the same issue on
latest kernel by implementing pty test code.
Thanks.
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] pty: cancel pty slave port buf's work in tty_release
2017-12-13 5:10 [PATCH] pty: cancel pty slave port buf's work in tty_release kpark3469
2017-12-13 8:23 ` Greg KH
@ 2017-12-13 13:22 ` Alan Cox
1 sibling, 0 replies; 4+ messages in thread
From: Alan Cox @ 2017-12-13 13:22 UTC (permalink / raw)
To: kpark3469; +Cc: linux-kernel, gregkh, jslaby, keun-o.park
> 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);
>
> tty_kref_put(tty->link);
> tty_kref_put(tty);
This looks correct to me, but I do worry about deadlock cases we may be
adding.
Alan
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-12-13 13:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-13 5:10 [PATCH] pty: cancel pty slave port buf's work in tty_release kpark3469
2017-12-13 8:23 ` Greg KH
2017-12-13 12:36 ` Keun-O Park
2017-12-13 13:22 ` Alan Cox
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox