[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cb540eb6c27f4a71803d094505725ae1@huawei.com>
Date: Mon, 10 Jul 2023 07:23:57 +0000
From: "yiyang (D)" <yiyang13@...wei.com>
To: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"jirislaby@...nel.org" <jirislaby@...nel.org>
CC: "jannh@...gle.com" <jannh@...gle.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"Fengtao (fengtao, Euler)" <fengtao40@...wei.com>,
"Guozihua (Scott)" <guozihua@...wei.com>
Subject: RE: [PATCH RFC] tty: tty_jobctrl: fix pid memleak in
tty_signal_session_leader()
Ping
-----Original Message-----
From: yiyang (D)
Sent: 2023年7月3日 16:03
To: gregkh@...uxfoundation.org; jirislaby@...nel.org
Cc: jannh@...gle.com; linux-kernel@...r.kernel.org; Fengtao (fengtao, Euler) <fengtao40@...wei.com>; Guozihua (Scott) <guozihua@...wei.com>; yiyang (D) <yiyang13@...wei.com>
Subject: [PATCH RFC] tty: tty_jobctrl: fix pid memleak in tty_signal_session_leader()
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@...wei.com>
Co-developed-by: GUO Zihua <guozihua@...wei.com>
Signed-off-by: GUO Zihua <guozihua@...wei.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
Powered by blists - more mailing lists