From nobody Tue Sep 9 01:06:01 2025 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 26443EB64DD for ; Mon, 24 Jul 2023 03:38:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230037AbjGXDiF (ORCPT ); Sun, 23 Jul 2023 23:38:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39580 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230045AbjGXDh5 (ORCPT ); Sun, 23 Jul 2023 23:37:57 -0400 Received: from szxga03-in.huawei.com (szxga03-in.huawei.com [45.249.212.189]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C7EA2E45 for ; Sun, 23 Jul 2023 20:37:55 -0700 (PDT) Received: from kwepemm600014.china.huawei.com (unknown [172.30.72.54]) by szxga03-in.huawei.com (SkyGuard) with ESMTP id 4R8Qls1ZCBzHqg7; Mon, 24 Jul 2023 11:35:21 +0800 (CST) Received: from ubuntu1804.huawei.com (10.67.175.28) by kwepemm600014.china.huawei.com (7.193.23.54) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.27; Mon, 24 Jul 2023 11:37:53 +0800 From: Yi Yang To: , CC: , , Subject: [PATCH v2] tty: tty_jobctrl: fix pid memleak in disassociate_ctty() Date: Mon, 24 Jul 2023 11:37:14 +0800 Message-ID: <20230724033714.175340-1-yiyang13@huawei.com> X-Mailer: git-send-email 2.17.1 MIME-Version: 1.0 X-Originating-IP: [10.67.175.28] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To kwepemm600014.china.huawei.com (7.193.23.54) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" 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 =3D NULL; tty =3D 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->pgrp) //tty->pgrp is not NULL p->signal->tty_old_pgrp =3D get_pid(tty->pgrp); //An extra get spin_unlock_irq(&p->sighand->siglock); ... tty_unlock(tty); if (tty) { tty_lock(tty); ... put_pid(tty->pgrp); tty->pgrp =3D 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() "tty_signal_session_leader()". To fix this issue, we move put_pid() after "if (tty)". Fixes: c8bcd9c5be24 ("tty: Fix ->session locking") Signed-off-by: Yi Yang Co-developed-by: GUO Zihua Signed-off-by: GUO Zihua --- v2:Completely refactor the solution, avoid the use of PF_EXITING flag and do put_pid() in disassociate_ctty() again instead. --- drivers/tty/tty_jobctrl.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/tty/tty_jobctrl.c b/drivers/tty/tty_jobctrl.c index 0d04287da098..17a6565f428b 100644 --- a/drivers/tty/tty_jobctrl.c +++ b/drivers/tty/tty_jobctrl.c @@ -300,12 +300,7 @@ void disassociate_ctty(int on_exit) return; } =20 - spin_lock_irq(¤t->sighand->siglock); - put_pid(current->signal->tty_old_pgrp); - current->signal->tty_old_pgrp =3D NULL; - tty =3D tty_kref_get(current->signal->tty); - spin_unlock_irq(¤t->sighand->siglock); - + tty =3D get_current_tty(); if (tty) { unsigned long flags; =20 @@ -320,6 +315,11 @@ void disassociate_ctty(int on_exit) tty_kref_put(tty); } =20 + spin_lock_irq(¤t->sighand->siglock); + put_pid(current->signal->tty_old_pgrp); + current->signal->tty_old_pgrp =3D NULL; + spin_unlock_irq(¤t->sighand->siglock); + /* Now clear signal->tty under the lock */ read_lock(&tasklist_lock); session_clear_tty(task_session(current)); --=20 2.17.1