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 3D460D18153 for ; Mon, 14 Oct 2024 21:13:39 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C30486B0085; Mon, 14 Oct 2024 17:13:38 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BE12F6B0088; Mon, 14 Oct 2024 17:13:38 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AD01A6B0089; Mon, 14 Oct 2024 17:13:38 -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 8FDCE6B0085 for ; Mon, 14 Oct 2024 17:13:38 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id BEC7314128C for ; Mon, 14 Oct 2024 21:13:29 +0000 (UTC) X-FDA: 82673459148.05.4814AF2 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf30.hostedemail.com (Postfix) with ESMTP id E88A08000D for ; Mon, 14 Oct 2024 21:13:23 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=cnIDC6Rl; spf=pass (imf30.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=1728940273; 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=M3lQ0KYOqpbftsqLlGfd6zYCT2XftyMCGH11qcJjy28=; b=j0l8/lRFSjTDiZzLMfUz2vnb/u3ghbfvSCwgu87unxSchpmymrC7E0dlRlE5r16hJYOS9v O79I70oD/BtKYr/ThNQNAZhO2yXuZjBshIrihna1toSMF21/1Jom60LUWK96SYR+u8kXpj SuVjBWoclDR27BsELC6mtiM0CTVBAUs= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1728940273; a=rsa-sha256; cv=none; b=1ldnA4xm4E5zitfP65yknjM8nXbe78b3BhgzkdiblgEpBkhjuoqWijRkfo6ZmaBqXQ3cXq +mBJX7ANlU7LMBRizbZFxJFDh3X/A1tA0DbgNdoTVZ3heKgp8IN9H/187ma5fwoxIidRZR 9KaTrYEr9DmsNJ1oToQPME2qVEl6az8= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=cnIDC6Rl; spf=pass (imf30.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 C6E3A5C5898; Mon, 14 Oct 2024 21:13:30 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id D4241C4CEC3; Mon, 14 Oct 2024 21:13:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1728940414; bh=hZ4sZBjojl8AVeRz94L+xmF0lc4B91BT4wKsw0/97uA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=cnIDC6Rlf5QyxMldNmF2WIuElwqTChk8cYTloV+tjUtl5nDqPp19DzMqQvX/TWvJc B3QDF8U9SpCJBVSu41XFqJxVPIbF60GgJuE+YL+HaLmLH4BJhNDuFTW0WJ0WXZpqLm JzDRd1qJ8W8lQHwWNFuNL/j41jajAJ0LlVKqJytcijLVnejYjMIs1hgH2KRRXNGL0L C9xwhpi2cpBP5gH4ffpM3EDu2KWy6iOpO5dfmcEH1rfl7wN4hKlTOchacQUoL4BJZ7 7StJVQ6ha8eECsf6mh0PKJr590C1cOpsJm8mw2jC9DS7fk5QqP89Vu55nbI+0/rOca iNlqkbt2pgjvw== Date: Mon, 14 Oct 2024 14:13:32 -0700 From: Kees Cook To: Tycho Andersen Cc: Zbigniew =?utf-8?Q?J=C4=99drzejewski-Szmek?= , "Eric W. Biederman" , Alexander Viro , Christian Brauner , Jan Kara , Jeff Layton , Chuck Lever , Alexander Aring , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Tycho Andersen , Aleksa Sarai Subject: Re: [RFC] exec: add a flag for "reasonable" execveat() comm Message-ID: <202410141403.D8B6671@keescook> References: <20240924141001.116584-1-tycho@tycho.pizza> <87msjx9ciw.fsf@email.froward.int.ebiederm.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: E88A08000D X-Stat-Signature: m5dffe5z46x68oi414f9uyh6djrux5u3 X-HE-Tag: 1728940403-355936 X-HE-Meta: U2FsdGVkX1+vjQJDpEG/sjgby64QD0onKXs7i7rd51E+qHIM+g9Yq7yvxoJ+luGkBW0+O/42+o/QLko63J54wkvQef9PHkX3QCk+gORT/P7f+AZqbd/A1a0ojM1u1P/uOQokCtGs8ai8tqs2V6yO/glEpQrdhtLqvWlKW8mzGV3e3emWcWpr00nQMETMPjmufubCMOjpIwZmLIHx0ITQg4Duu5xcKzUl+/LUU9myj39qQ62Iaxl9xwD9rKVpSyQfmo5mhOZU70vnPa0JZ6PqmyNkFyzTZuiW9B0L6Hl44S3unrSUsgkjYYkD4PQEU0ufqbSWpDTxLhRfn/MR07zenKsUOwRKxG9tEy/je0KLF+2UY1+P1YT7hOJxrkYxAAkXDjyj93RBZPBPk0nVE8YPxXCu0WivWmlTIfkCyX8lqcIQ6ViCgo8lUMr4i5nM7/VbqlfRiCr0Fe7W01Nmwt08OABSXIPos3OStB+tCoVCxeEAe9JgMPFEMML6CzkAd8hCBJheGm3/bxt5l9ky5cTIDTmaiFK0a7F6vgrPuZpFAYb2biWJ7cfMBaLbh0w2bf+gnJIdL8XMS8apV67pKI53Cgg00imCzREaTXgWjq7G2WbxiOYEvtvzC5/vzjWpdKV31uNsXSeLW6y1+EYDfJZDJuugAAFb8mpj6Sc57wOGYnNooXAms3W7TOU4cPYiEXIaaj6AsBymE3QMLspsSmZgMD4f3b69u3nb6rdDl0UXJj7ggTcfDDc3BHH2ouyfteyG66Kad3AMc9oIy0CdNBgSawVQvdQWlZoQvyrV55l0c+qc0p+Hb0PZrgChBkrD+OY+yQC4uHhoeCvraMoWgH21DFRLVag+MH0TVeQcjmVKgOELrOGqm6GOVASsYQo+i2+DRF1dGIvrA8W+tzaKUbnRnAKrBFMUFgpuszBwrlYHdx2GQvzfF+ZvJcB8jK+zZ1RctgAb6mIvUNL/eSYfoT1 kNEStMMA aPXPcmbKizm0Odz5YenE9PkvWU7leN4H1/N7lVizMUywBXaIcTWNnS/TvXjFT69zqo7xqdwnqxfGDcg+YKIK1wsE/eTBmx3WXbNWUuiArvqHWMqn8R4ooBgRpPrOw5VYkieQJxTHv/5/CTo/pMmjfp/mGoLwMQ19SpyD+80cU1auGykLXHaWjdsmWWig8CrvKGvS5LXL5JiJ51Qak2Ge1rkdJuZIfWHZGKIziCcr24BR4+Wkx4aywJo6AliybXkx0JRE25sQM3AKSHxf5V086irD0U2MZAbLP+oAVNsz/wSJ+IU4NrefyJTzkZKtRD0xwicioSFxp/t+izTeoebbQKtM0fNcADlZVV3NQhNb2P8PS5fl81RUbJDW7Mw== 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 Wed, Oct 09, 2024 at 08:41:31AM -0600, Tycho Andersen wrote: > On Wed, Oct 02, 2024 at 02:34:43PM +0000, Zbigniew Jędrzejewski-Szmek wrote: > > On Tue, Sep 24, 2024 at 12:39:35PM -0500, Eric W. Biederman wrote: > > > 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. > > > > I tried this version (with a local modification to drop the flag and > > enable the new codepath if get_user_arg_ptr(argv, 0) returns nonnull > > as suggested later in the thread), and it seems to work as expected. > > In particular, 'pgrep' finds for the original name in case of > > symlinks. > > Here is a version that only affects /proc/pid/comm, without a flag. We > still have to do the dance of keeping the user argv0 before actually > doing __set_task_comm(), since we want to surface the resulting fault > if people pass a bad argv0. Thoughts? > > Tycho > > > > diff --git a/fs/exec.c b/fs/exec.c > index dad402d55681..61de8a71f316 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -1416,7 +1416,16 @@ int begin_new_exec(struct linux_binprm * bprm) > set_dumpable(current->mm, SUID_DUMP_USER); > > 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->argv0) > + __set_task_comm(me, kbasename(bprm->argv0), true); > + else > + __set_task_comm(me, kbasename(bprm->filename), true); This isn't checking fdpath? > > /* An exec changes our domain. We are no longer part of the thread > group */ > @@ -1566,9 +1575,30 @@ static void free_bprm(struct linux_binprm *bprm) > if (bprm->interp != bprm->filename) > kfree(bprm->interp); > kfree(bprm->fdpath); > + kfree(bprm->argv0); > kfree(bprm); > } > > +static int bprm_add_fixup_comm(struct linux_binprm *bprm, struct user_arg_ptr argv) > +{ > + const char __user *p = get_user_arg_ptr(argv, 0); > + > + /* > + * In keeping with the logic in do_execveat_common(), we say p == NULL > + * => "" for comm. > + */ > + if (!p) { > + bprm->argv0 = kstrdup("", GFP_KERNEL); > + return 0; > + } > + > + bprm->argv0 = strndup_user(p, MAX_ARG_STRLEN); > + if (bprm->argv0) > + return 0; > + > + return -EFAULT; > +} I'd rather this logic got done in copy_strings() and to avoid duplicating a copy for all exec users. I think it should be possible to just do this, to find the __user char *: diff --git a/fs/exec.c b/fs/exec.c index 77364806b48d..e12fd706f577 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -642,6 +642,8 @@ static int copy_strings(int argc, struct user_arg_ptr argv, goto out; } } + if (argc == 0) + bprm->argv0 = str; } ret = 0; out: Once we get to begin_new_exec(), only if we need to do the work (fdpath set), then we can do the strndup_user() instead of making every exec hold a copy regardless of whether it will be needed. -Kees > + > static struct linux_binprm *alloc_bprm(int fd, struct filename *filename, int flags) > { > struct linux_binprm *bprm; > @@ -1975,6 +2005,10 @@ static int do_execveat_common(int fd, struct filename *filename, > goto out_ret; > } > > + retval = bprm_add_fixup_comm(bprm, argv); > + if (retval != 0) > + goto out_free; > + > retval = count(argv, MAX_ARG_STRINGS); > if (retval == 0) > pr_warn_once("process '%s' launched '%s' with NULL argv: empty string added\n", > diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h > index e6c00e860951..0cd1f2d0e8c6 100644 > --- a/include/linux/binfmts.h > +++ b/include/linux/binfmts.h > @@ -55,6 +55,7 @@ struct linux_binprm { > of the time same as filename, but could be > different for binfmt_{misc,script} */ > const char *fdpath; /* generated filename for execveat */ > + const char *argv0; /* argv0 from execveat */ > unsigned interp_flags; > int execfd; /* File descriptor of the executable */ > unsigned long loader, exec; -- Kees Cook