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 9C569CF6498 for ; Mon, 30 Sep 2024 02:59:49 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 292DB80010; Sun, 29 Sep 2024 22:59:49 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 21AF480009; Sun, 29 Sep 2024 22:59:49 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0964080010; Sun, 29 Sep 2024 22:59:49 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id DBEAD80009 for ; Sun, 29 Sep 2024 22:59:48 -0400 (EDT) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 6223812077D for ; Mon, 30 Sep 2024 02:59:48 +0000 (UTC) X-FDA: 82619899656.29.447CD7C Received: from out01.mta.xmission.com (out01.mta.xmission.com [166.70.13.231]) by imf19.hostedemail.com (Postfix) with ESMTP id B7CD81A0006 for ; Mon, 30 Sep 2024 02:59:45 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=none; spf=pass (imf19.hostedemail.com: domain of ebiederm@xmission.com designates 166.70.13.231 as permitted sender) smtp.mailfrom=ebiederm@xmission.com; dmarc=pass (policy=none) header.from=xmission.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1727665147; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=kU6t+Z9/A12xBOBFL47NlWBmuTlEoOAoW+CTJVkX3RY=; b=hS7j3CbXDJskyX2VlDIRXB4alOcrYs4vUuhVCNtu+sYskIw/HlaLSpX70ts+kiaMfYyAOK 8sVV7mWbPWvwuuYxPr1oPdumuSjdkBpouq5hPxSQLLHLwujqcfMgeZ5AHytn9mlVW82swA tLCN5HQrcDaraoX34ghXnqszxkgR4zY= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=none; spf=pass (imf19.hostedemail.com: domain of ebiederm@xmission.com designates 166.70.13.231 as permitted sender) smtp.mailfrom=ebiederm@xmission.com; dmarc=pass (policy=none) header.from=xmission.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1727665147; a=rsa-sha256; cv=none; b=3Votz+EoP6VAJQOvrFDiGVeduKDc1lqlAnBr4UmQP/GpJOqgcKt5NJc7esT1cT/1fPQju6 BPNeHgSNyOhJbDLleeZubPGXXQZLn0s6QrsS8zH7SxJ970znlI+B6oQONmEzOOaXLMMt9W PNSMM4a1B/Clx4RlpgBMzzVqOR+lx9Q= Received: from in01.mta.xmission.com ([166.70.13.51]:52830) by out01.mta.xmission.com with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1sv6dS-008ETw-8l; Sun, 29 Sep 2024 20:59:42 -0600 Received: from ip68-227-165-127.om.om.cox.net ([68.227.165.127]:32856 helo=email.froward.int.ebiederm.org.xmission.com) by in01.mta.xmission.com with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1sv6dP-00EYe7-PT; Sun, 29 Sep 2024 20:59:41 -0600 From: "Eric W. Biederman" To: Kees Cook Cc: Tycho Andersen , Alexander Viro , Christian Brauner , Jan Kara , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, Tycho Andersen , Zbigniew =?utf-8?Q?J=C4=99drzejewski-S?= =?utf-8?Q?zmek?= , Aleksa Sarai In-Reply-To: <202409281453.B9B9999D@keescook> (Kees Cook's message of "Sat, 28 Sep 2024 14:56:02 -0700") References: <20240927151746.391931-1-tycho@tycho.pizza> <87ikuhw155.fsf@email.froward.int.ebiederm.org> <202409281453.B9B9999D@keescook> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) Date: Sun, 29 Sep 2024 21:59:30 -0500 Message-ID: <87bk05vobx.fsf@email.froward.int.ebiederm.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-XM-SPF: eid=1sv6dP-00EYe7-PT;;;mid=<87bk05vobx.fsf@email.froward.int.ebiederm.org>;;;hst=in01.mta.xmission.com;;;ip=68.227.165.127;;;frm=ebiederm@xmission.com;;;spf=pass X-XM-AID: U2FsdGVkX18j3SuS4P3g62L+fkN3a1rTsScWHuGCQ9o= Subject: Re: [PATCH v2 1/2] exec: add a flag for "reasonable" execveat() comm X-SA-Exim-Connect-IP: 166.70.13.51 X-SA-Exim-Rcpt-To: cyphar@cyphar.com, zbyszek@in.waw.pl, tandersen@netflix.com, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, jack@suse.cz, brauner@kernel.org, viro@zeniv.linux.org.uk, tycho@tycho.pizza, kees@kernel.org X-SA-Exim-Mail-From: ebiederm@xmission.com X-SA-Exim-Scanned: No (on out01.mta.xmission.com); SAEximRunCond expanded to false X-Rspam-User: X-Stat-Signature: kjnhd5bp97s5c7j19nq43y7xrd4c9edi X-Rspamd-Queue-Id: B7CD81A0006 X-Rspamd-Server: rspam11 X-HE-Tag: 1727665185-405027 X-HE-Meta: U2FsdGVkX1/+vJ+g2AGjfMLbRMA79vANOi6uYpiAjlL3K95VLJ3XeIg6iwqeZ6qYbgQtEfI5r+IY6YzYfsTWom6cDObGp3N5tv2CSpPQPFJvWn8YK8RQoaXkDeZ+2OOMW87wqgPCIaEKQpCe2Q4W5+QfLb1qMG0dOoCSGkHDIDwHDbSdEOTvvXVtiAxMp/fCi2F+A2nxEjOps0H6zPd/e7zwjaWiKPnC1iaiY/3ENG6tHNXYnnic1TwZ/4uDl5pkL7AYW9/LQJmi/0LX0gj5ix2MZ04KdmULGftSWaLLT81vM7fYtqRwZxtTWCiEO/RxP9rmic5R2IozPFciAIEbcL6Ieq3/IJV5hZkwgX70BlLu2ksEkuTwy99IPfyFBwjTTzpDDUL5jmTKiJk5CFlbQTqT8L3EK3VbD3nt/YmaycFyP3HFl6ozRzy/dypx4fH9vqvuhBRacrqzUGycCvAoCelVEfu3eILEthVLmHikwTmpAei0bRI96Q0pznVg+/mmtB9dBimYPokutFfRPEZ817LCGYwjgWBi0jvG8LY3L02WGfXmJbn1L1Er5BdUmMBSapgzBzSz99HIzYNN/Gdg15P04kct9cbffUrjpabGSGYhxxQE3Yl2MOnY1c5qhlSXCl44Z2N38dpG2bHFIY7y83vfNOjLsa3mrHS+OW9Pz23c+3RSzEKx5/rlQ3rajRr7Fuxa8yvLf7mTV6TFfilclMpdW1TF3j1LyeivEWg7iWsJujM+pMwqkVZlK6MR4UmJ+tItS1j5AyYbPIND6WlTgbsaVqcpgKhDj9xh7LUJVPOlcOPtzsMGDSVMNDnj19r9qRLmi+84f2jUy6tzFtsb3JqNB9jTWha7YoGRdSp8nO8vmhp6aXj26tUv63iJ41dTSwQT8rymaajJBjKlyGeMnWXPybuFBtd2InKFhIJ0NrfU9UqrFmI2sAjr8NyDWpoQOxuvXrtnLBLowzIqTaW YN1Bm1ut 1mE6+FDk3IurkWWFMFDyQq5FyP8O6xU3TAs3xgGKOulHoddTZiDC22OVJZzz3ykuESzaCTxrkGu/OfS4C0byZEoTCXOlwLb1n7i/yrcF8Z1Ai1f1lLg+xCGUr0x8E/qj/eeUL5BQ02LsW+rOu65OZG8WmjFghbP4X+YTkgKcdhhoQ13ocH3VAZElzUbJDcg28uiLjEcp+Fj3hyZaIFtbGsND7MrfHu5+6G4OJE+kq4GZ+k9E= 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: Kees Cook writes: > On Fri, Sep 27, 2024 at 10:45:58AM -0500, Eric W. Biederman wrote: >> Tycho Andersen writes: >>=20 >> > From: Tycho Andersen >> > >> > Zbigniew mentioned at Linux Plumber's that systemd is interested in >> > switching to execveat() for service execution, but can't, because the >> > contents of /proc/pid/comm are the file descriptor which was used, >> > instead of the path to the binary. This makes the output of tools like >> > top and ps useless, especially in a world where most fds are opened >> > CLOEXEC so the number is truly meaningless. >> > >> > Change exec path to fix up /proc/pid/comm in the case where we have >> > allocated one of these synthetic paths in bprm_init(). This way the ac= tual >> > exec machinery is unchanged, but cosmetically the comm looks reasonabl= e to >> > admins investigating things. >>=20 >> Perhaps change the subject to match the code. >>=20 >> > Signed-off-by: Tycho Andersen >> > Suggested-by: Zbigniew J=C4=99drzejewski-Szmek >> > CC: Aleksa Sarai >> > Link: https://github.com/uapi-group/kernel-features#set-comm-field-bef= ore-exec >> > --- >> > v2: * drop the flag, everyone :) >> > * change the rendered value to f_path.dentry->d_name.name instead = of >> > argv[0], Eric >> > --- >> > fs/exec.c | 13 ++++++++++++- >> > 1 file changed, 12 insertions(+), 1 deletion(-) >> > >> > diff --git a/fs/exec.c b/fs/exec.c >> > index dad402d55681..9520359a8dcc 100644 >> > --- a/fs/exec.c >> > +++ b/fs/exec.c >> > @@ -1416,7 +1416,18 @@ int begin_new_exec(struct linux_binprm * bprm) >> > set_dumpable(current->mm, SUID_DUMP_USER); >> >=20=20 >> > perf_event_exec(); >> > - __set_task_comm(me, kbasename(bprm->filename), true); >> > + >> > + /* >> > + * If fdpath was set, execveat() made up a path that will >> > + * probably not be useful to admins running ps or similar. >> > + * Let's fix it up to be something reasonable. >> > + */ >> > + if (bprm->fdpath) { >> > + BUILD_BUG_ON(TASK_COMM_LEN > DNAME_INLINE_LEN); >> > + __set_task_comm(me, bprm->file->f_path.dentry->d_name.name, true); >>=20 >> We can just do this regardless of bprm->fdpath. >>=20 >> It will be a change of behavior on when executing symlinks and possibly >> mount points but I don't think we care. If we do then we can add make >> it conditional with "if (bprm->fdpath)" >>=20 >> At the very least using the above version unconditionally ought to flush >> out any bugs. > > I'm not super comfortable doing this regardless of bprm->fdpath; that > seems like too many cases getting changed. Can we just leave it as > depending on bprm->fdpath? > > Also, is d_name.name always going to be set? e.g. what about memfd, > etc? Reading __d_alloc I don't see how a dentry can ever be allocated with a NULL pointer in d_name.name. There are filesystems that implement .d_dname and have a special case in d_path(). I don't imagine when dealing with executables we care. I can see an argument for having a helper function that wraps the pointer load and uses READ_ONCE() or smp_load_acquire() something like: const char *d_dname(const struct path *path) { /* see prepend_name for the rational */ return smp_load_acquire(&path->dentry->d_name.name); } I kind of see the optimization being nice enough in the common case, and the weird corner cases where the differences are apparent in task->comm (symlinks, bind mounts) being rare enough. That it is probably worth the optimization. Like a number of things that change userspace behavior we just document that in rare cases task->comm will have a different value and if it is a problem for someone we just add back in the fdpath check. Given the ease of code maintenance we get if one piece of straight line code can work for everyone it seems worth trying. Kees at the end of the day it is your call what code to merge. I just expect it is unnecessary complexity to be tentative about the change. Eric