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 E8A7EC369A8 for ; Wed, 25 Sep 2024 08:31:15 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 81A316B007B; Wed, 25 Sep 2024 04:31:15 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7CA0F6B0082; Wed, 25 Sep 2024 04:31:15 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6912B6B0088; Wed, 25 Sep 2024 04:31:15 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 4A3E16B007B for ; Wed, 25 Sep 2024 04:31:15 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id C146280130 for ; Wed, 25 Sep 2024 08:31:14 +0000 (UTC) X-FDA: 82602590868.10.B48264D Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf13.hostedemail.com (Postfix) with ESMTP id 0FD162000E for ; Wed, 25 Sep 2024 08:31:10 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=ceByRlbM; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf13.hostedemail.com: domain of brauner@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=brauner@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1727252982; a=rsa-sha256; cv=none; b=161O4Pvm6PAgRd1x3bJtbqlvYuuhtcXpfNk9YBsXppXenIA61mmQ6ITgsA3FPhSF52bEuK yrpQSl43Tw0u1R5waw/oMMMHUlUfcNFtVyXf8YkO00tuuUy3WFNnJkDt5R5xOivmD+X0n6 OVfl3rgve4joeqyRxMkfoKnjhq02zCk= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=ceByRlbM; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf13.hostedemail.com: domain of brauner@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=brauner@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1727252982; 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:dkim-signature; bh=KaBFErfbhFKV8ilHW7NWTz/JBOv30u582tabOParcnk=; b=FDraBN8r9oPi+xfQzVvvW3UfBmnfpJZ70xQAc0pbzA41vhJyDDzSuC1TDh7a6l8j51S2FM eBJcPK+EX1FVhtHsofCXU3hytxbnGluL8KY4AWgiVwizwAH/MpUOxZ5bi5KQ5sQMJVpBGt MEv17HfkekkWBkIvARvTJ65hmU+RmyU= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 44AC55C06F5; Wed, 25 Sep 2024 08:31:06 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id CFF64C4CEC3; Wed, 25 Sep 2024 08:31:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1727253069; bh=HZAomvSklThtRAOmnplclgrQHIBXha+XYRV1StbeV9c=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=ceByRlbMy2nybxNP5/Fxc9GTkzweijxAkfovrht8aYhHVaV7S8ghpODKEjZ5Rxv1/ Pt84bTCynTvqXYp9BKxEHYC2wybbCh7k6KXrmSz3zMQH+3oFRA47IMrQbl+io/t6DX OeLG1SnEnlXDutSILoqWbPshrdxXPbAsAcLaBpj1NbqH8NXaqr6pD1egqN0RPQLg63 fbb38Ttiu47PzAs12sO3rd6tlTKxdhGPR/8JC4FygBCMpzeizx7LzTubOVlxaPf9PI D4D63FfvWvpA/9UcBgD7XAIz0VkxotecGMq7AfykQpwxTbbrkIZ1PDCCO0YFvkNciK fdj5JQnn7blYQ== Date: Wed, 25 Sep 2024 10:31:04 +0200 From: Christian Brauner To: Tycho Andersen Cc: Alexander Viro , Jan Kara , Eric Biederman , 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 Subject: Re: [RFC] exec: add a flag for "reasonable" execveat() comm Message-ID: <20240925-herziehen-unerbittlich-23c5845fed06@brauner> References: <20240924141001.116584-1-tycho@tycho.pizza> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20240924141001.116584-1-tycho@tycho.pizza> X-Rspamd-Queue-Id: 0FD162000E X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: 1yj1mu5ksbagkq91ad3dmibyfid87wyf X-HE-Tag: 1727253070-644706 X-HE-Meta: U2FsdGVkX18CZmVm+d+42UPJqqi+Al8IyNwwJTKy73xAfq9l2xMmg3Qj95kDqk0G2iUaLBAH/Dnzsb+yomZtxW8V+uOGMYks2Z7p3pxF8P6I+LltQ4nT7QwazUh0M9aeNfdOC+BT/tbtzMYM23ecibT8qvQe62t0rNLPlXRCRgzOFb6zkGJNVyoBIfZl1ZzwJ8RFPEyMjS1qoVp7fPj+fP/IFNx16Md9aEbfkTfqSndNwmPt0RWHafGBmVF76gljkRTybRZsaKn+4dvMkKMZ27zfHRmHlMSkFSmzyoY4CWR4LLpm/+zOnrvhN+Hv91N+GbAtsgAOHEQJT3YPhJlv258nlqS7cJbQayREqoIaWzgVfLCB+zyITv54ZnGX2rQk224wA+4/1QLlNubMaUyznVw9WCwCo+ydkTUnN5l2OFbr/evPcy5O3PjCkFeG6pGdjq+1NHC2m2C+ZTbEH+d/sa2CHYkGkgAKm/2IzbuU+Ukc355/7Th54cJ1frRgwlKCYqDCTreEz3JyP9w/XhyJfTzY4OBJVoXJYCEeDyxKdE6QyEqGsd2J4iPDu5PWAnQrnJhPI7QOTfHXGwAiC4uEi+CLZbU9QwewI4cEdygMUA9ZDN7chVxqA4PnwsEUh4WoZc9n1Xp3c1WaQH/XurYei/PEY9QAy1AI+pzALZuSn4oCWcHxwUqad53v2Ju4BtVQ/2qxdJ6oauaddS9LAEVGYxsaTqUSXXUBoyjE+8yejLS9WJ1SsxdCUx2rh8Ngvb5tftx7ngYdH3BzbVaHotqkwbdslsFAnKDZ5UQ8/IZw0Jdxhz5T7/9BOI8NY0QIyYea27ZYpTFx3TLnbRRJF9xqUrHxODYGfNxRNc/xw2DBa09SCiqyLAhmdk0Bkp5phicVaAAKFRARBJMl4gp92VZqW0v5g6eTPsGLi+G62JHFgFV8Es+iKQ5TqLIJq28qKhrwtQwDiAYHc2yv1aiQuzy WcdBHTVw zPCPI5rg1AifcPanrbHA4OxZknkfiM5/G4Yy0wAczFvrnmrnrSZ9V8Uow+6m944qLGNZ0EzKIiPsR0qiyEdCKUM+SOLyKwK+UgpbZVyb8zBdv1gK3YyGpsXZzL1pyKu+sNxA82mweFdHnPYclwJfKI7Z34MOvinvaDdpLOYN5933+5FO2P0yGOlopSAcOIyADFOHP7jQrQRTK+4LvxifpBtaCVxHMxqRKx9FE3fA3z7vqJNOKUPIlQ8tELe2AAuV2C+0mNCxdzWQMhOAS7iC9/fEMOW8ITqCDBjnxIKse1o2kV7R+MiPGYl4lKnBgYSbBXsoooOgU+au13urXj3vp3hz7ubmN3FsXCxXov9dIDy6dJoXWT4BZrDAh75PblMdT4kkZCL/R9QGfPZJ6S7b1kn+iiqWJ5JTfn2kfsgV7LA5mGf001r6OjR7lmW2PSwHC5Tu8gLSvvvg5Sq2PnAZiIAVWxBYECZAh6lna2u3Bt5LNLbo/NTWgwraFafw4avybfnsb 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 Tue, Sep 24, 2024 at 08:10:01AM GMT, Tycho Andersen wrote: > 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. > > Signed-off-by: Tycho Andersen > Suggested-by: Zbigniew Jędrzejewski-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 not > 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 because > 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 = 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 relative */ > #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 = argc > 1; > > pid = fork(); > if (pid < 0) { > perror("fork"); > exit(1); > } > > if (pid == 0) { > int fd; > long ret, flags; > > fd = open("./catprocselfcomm", O_PATH); > if (fd < 0) { > perror("open catprocselfname"); > exit(1); > } > > flags = AT_EMPTY_PATH; > if (wants_reasonable_comm) > flags |= AT_EXEC_REASONABLE_COMM; > syscall(__NR_execveat, fd, "", (char *[]){"./catprocselfcomm", NULL}, NULL, flags); Yes, that one is the actually palatable solution that I mentioned during the session and not the questionable version where the path argument is overloaded by the flag. Please add a: Link: https://github.com/uapi-group/kernel-features#set-comm-field-before-exec to the commit where this originated from. > fprintf(stderr, "execveat failed %d\n", errno); > exit(1); > } > > if (waitpid(pid, &status, 0) != pid) { > fprintf(stderr, "wrong child\n"); > exit(1); > } > > if (!WIFEXITED(status)) { > fprintf(stderr, "exit status %x\n", status); > exit(1); > } > > if (WEXITSTATUS(status) != 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); > } > > -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 = -ENOMEM; > + bool needs_comm_fixup = flags & AT_EXEC_REASONABLE_COMM; > + > + flags &= ~AT_EXEC_REASONABLE_COMM; > > file = do_open_execat(fd, filename, flags); > if (IS_ERR(file)) > @@ -1590,11 +1594,20 @@ static struct linux_binprm *alloc_bprm(int fd, struct filename *filename, int fl > if (fd == AT_FDCWD || filename->name[0] == '/') { > bprm->filename = filename->name; > } else { > - if (filename->name[0] == '\0') > + if (needs_comm_fixup) { > + const char __user *p = get_user_arg_ptr(argv, 0); > + > + retval = -EFAULT; > + if (!p) > + goto out_free; > + > + bprm->fdpath = strndup_user(p, MAX_ARG_STRLEN); > + } else if (filename->name[0] == '\0') > bprm->fdpath = kasprintf(GFP_KERNEL, "/dev/fd/%d", fd); > else > bprm->fdpath = kasprintf(GFP_KERNEL, "/dev/fd/%d/%s", > fd, filename->name); > + retval = -ENOMEM; > if (!bprm->fdpath) > goto out_free; > > @@ -1969,7 +1982,7 @@ static int do_execveat_common(int fd, struct filename *filename, > * further execve() calls fail. */ > current->flags &= ~PF_NPROC_EXCEEDED; > > - bprm = alloc_bprm(fd, filename, flags); > + bprm = alloc_bprm(fd, filename, argv, flags); > if (IS_ERR(bprm)) { > retval = PTR_ERR(bprm); > goto out_ret; > @@ -2034,6 +2047,7 @@ int kernel_execve(const char *kernel_filename, > struct linux_binprm *bprm; > int fd = AT_FDCWD; > int retval; > + struct user_arg_ptr user_argv = {}; > > /* 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); > > - bprm = alloc_bprm(fd, filename, 0); > + bprm = alloc_bprm(fd, filename, user_argv, 0); > if (IS_ERR(bprm)) { > retval = 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 > -- > 2.34.1 >