From mboxrd@z Thu Jan 1 00:00:00 1970 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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3FE38C0218A for ; Sat, 1 Feb 2025 09:40:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 811006B007B; Sat, 1 Feb 2025 04:40:07 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 7C0D36B0082; Sat, 1 Feb 2025 04:40:07 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 688526B0083; Sat, 1 Feb 2025 04:40:07 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 4B97B6B007B for ; Sat, 1 Feb 2025 04:40:07 -0500 (EST) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id C4140B16BA for ; Sat, 1 Feb 2025 09:40:06 +0000 (UTC) X-FDA: 83070879612.26.8D926D4 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf22.hostedemail.com (Postfix) with ESMTP id 187D1C0006 for ; Sat, 1 Feb 2025 09:40:04 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=PSCfX8KN; spf=pass (imf22.hostedemail.com: domain of kees@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=kees@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1738402805; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=K6iIoWVsEkDXhPd5aqOGkxGMVeCVJRwDN+bUJUoai0o=; b=fg0dWTPD1XZQoJnb2v6a2JwM+0Cy5IBW11MZNGbKxCdydR2s4NNodFOQI2LKC22YGZOYER 5GyeBC9lrjS77r23LNUm368ipSMeco4fx+9vHn+kS+gHGZze1NuxstmXcqN4siVeiDv7Zl oDqFGC+MqqtjwCvgR6RCkCgl6IX+suI= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1738402805; a=rsa-sha256; cv=none; b=U/rb9GdZPWaFjdr/zzsx5lVhsNE/y4d6h21v4CL8Jkv6kKubKcfGkrspUOj8IW6UrbYHCC D6rKjdbCZQD64f/2obbweLtx5dZTZmcxUPBxSIFoklOH4guax14U/6976pZPnma4LxScYm +2ZdaPL9aN8ve2bjuvSCi2BvXLCoxs0= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=PSCfX8KN; spf=pass (imf22.hostedemail.com: domain of kees@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=kees@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 15BFD5C47CD; Sat, 1 Feb 2025 09:39:24 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 827E1C4CED3; Sat, 1 Feb 2025 09:40:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1738402803; bh=aAMRfRl5ZHwn79N0tp/fQ8i+GehS66SCFBO187z07bE=; h=Date:From:To:Subject:In-Reply-To:References:From; b=PSCfX8KNh15ymm4kfHgnW1KsRLIzB1kmBvu8P/ZPITWkUKaBP8aAJmCDo1aWrhcS7 9J6U2aHbzUf0YQ0GIKl15eCr5SimYPJW3PxE4XX/0Lqn3+pDD8VgvTIdQvJJ5eUOAi rOxvqva+v77/EA8iyoc0Vc4wQJk71yQUFokKP6mka3zq89AWRv5q8y5oaaN/n9a3z2 w8g77L2qq9oFzb63YSWFLmojozL8GTBtqJmG5zBE9GAh8OUQOAZdMt8hNy9w9vTLwn PhmTwoImt5Lybw/oY+qB2ypewSuiKAm0Kwk6h9w2cAeiExi1CL9BZUYlqgA+vyRyFY PDrbycaYs4fmA== Date: Sat, 01 Feb 2025 01:40:00 -0800 From: Kees Cook To: Nir Lichtman , viro@zeniv.linux.org.uk, brauner@kernel.org, jack@suse.cz, ebiederm@xmission.com, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] exec: remove redundant save asides of old pid/vpid User-Agent: K-9 Mail for Android In-Reply-To: <20250201083127.GA1185473@lichtman.org> References: <20250201083127.GA1185473@lichtman.org> Message-ID: <0B25310A-0907-481E-8ADF-EEFA78927BFF@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 187D1C0006 X-Stat-Signature: 9jfkmyk6aymog84msdnf5okix891gmkz X-Rspam-User: X-Rspamd-Server: rspam12 X-HE-Tag: 1738402804-911376 X-HE-Meta: U2FsdGVkX1+BldjYVWevGJyCBRN4xkS+NXNOiE7yvgQTRUFPylDsyfuZzTWgFEa80OrdBoJR7Qilq9a4vRsPh5NdZISeHGwmjJKLwP2alSNrvpbTJgfnr6InDgWxpCyFTJcqV3yAVf2bfHEhIcmpdteGSpIFxIE0/6d/8+DhZ21yxlJrsIc2wf/wEdSHBL7dHstw/CQzh1+0WHreKCPKmh1U9QzLsTmJ2DNl2Ej8Vpgql5Apfdv9a/GV2erKCWlzAINbGixoH7Gm+YoTFVd/lFVaVRcDvecXwsZZ9FOGKTUxk+ibxuFu7dCJlXbNfvPzMIbVZaln3bpYFuqMFuoClT0zpuk9Duzdl6UG1LpdB5ucyWhIbiX4Wu027eooVv03bZiWd37HUTzBwKth8fhJJwMCEBcaJw9uDgiEmsPNhQHU4BdeD3Mn4Q1AKIhVXBJfk0IiP8daR8eOLV4g1v1KNrX8f+ic69SzraMa4fG887SFYbWuxWqAp+qXNlpGmUVT2Rv/KtWFfuIFZRRQKoLLw3nQGSv/SpukFb9SCdP9ZWayJvOcA4B2k/3EgEg36z7ivV6Ut/ETZTIJzWbX90rc3Fcq5RXZOYvmivYJCNhb6rvH1NCbtM4nY9mT4kmE6foN6ApKVTTg/yCkjdbnpRB+Plz0ooDNE6gI9f4C8tS2VouMR9aW/SWzi4rvPNUYV2O5RrTfrDcAmhM7nH3TKkkjBBtgLEL9ToSW37U1WgeEyxDixvVh6Cajlurn4kx977Gesn7VkDuEsi8HBE9nTCncNiPasuPjC4rcEVv2s6O4miox3ktqJNfKHhlJLEvLrHsXS49uVDefa2lZjBJVVsbBfAXWRlkahmEuwV8C3IInON8NRz3OPjvsm2aUKecfcuU8bdyXemAPPcOm3ATAlccETy12+Jm8IePyttLD4/fEE2yP00dbyYQtFx+exhDnchzCnGNyC6/P3Cgn1LE8v1O 69umV6Vd 6M1I6kUXY2QTtSEeIhFt8TkEFlDBDSuiGVUwZRAJrUxx+jip4KTv5XgqRXyXK7Py/nWFxKLhSKaNnKfYW3PcMvTqzMNsjG17RDympLTGrsN5S0I3XU4iBZc790VJ4bm/HuYZsFgUeXBVv7zMDglwV+3BIXX0FVI60YLMUKW5PZDf6cFMbb6j0kqmuaxxdGVQwi6yjCUNQPvQCx2aydz85FLyTTu9MphLMz/j9DKgsUYLirWxnAsgUewhWtxn2XR4lBKuZtkP9TCN7DfTs2qgcf+028w== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On February 1, 2025 12:31:27 AM PST, Nir Lichtman wro= te: >Problem: Old pid and vpid are redundantly saved aside before starting to >parse the binary, with the comment claiming that it is required since >load_binary changes it, though from inspection in the source, >load_binary does not change the pid and this wouldn't make sense since >execve does not create any new process, quote from man page of execve: >"there is no new process; many attributes of the calling process remain >unchanged (in particular, its PID)=2E" See commit bb188d7e64de ("ptrace: make former thread ID available via PTRA= CE_GETEVENTMSG after PTRACE_EVENT_EXEC stop") This is for making sense of a concurrent exec made by a multi threaded pro= cess=2E Specifically see de_thread(), where the pid *can* change: /* * At this point all other threads have exited, all we have to * do is to wait for the thread group leader to become inactive, * and to assume its PID: */ The described problem in the commit hasn't changed, so this code needs to = stay as-is=2E Or perhaps the comment could be improved? -Kees > >Solution: Remove the saving aside of both and later on use them directly >from the current object, instead of via the saved aside objects=2E > >Signed-off-by: Nir Lichtman >--- > >Side-note: Tested this solution with a defconfig x86_64 and an initramfs >with Busybox and confirmed to work fine=2E > > fs/exec=2Ec | 12 +++--------- > 1 file changed, 3 insertions(+), 9 deletions(-) > >diff --git a/fs/exec=2Ec b/fs/exec=2Ec >index 506cd411f4ac=2E=2E6bb0a7b15f7e 100644 >--- a/fs/exec=2Ec >+++ b/fs/exec=2Ec >@@ -1789,15 +1789,8 @@ static int search_binary_handler(struct linux_binp= rm *bprm) > /* binfmt handlers will call back into begin_new_exec() on success=2E */ > static int exec_binprm(struct linux_binprm *bprm) > { >- pid_t old_pid, old_vpid; > int ret, depth; >=20 >- /* Need to fetch pid before load_binary changes it */ >- old_pid =3D current->pid; >- rcu_read_lock(); >- old_vpid =3D task_pid_nr_ns(current, task_active_pid_ns(current->parent= )); >- rcu_read_unlock(); >- > /* This allows 4 levels of binfmt rewrites before failing hard=2E */ > for (depth =3D 0;; depth++) { > struct file *exec; >@@ -1826,8 +1819,9 @@ static int exec_binprm(struct linux_binprm *bprm) > } >=20 > audit_bprm(bprm); >- trace_sched_process_exec(current, old_pid, bprm); >- ptrace_event(PTRACE_EVENT_EXEC, old_vpid); >+ trace_sched_process_exec(current, current->pid, bprm); >+ ptrace_event(PTRACE_EVENT_EXEC, >+ task_pid_nr_ns(current, task_active_pid_ns(current->parent))); > proc_exec_connector(current); > return 0; > } --=20 Kees Cook