[PATCH] tty:tty_ldisc: add tty_ldisc_lock|unlock to prevent concurrent update to ldisc in tty_ldisc_deinit

[PATCH] tty:tty_ldisc: add tty_ldisc_lock|unlock to prevent concurrent update to ldisc in tty_ldisc_deinit

[Wang YanQing]

This patch could fix the issue that free_tty_struct in tty_io
calling tty_ldisc_deinit without holding tty->ldisc_sem.

Signed-off-by: Wang YanQing <udknight@gmail.com>
---
 drivers/tty/tty_ldisc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index b1f7fa5..674421b 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -771,7 +771,9 @@ void tty_ldisc_init(struct tty_struct *tty)
  */
 void tty_ldisc_deinit(struct tty_struct *tty)
 {
+	tty_ldisc_lock(tty, MAX_SCHEDULE_TIMEOUT);
 	if (tty->ldisc)
 		tty_ldisc_put(tty->ldisc);
 	tty->ldisc = NULL;
+	tty_ldisc_unlock(tty);
 }
-- 
1.8.5.6.2.g3d8a54e.dirty

Re: [PATCH] tty:tty_ldisc: add tty_ldisc_lock|unlock to prevent concurrent update to ldisc in tty_ldisc_deinit

[Michael Neuling]

Wang,

Applying this, with the other one on top and it doesn't fix the problem (applied
on next-20170405). I tried each patch by itself, with the same bad result.

Thanks for the help but the backtrace is the same:

Unable to handle kernel paging request for data at address 0x00002260
Faulting instruction address: 0xc000000000568800
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=32 
NUMA 
PowerNV
Modules linked in:
CPU: 6 PID: 177 Comm: kworker/u56:1 Not tainted 4.11.0-rc5-next-20170405-00002-g34d2ff03e6 #9
Workqueue: events_unbound flush_to_ldisc
task: c0000077c498a280 task.stack: c0000077c49f8000
NIP: c000000000568800 LR: c0000000005687e8 CTR: c000000000569310
REGS: c0000077c49fb890 TRAP: 0300   Not tainted  (4.11.0-rc5-next-20170405-00002-g34d2ff03e6)
MSR: 900000000280b033 <SF,HV,VEC,VSX,EE,FP,ME,IR,DR,RI,LE>
  CR: 24042428  XER: 00000000
CFAR: c000000000956adc DAR: 0000000000002260 DSISR: 40000000 SOFTE: 1 
GPR00: c0000000005687e8 c0000077c49fbb10 c000000000f3cb00 c0000077c32710d8 
GPR04: c0000077bf556c20 c0000077bf556d20 0000000000000100 0000000000000001 
GPR08: c0000077c32710d8 c0000077c3271220 c0000077c3271248 c000007995c28508 
GPR12: 0000000084002428 c00000000fff7e00 c0000000000f2e08 c0000077c48c4040 
GPR16: 0000000000000000 0000000000000000 c0000079940102a8 c000007994010078 
GPR20: c000007994010020 0000000000000000 0000000000000000 0000000100000000 
GPR24: 0000000000000000 0000000000000000 c0000077bf556c20 c0000077bf556d20 
GPR28: 0000000000000100 0000000000000100 c0000077bf556d20 c0000077c3271000 
NIP [c000000000568800] n_tty_receive_buf_common+0xb0/0xbc0
LR [c0000000005687e8] n_tty_receive_buf_common+0x98/0xbc0
Call Trace:
[c0000077c49fbb10] [c0000000005687e8] n_tty_receive_buf_common+0x98/0xbc0 (unreliable)
[c0000077c49fbbe0] [c00000000056d02c] tty_ldisc_receive_buf+0x3c/0xd0
[c0000077c49fbc10] [c00000000056dedc] tty_port_default_receive_buf+0x5c/0xe0
[c0000077c49fbc50] [c00000000056d340] flush_to_ldisc+0x110/0x130
[c0000077c49fbca0] [c0000000000ea88c] process_one_work+0x1dc/0x550
[c0000077c49fbd30] [c0000000000eac88] worker_thread+0x88/0x5c0
[c0000077c49fbdc0] [c0000000000f2f60] kthread+0x160/0x1a0
[c0000077c49fbe30] [c00000000000bc60] ret_from_kernel_thread+0x5c/0x7c
Instruction dump:
fba1ffe8 fbc1fff0 f821ff31 f9010030 eb3f0280 483ee2a5 60000000 393f0220 
395f0248 f9210020 f9410028 60420000 <e9192260> 7c2004ac 80ff0130 e8d90000 
---[ end trace b30eea9f71cf8d4a ]---


Thanks for the help
Mikey

On Mon, 2017-04-10 at 00:59 +0800, Wang YanQing wrote:
> This patch could fix the issue that free_tty_struct in tty_io
> calling tty_ldisc_deinit without holding tty->ldisc_sem.
> 
> Signed-off-by: Wang YanQing <udknight@gmail.com>
> ---
>  drivers/tty/tty_ldisc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
> index b1f7fa5..674421b 100644
> --- a/drivers/tty/tty_ldisc.c
> +++ b/drivers/tty/tty_ldisc.c
> @@ -771,7 +771,9 @@ void tty_ldisc_init(struct tty_struct *tty)
>   */
>  void tty_ldisc_deinit(struct tty_struct *tty)
>  {
> +	tty_ldisc_lock(tty, MAX_SCHEDULE_TIMEOUT);
>  	if (tty->ldisc)
>  		tty_ldisc_put(tty->ldisc);
>  	tty->ldisc = NULL;
> +	tty_ldisc_unlock(tty);
>  }