From 8b464de1c458d79613d9d6c186715fae2f57574b Mon Sep 17 00:00:00 2001 From: Yi Yang Date: Thu, 9 Nov 2023 03:11:36 +0000 Subject: [PATCH 1/2] Revert "tty: fix pid memleak in disassociate_ctty()" hulk inclusion category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/I7PBYJ -------------------------------- This reverts commit a2aa6b07bf933a57e5fccab3812d0f7d72c30d56. Prepare to backport patch from mainline. Signed-off-by: Yi Yang --- drivers/tty/tty_jobctrl.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/drivers/tty/tty_jobctrl.c b/drivers/tty/tty_jobctrl.c index e04360de3757..ffcab80ba77d 100644 --- a/drivers/tty/tty_jobctrl.c +++ b/drivers/tty/tty_jobctrl.c @@ -308,18 +308,6 @@ void disassociate_ctty(int on_exit) spin_unlock_irqrestore(&tty->ctrl_lock, flags); tty_unlock(tty); tty_kref_put(tty); - - /* - * Race with tty_signal_session_leader(), current->signal - * ->tty_old_pgrp may be reassigned, put_pid() again to ensure - * the pid does not leak memory. - */ - if (on_exit) { - spin_lock_irq(¤t->sighand->siglock); - put_pid(current->signal->tty_old_pgrp); - current->signal->tty_old_pgrp = NULL; - spin_unlock_irq(¤t->sighand->siglock); - } } /* Now clear signal->tty under the lock */ -- Gitee From 309f0d3824f5801c7cc118d4474a2a20a4b7b1a1 Mon Sep 17 00:00:00 2001 From: Yi Yang Date: Thu, 9 Nov 2023 03:11:37 +0000 Subject: [PATCH 2/2] tty: tty_jobctrl: fix pid memleak in disassociate_ctty() mainline inclusion from mainline-v6.7-rc1 commit 11e7f27b79757b6586645d87b95d5b78375ecdfc category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/I7PBYJ CVE: NA Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=11e7f27b79757b6586645d87b95d5b78375ecdfc --------------------------- There is a pid leakage: ------------------------------ unreferenced object 0xffff88810c181940 (size 224): comm "sshd", pid 8191, jiffies 4294946950 (age 524.570s) hex dump (first 32 bytes): 01 00 00 00 00 00 00 00 00 00 00 00 ad 4e ad de .............N.. ff ff ff ff 6b 6b 6b 6b ff ff ff ff ff ff ff ff ....kkkk........ backtrace: [] kmem_cache_alloc+0x5c6/0x9b0 [] alloc_pid+0x72/0x570 [] copy_process+0x1374/0x2470 [] kernel_clone+0xb7/0x900 [] __se_sys_clone+0x85/0xb0 [] __x64_sys_clone+0x2b/0x30 [] do_syscall_64+0x32/0x80 [] entry_SYSCALL_64_after_hwframe+0x61/0xc6 It turns out that there is a race condition between disassociate_ctty() and tty_signal_session_leader(), which caused this leakage. The pid memleak is triggered by the following race: task[sshd] task[bash] ----------------------- ----------------------- 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_vhangup(); tty_lock(tty); ... tty_signal_session_leader(); spin_lock_irq(&p->sighand->siglock); ... if (tty->ctrl.pgrp) //tty->ctrl.pgrp is not NULL p->signal->tty_old_pgrp = get_pid(tty->ctrl.pgrp); //An extra get spin_unlock_irq(&p->sighand->siglock); ... tty_unlock(tty); if (tty) { tty_lock(tty); ... put_pid(tty->ctrl.pgrp); tty->ctrl.pgrp = NULL; //It's too late ... tty_unlock(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)" and at the entrance of tty_signal_session_leader(). To fix this issue, we move "put_pid(current->signal->tty_old_pgrp)" after "tty->ctrl.pgrp = NULL". Fixes: c8bcd9c5be24 ("tty: Fix ->session locking") Signed-off-by: Yi Yang Co-developed-by: GUO Zihua Signed-off-by: GUO Zihua Link: https://lore.kernel.org/r/20230831023329.165737-1-yiyang13@huawei.com Signed-off-by: Greg Kroah-Hartman Signed-off-by: Yi Yang --- drivers/tty/tty_jobctrl.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/tty/tty_jobctrl.c b/drivers/tty/tty_jobctrl.c index ffcab80ba77d..73fdd55c6bef 100644 --- a/drivers/tty/tty_jobctrl.c +++ b/drivers/tty/tty_jobctrl.c @@ -290,12 +290,7 @@ void disassociate_ctty(int on_exit) return; } - 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 = get_current_tty(); if (tty) { unsigned long flags; @@ -310,6 +305,16 @@ void disassociate_ctty(int on_exit) tty_kref_put(tty); } + /* If tty->ctrl.pgrp is not NULL, it may be assigned to + * current->signal->tty_old_pgrp in a race condition, and + * cause pid memleak. Release current->signal->tty_old_pgrp + * after tty->ctrl.pgrp set to NULL. + */ + spin_lock_irq(¤t->sighand->siglock); + put_pid(current->signal->tty_old_pgrp); + current->signal->tty_old_pgrp = NULL; + spin_unlock_irq(¤t->sighand->siglock); + /* Now clear signal->tty under the lock */ read_lock(&tasklist_lock); session_clear_tty(task_session(current)); -- Gitee