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 24E65CF9C69 for ; Tue, 24 Sep 2024 17:40:51 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AB9C86B00AB; Tue, 24 Sep 2024 13:40:50 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A68F86B00AC; Tue, 24 Sep 2024 13:40:50 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8E34D6B00AD; Tue, 24 Sep 2024 13:40:50 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 686406B00AB for ; Tue, 24 Sep 2024 13:40:50 -0400 (EDT) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 3E4671A0245 for ; Tue, 24 Sep 2024 17:40:49 +0000 (UTC) X-FDA: 82600347018.17.FCCD86C Received: from out02.mta.xmission.com (out02.mta.xmission.com [166.70.13.232]) by imf15.hostedemail.com (Postfix) with ESMTP id 020F7A0019 for ; Tue, 24 Sep 2024 17:40:45 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=none; spf=pass (imf15.hostedemail.com: domain of ebiederm@xmission.com designates 166.70.13.232 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=1727199586; 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=jesO6dRSmS/RMDNyY9M3MkIrRSYDgba24O0xb+IBz7E=; b=cx/XzxBV0V9zTYR5fqcLVnidJ/xCVcq+T4fpAh/zMMlKbOG1NTCZnmZ6PxAIKb39GidCIO wwxhyRrhYXrZAsu31TGF/OulELbRZfynGTXJ2fslUs25Zm6yX8g9BoRhu9X1SDl+PZhIjE CMZ5ZwcRM1GyVJx5z3dIEO4HiFEMyPY= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=none; spf=pass (imf15.hostedemail.com: domain of ebiederm@xmission.com designates 166.70.13.232 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=1727199586; a=rsa-sha256; cv=none; b=SWT3yUIR7XjoW+ZEko26Y/dswqhxSDsvC14OQMSNAZjo4QV27LOdTvEMPMrOKLFHLqQdSN tu3Jy7+kbLsFKbZeRjQn9e1pVosI2mp7AyjMgw1H1szSUKxhKgTFDn8oM8cIXkM/KF/oLv 3LqcVE8kH+cC6Y08R4bRQAs8MhHwEcY= Received: from in02.mta.xmission.com ([166.70.13.52]:55804) by out02.mta.xmission.com with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1st9Wk-005D2q-Vn; Tue, 24 Sep 2024 11:40:43 -0600 Received: from ip68-227-165-127.om.om.cox.net ([68.227.165.127]:34342 helo=email.froward.int.ebiederm.org.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1st9Wj-005TZN-MX; Tue, 24 Sep 2024 11:40:42 -0600 From: "Eric W. Biederman" To: Tycho Andersen Cc: Alexander Viro , Christian Brauner , Jan Kara , Kees Cook , Jeff Layton , Chuck Lever , Alexander Aring , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Tycho Andersen , Zbigniew =?utf-8?Q?J=C4=99drzejewski-Szmek?= , Aleksa Sarai References: <20240924141001.116584-1-tycho@tycho.pizza> Date: Tue, 24 Sep 2024 12:39:35 -0500 In-Reply-To: <20240924141001.116584-1-tycho@tycho.pizza> (Tycho Andersen's message of "Tue, 24 Sep 2024 08:10:01 -0600") Message-ID: <87msjx9ciw.fsf@email.froward.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-XM-SPF: eid=1st9Wj-005TZN-MX;;;mid=<87msjx9ciw.fsf@email.froward.int.ebiederm.org>;;;hst=in02.mta.xmission.com;;;ip=68.227.165.127;;;frm=ebiederm@xmission.com;;;spf=pass X-XM-AID: U2FsdGVkX1+fnbih3tUHG8eMw0G+LQU28U/qMv8L3Bk= Subject: Re: [RFC] exec: add a flag for "reasonable" execveat() comm X-SA-Exim-Connect-IP: 166.70.13.52 X-SA-Exim-Rcpt-To: cyphar@cyphar.com, zbyszek@in.waw.pl, tandersen@netflix.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, alex.aring@gmail.com, chuck.lever@oracle.com, jlayton@kernel.org, kees@kernel.org, jack@suse.cz, brauner@kernel.org, viro@zeniv.linux.org.uk, tycho@tycho.pizza X-SA-Exim-Mail-From: ebiederm@xmission.com X-SA-Exim-Scanned: No (on out02.mta.xmission.com); SAEximRunCond expanded to false X-Rspamd-Server: rspam03 X-Rspam-User: X-Rspamd-Queue-Id: 020F7A0019 X-Stat-Signature: eug9a1yierzqd7zwty7m6my9t8dte7t7 X-HE-Tag: 1727199645-927110 X-HE-Meta: U2FsdGVkX18QidDeR4q6I1uSt9xEmRlZU26WVeyRqKC//UYIj1N6uYZk5S1tzbfCwx0AWA7WfQm8RjSxpDKxboXokFgZstz1p6vtogI044HnBy6hoH6g0lcDrYcfP7fSiXUFLDbuywO9XXr9Eq3/wSjDtkcsbS4jMkPXfOqElG6jhb0ZcJeciwwM9dlbTt66GIzNO3gO2Gexn8QUHfs03QImySjB22YEsFsvwaITvVEsq5N3pVktqdtkwZnWrypRR1LgveZkRSe5DM4QzQ8gWtRXuF+MEPTEVi1+iRLMUFqfODFE0lykHybu5LH/UREahU3mhY4AcXdP36XAbdOBsy4PQXtL/TG3Ospt+ypVA27dNltkptFf9VDAUyCYALx0+xSeccCBAp8uSdJBs+i879+q2ppPPqT1Eidf98Ul0TO5p6D40T9z8PscJfJ9K+qs3+UerkXFOsHfkBcEKsAdB13LOwFzfqkASKvXY90ud1NtkZ+JXVPguLH8KxCgA9iZkwE+1h7yX8FUdHpxevKhC5NkToznmEzP+TJ5eGlUbGSmcVx6Tg5zIw2tpAcjQlJ4dA+ksTAKLV2UEGWfmnProzk+ln4/sX58AtyRZO75KlHFLRgk2UMGJjA43twN8fCjft6c0QJeSD+jRfmUpa4DIoi3JH1waX3yltNXJN4NoNO00ZYrYWJcpWwo44joGixE9K+W8UeHxWDet/VzWQ9D1g5la2jqvq1iWjK/0oIFyraMyKpHg3pedTZCVxwYC01TqqiqI47G1FBrQFFwuFVZaB5muSq/vLiNPQ4xWupPuEl7Cf7Xw+qZhBpX34gi06HZhgaQlyTMS2MtxxRI3KGOWnrr/pKFq8ZXeo8Uc0wJV2591hD+Hg5tfdCcBSAaIKrr6vm+A0RntBmhBaEQs8Amzuh8+PKJhEC/xYQS1/dsP9r6iPvIleGa2S1j9VeZk8C/NMxEN/84zDOA+Sejk01 fRIp05L8 kZENLVe0mS8K3u1Fk3O1eMm90iH5WgISDjzmtnYSbhCedvMJ9ZvdhYbTJk1NB5w5nY2YJWeEN47go40IukAeuMDJR44pJs6nAPZsb4J0/GLHhKm87oenUU9b2FrOsW6Rn5QEOgYPW4meRmTZQhkueJoKTmA8jVsx4+9jLhmiBXg94F3sEiNl0CvV78Iwi+ng9alUHtaEXTKP/ZPOkneqtvNd7xH8LFKf545O7 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: Tycho Andersen writes: > 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. > > This patch adds an AT_ flag to fix up /proc/pid/comm to instead be the > contents of argv[0], instead of the fdno. The kernel allows prctl(PR_SET_NAME, ...) without any permission checks so adding an AT_ flat to use argv[0] instead of the execed filename seems reasonable. Maybe the flag should be called AT_NAME_ARGV0. That said I am trying to remember why we picked /dev/fd/N, as the filename. My memory is that we couldn't think of anything more reasonable to use. Looking at commit 51f39a1f0cea ("syscalls: implement execveat() system call") unfortunately doesn't clarify anything for me, except that /dev/fd/N was a reasonable choice. I am thinking the code could reasonably try: get_fs_root_rcu(current->fs, &root); path =3D __d_path(file->f_path, root, buf, buflen); To see if a path to the file from the current root directory can be found. For files that are not reachable from the current root the code still need to fallback to /dev/fd/N. Do you think you can investigate that and see if that would generate a reasonable task->comm? If for no other reason than because it would generate a usable result for #! scripts, without /proc mounted. It looks like a reasonable case can be made that while /dev/fd/N is a good path for interpreters, it is never a good choice for comm, so perhaps we could always use argv[0] if the fdpath is of the form /dev/fd/N. All of that said I am not a fan of the implementation below as it has the side effect of replacing /dev/fd/N with a filename that is not usable by #! interpreters. So I suggest an implementation that affects task->comm and not brpm->filename. Eric > Signed-off-by: Tycho Andersen > Suggested-by: Zbigniew J=C4=99drzejewski-Szmek > CC: Aleksa Sarai > --- > There is some question about what to name the flag; it seems to me that > "everyone wants this" instead of the fdno, but probably "REASONABLE" is n= ot > a good choice. > > Also, requiring the arg to alloc_bprm() is a bit ugly: kernel-based execs > will never use this, so they just have to pass an empty thing. We could > introduce a bprm_fixup_comm() to do the munging there, but then the code > paths start to diverge, which is maybe not nice. I left it this way becau= se > this is the smallest patch in terms of size, but I'm happy to change it. > > Finally, here is a small set of test programs, I'm happy to turn them into > kselftests if we agree on an API > > #include > #include > #include > #include > #include > #include > > int main(void) > { > int fd; > char buf[128]; > > fd =3D open("/proc/self/comm", O_RDONLY); > if (fd < 0) { > perror("open comm"); > exit(1); > } > > if (read(fd, buf, 128) < 0) { > perror("read"); > exit(1); > } > > printf("comm: %s", buf); > exit(0); > } > > #define _GNU_SOURCE > #include > #include > #include > #include > #include > #include > #include > #include > > #ifndef AT_EMPTY_PATH > #define AT_EMPTY_PATH 0x1000 /* Allow empty relat= ive */ > #endif > > #ifndef AT_EXEC_REASONABLE_COMM > #define AT_EXEC_REASONABLE_COMM 0x200 > #endif > > int main(int argc, char *argv[]) > { > pid_t pid; > int status; > bool wants_reasonable_comm =3D argc > 1; > > pid =3D fork(); > if (pid < 0) { > perror("fork"); > exit(1); > } > > if (pid =3D=3D 0) { > int fd; > long ret, flags; > > fd =3D open("./catprocselfcomm", O_PATH); > if (fd < 0) { > perror("open catprocselfname"); > exit(1); > } > > flags =3D AT_EMPTY_PATH; > if (wants_reasonable_comm) > flags |=3D AT_EXEC_REASONABLE_COMM; > syscall(__NR_execveat, fd, "", (char *[]){"./catprocselfcomm", NULL}, N= ULL, flags); > fprintf(stderr, "execveat failed %d\n", errno); > exit(1); > } > > if (waitpid(pid, &status, 0) !=3D pid) { > fprintf(stderr, "wrong child\n"); > exit(1); > } > > if (!WIFEXITED(status)) { > fprintf(stderr, "exit status %x\n", status); > exit(1); > } > > if (WEXITSTATUS(status) !=3D 0) { > fprintf(stderr, "child failed\n"); > exit(1); > } > > return 0; > } > --- > fs/exec.c | 22 ++++++++++++++++++---- > include/uapi/linux/fcntl.h | 3 ++- > 2 files changed, 20 insertions(+), 5 deletions(-) > > diff --git a/fs/exec.c b/fs/exec.c > index dad402d55681..36434feddb7b 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -1569,11 +1569,15 @@ static void free_bprm(struct linux_binprm *bprm) > kfree(bprm); > } >=20=20 > -static struct linux_binprm *alloc_bprm(int fd, struct filename *filename= , int flags) > +static struct linux_binprm *alloc_bprm(int fd, struct filename *filename, > + struct user_arg_ptr argv, int flags) > { > struct linux_binprm *bprm; > struct file *file; > int retval =3D -ENOMEM; > + bool needs_comm_fixup =3D flags & AT_EXEC_REASONABLE_COMM; > + > + flags &=3D ~AT_EXEC_REASONABLE_COMM; >=20=20 > file =3D do_open_execat(fd, filename, flags); > if (IS_ERR(file)) > @@ -1590,11 +1594,20 @@ static struct linux_binprm *alloc_bprm(int fd, st= ruct filename *filename, int fl > if (fd =3D=3D AT_FDCWD || filename->name[0] =3D=3D '/') { > bprm->filename =3D filename->name; > } else { > - if (filename->name[0] =3D=3D '\0') > + if (needs_comm_fixup) { > + const char __user *p =3D get_user_arg_ptr(argv, 0); > + > + retval =3D -EFAULT; > + if (!p) > + goto out_free; > + > + bprm->fdpath =3D strndup_user(p, MAX_ARG_STRLEN); > + } else if (filename->name[0] =3D=3D '\0') > bprm->fdpath =3D kasprintf(GFP_KERNEL, "/dev/fd/%d", fd); > else > bprm->fdpath =3D kasprintf(GFP_KERNEL, "/dev/fd/%d/%s", > fd, filename->name); > + retval =3D -ENOMEM; > if (!bprm->fdpath) > goto out_free; >=20=20 > @@ -1969,7 +1982,7 @@ static int do_execveat_common(int fd, struct filena= me *filename, > * further execve() calls fail. */ > current->flags &=3D ~PF_NPROC_EXCEEDED; >=20=20 > - bprm =3D alloc_bprm(fd, filename, flags); > + bprm =3D alloc_bprm(fd, filename, argv, flags); > if (IS_ERR(bprm)) { > retval =3D PTR_ERR(bprm); > goto out_ret; > @@ -2034,6 +2047,7 @@ int kernel_execve(const char *kernel_filename, > struct linux_binprm *bprm; > int fd =3D AT_FDCWD; > int retval; > + struct user_arg_ptr user_argv =3D {}; >=20=20 > /* It is non-sense for kernel threads to call execve */ > if (WARN_ON_ONCE(current->flags & PF_KTHREAD)) > @@ -2043,7 +2057,7 @@ int kernel_execve(const char *kernel_filename, > if (IS_ERR(filename)) > return PTR_ERR(filename); >=20=20 > - bprm =3D alloc_bprm(fd, filename, 0); > + bprm =3D alloc_bprm(fd, filename, user_argv, 0); > if (IS_ERR(bprm)) { > retval =3D PTR_ERR(bprm); > goto out_ret; > diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h > index 87e2dec79fea..7178d1e4a3de 100644 > --- a/include/uapi/linux/fcntl.h > +++ b/include/uapi/linux/fcntl.h > @@ -100,7 +100,8 @@ > /* Reserved for per-syscall flags 0xff. */ > #define AT_SYMLINK_NOFOLLOW 0x100 /* Do not follow symbolic > links. */ > -/* Reserved for per-syscall flags 0x200 */ > +#define AT_EXEC_REASONABLE_COMM 0x200 /* Use argv[0] for comm in > + execveat */ > #define AT_SYMLINK_FOLLOW 0x400 /* Follow symbolic links. */ > #define AT_NO_AUTOMOUNT 0x800 /* Suppress terminal automount > traversal. */ > > base-commit: baeb9a7d8b60b021d907127509c44507539c15e5