Messages in this thread Patch in this message | | | From | Yi Yang <> | Subject | [PATCH RFC] tty: tty_jobctrl: fix pid memleak in tty_signal_session_leader() | Date | Mon, 3 Jul 2023 16:03:23 +0800 |
| |
There is a leaked pid in tty.
unreferenced object 0xffff889362619440 (size 112): comm "sudo", pid 3603376, jiffies 4462415649 (age 71614.172s) hex dump (first 32 bytes): 01 00 00 00 00 00 00 00 00 00 00 00 0f 00 40 da ..............@. 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ backtrace: [<00000000fd13ed06>] alloc_pid+0x85/0x6b0 [<000000007c449cf0>] copy_process+0xf60/0x2840 [<000000008c3ae147>] kernel_clone+0x11a/0x510 [<000000005d9b1265>] __se_sys_clone+0xcd/0x110 [<000000009d4d672e>] do_syscall_64+0x33/0x40 [<000000002fc0b8b9>] entry_SYSCALL_64_after_hwframe+0x61/0xc6
Race condition between disassociate_ctty() and tty_signal_session_leader() was found, which would cause a leakage of tty_old_pgrp. The race condition is described as follows:
CPU1: CPU2: disassociate_ctty() { ... spin_lock_irq(¤t->sighand->siglock); put_pid(current->signal->tty_old_pgrp); current->signal->tty_old_pgrp = NULL; tty = tty_kref_get(current->signal->tty); spin_unlock_irq(¤t->sighand->siglock);
tty_signal_session_leader() { spin_lock_irq(&p->sighand->siglock); ... spin_lock(&tty->ctrl_lock); tty_pgrp = get_pid(tty->pgrp); if (tty->pgrp) An extra get>> p->signal->tty_old_pgrp = get_pid(tty->pgrp); spin_unlock(&tty->ctrl_lock); spin_unlock_irq(&p->sighand->siglock); }
if (tty) { tty_lock(tty); spin_lock_irqsave(&tty->ctrl_lock, flags); ... tty->pgrp = NULL; spin_unlock_irqrestore(&tty->ctrl_lock, flags); tty_unlock(tty); tty_kref_put(tty); } }
The issue is believed to be introduced by commit c8bcd9c5be24 ("tty: Fix ->session locking") who moves the unlock of siglock in disassociate_ctty() above "if (tty)", making a small window allowing tty_signal_session_leader()to kick in. It can be easily reproduced by adding a delay before "if (tty)".
To fix this issue, we check whether the session leader is exiting before assigning a new tty_old_pgrp.
Fixes: c8bcd9c5be24 ("tty: Fix ->session locking") Signed-off-by: Yi Yang <yiyang13@huawei.com> Co-developed-by: GUO Zihua <guozihua@huawei.com> Signed-off-by: GUO Zihua <guozihua@huawei.com> --- drivers/tty/tty_jobctrl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/tty_jobctrl.c b/drivers/tty/tty_jobctrl.c index 0d04287da098..f9a144aaedfc 100644 --- a/drivers/tty/tty_jobctrl.c +++ b/drivers/tty/tty_jobctrl.c @@ -220,7 +220,7 @@ int tty_signal_session_leader(struct tty_struct *tty, int exit_session) put_pid(p->signal->tty_old_pgrp); /* A noop */ spin_lock(&tty->ctrl.lock); tty_pgrp = get_pid(tty->ctrl.pgrp); - if (tty->ctrl.pgrp) + if (tty->ctrl.pgrp && !(p->flags & PF_EXITING)) p->signal->tty_old_pgrp = get_pid(tty->ctrl.pgrp); spin_unlock(&tty->ctrl.lock); -- 2.17.1
| |