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 499B7D3C530 for ; Thu, 17 Oct 2024 20:38:23 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C25046B0083; Thu, 17 Oct 2024 16:38:22 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BD46D6B0085; Thu, 17 Oct 2024 16:38:22 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A9C276B0088; Thu, 17 Oct 2024 16:38:22 -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 8CE976B0083 for ; Thu, 17 Oct 2024 16:38:22 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 74CB21616E9 for ; Thu, 17 Oct 2024 20:38:09 +0000 (UTC) X-FDA: 82684256718.14.F25D4F8 Received: from fhigh-a1-smtp.messagingengine.com (fhigh-a1-smtp.messagingengine.com [103.168.172.152]) by imf23.hostedemail.com (Postfix) with ESMTP id 54D8F140015 for ; Thu, 17 Oct 2024 20:38:13 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=tycho.pizza header.s=fm1 header.b=THj0SaVX; dkim=pass header.d=messagingengine.com header.s=fm2 header.b=eeTo40iK; dmarc=none; spf=pass (imf23.hostedemail.com: domain of tycho@tycho.pizza designates 103.168.172.152 as permitted sender) smtp.mailfrom=tycho@tycho.pizza ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1729197340; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=qto/GT0HoXk1tj2IzlxYMkHo58TNLRqQ9QyZleoivb8=; b=nFkeV68LXR/UjLAVzLj2IO8Xy5cTinF6UIT507Zq4Y+FhKb6J5U87m4NbGJzV1EdYKq9wW QF3UCQ7vtMVe3tYLgk/xAxESlLvKoc+Squ1f3Ckcy/+k2Yze35WPeZCnuSGV/sfFphYaGG oOFiF7tLnaRk/R2712fMb76u8tx0yWA= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1729197340; a=rsa-sha256; cv=none; b=caVe+BfudOTgjDD1ZjkC3NiWI45iHBPu2vfkNc/pwP+tZFqtZ0rGJeUsukgQFkbkTYDTqU k/+iZ99hZ+YqdxP1a4PiW/9hBdwgw9Buobj00pHWR1A+2aWIH/Cod9AfRYv1435Wyot6TB fe25jUm+WahrBGAm/QKQZDrr/TX+SqY= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=tycho.pizza header.s=fm1 header.b=THj0SaVX; dkim=pass header.d=messagingengine.com header.s=fm2 header.b=eeTo40iK; dmarc=none; spf=pass (imf23.hostedemail.com: domain of tycho@tycho.pizza designates 103.168.172.152 as permitted sender) smtp.mailfrom=tycho@tycho.pizza Received: from phl-compute-10.internal (phl-compute-10.phl.internal [10.202.2.50]) by mailfhigh.phl.internal (Postfix) with ESMTP id F23FD114016A; Thu, 17 Oct 2024 16:38:18 -0400 (EDT) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-10.internal (MEProxy); Thu, 17 Oct 2024 16:38:18 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tycho.pizza; h= cc:cc:content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm1; t=1729197498; x=1729283898; bh=qto/GT0HoX k1tj2IzlxYMkHo58TNLRqQ9QyZleoivb8=; b=THj0SaVXGdS/4PZXbPbCmflPXY vCDQQodb/x8pnX1QgEMxHQgklrXeDpCqVuWh0KnU3z3VRc/vv96A9K7q79Osx3hi krqcMmB64h/RD49pYO5YJvavyUrEo5i7fOE2UffX7OW2FVzYj4J5s0YLPAdWfFDx jzEd5Q3fMlG3d8CF9R5pxXOyBE4PCANt2X5guWC1617Y/HuSj0E2KMmnH4oXcxRh 8sZkVCJuXkdFXo60f6DoQDlXpJ1sPeIlqY4wGwMIyDEeqPhkZbse52qdVJWcekM1 VPOSATud+4aWemaO3EbBed2TIEJzxH+aA0WV0S5ALTDN+MVFrCJd9ZthkaGg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm2; t=1729197498; x=1729283898; bh=qto/GT0HoXk1tj2IzlxYMkHo58TN LRqQ9QyZleoivb8=; b=eeTo40iK0JRaW3XGdDv+XFsCho+d2VJmfkM3Qf+dG6sH DvG4WmkqIHIjjmENZymz7RbJIJxD58/epvIFg9vdF/eWuG+SQKZgfF6cpkPndlhK rfhCxFX9RE5y1Oa/kN7ZsIXsl7rjqQK3F29IJ4+OTH+YvJmpa6uVBqeGb9R3+C8J IeglPUQaEozGS6wUfhWgDm4qSHNoq3XCU1z6LP5jOlLwaUw6NWIvQ6W2fiG1jmBC m7J4rKVH4hmuf6lNm8fEOowr759zAX/P95D+kwmQNUlAmCiTAQGs7Cc7o7LBcqxm 3T6UVQMaJTzIz7dO8v6w5TSDAOKVdxclb7/QEKjB7Q== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrvdehuddgudehgecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpggftfghnshhusghstghrihgsvgdp uffrtefokffrpgfnqfghnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivg hnthhsucdlqddutddtmdenucfjughrpeffhffvvefukfhfgggtuggjsehttdertddttddv necuhfhrohhmpefvhigthhhoucetnhguvghrshgvnhcuoehthigthhhosehthigthhhord hpihiiiigrqeenucggtffrrghtthgvrhhnpeeutedttefgjeefffehffffkeejueevieef udelgeejuddtfeffteeklefhleelteenucevlhhushhtvghrufhiiigvpedtnecurfgrrh grmhepmhgrihhlfhhrohhmpehthigthhhosehthigthhhordhpihiiiigrpdhnsggprhgt phhtthhopedugedpmhhouggvpehsmhhtphhouhhtpdhrtghpthhtohepkhgvvghssehkvg hrnhgvlhdrohhrghdprhgtphhtthhopeiisgihshiivghksehinhdrfigrfidrphhlpdhr tghpthhtohepvggsihgvuggvrhhmseigmhhishhsihhonhdrtghomhdprhgtphhtthhope hvihhrohesiigvnhhivhdrlhhinhhugidrohhrghdruhhkpdhrtghpthhtohepsghrrghu nhgvrheskhgvrhhnvghlrdhorhhgpdhrtghpthhtohepjhgrtghksehsuhhsvgdrtgiipd hrtghpthhtohepjhhlrgihthhonheskhgvrhhnvghlrdhorhhgpdhrtghpthhtoheptghh uhgtkhdrlhgvvhgvrhesohhrrggtlhgvrdgtohhmpdhrtghpthhtoheprghlvgigrdgrrh hinhhgsehgmhgrihhlrdgtohhm X-ME-Proxy: Feedback-ID: i21f147d5:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 17 Oct 2024 16:38:15 -0400 (EDT) Date: Thu, 17 Oct 2024 14:38:13 -0600 From: Tycho Andersen To: Kees Cook 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: References: <20240924141001.116584-1-tycho@tycho.pizza> <87msjx9ciw.fsf@email.froward.int.ebiederm.org> <202410141403.D8B6671@keescook> <202410170840.8E974776@keescook> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <202410170840.8E974776@keescook> X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 54D8F140015 X-Stat-Signature: nqfodmx4au8pg7xzj6jddids3x7emc4a X-Rspam-User: X-HE-Tag: 1729197493-940600 X-HE-Meta: U2FsdGVkX1/5S65SKGpiNy6LFehIdidt3KAaez0FLBdZD/mG3rqCnH3xqAhZBLJ3+FZM7HnHb5H7lvJKo+iF1CpWgwu1bJ5QsT8XXDlIx+QqvJ69QQXCJASYekMBujg9uMHQ6vTl7ZeKg7g/EFkroo0rHIyqkS6AeLRy6thWAKTd4dqt2vDzTXjMIgJ65uiki/VMyuYqaDmLmUYtgjJnApAn8emjft4KTE1+Rp0ZA5cbWICV0sm+Qw61/D3yorc+Zj0Dd3zZ0cZQlDi/5bXEFQTtY6o17V+pMMZfCtD5G1fg9s2YD5N5ghYnr7zgl4pfYLqT3VUoTR0JUW7ieQ8pWdQT+edswFUb+2eRdWbybTz1affWBn9QYQCPBwO0/lVO25YFuOZk9cQETnCskAPuPJJi0WqDMWU7TYfowdTMPRtjlzDNxUKgjDcR8jPH9A5Tsqg2a0Z5kFK1z6qE3aRAp8KvztMMpZkneDX1kADj0goRTjwn/YMcr5+6f1cyLaknUcL7PL/XEU2mVJ8ZsxJj33FreLlnDyxXrC1aBHXZCw8S31S8BtJ1cVHiKXCcKiZvYM+HhRLzuw+4B/qapM8aRKFTzVUWETfRQEl+C8b02yMlsO0ndq3d46L04Yg6KmFWOjkXbC+HeJEGQxAeNHVcQgeqde3aaO1F8vcW/lzXYYib415NzI+7PlEhDslSPotI3VY54yP2G6sxJr1cEWv4GL31mQ+6anKJ4XLUu7BM8KoniK+hkU3h1hGldHIae4KGSEuHhapH4fRj4n3hIFZnHTRM0zzR0flkKGz9QcOx224UaPhqdFmwWJYUnbeqfcSCZ+CL/JsNplGzQI8I6xDLd+G/xDdxid8RkryCTMmMK4121MstMoRZEzmQtGUTJp615RMXgOgxTds1aTu6P/eOagUbRNXCEW8UpZm2Lq3cC84uLHPYyTSXlvSOrqHjH8vK0fgF0fMXDvphRJaQThd 6tKSww1X wtjdggRjF4Y4mK1jvlOCoJJDYZ/bVyK0HCHR1y6LTuEoyLTZPRZtrqHoS+TmAh9xfAQOSmcm/W4MPbfZwoKOdkLKNue2SGUje8jvRyMoXQ1M4IZyR/BtkXrcBDCsEe3Py7FYJnEUpv4vFngTJNnDffo5JZNDQ7vfhqbVZJH7lLzc2nIL30XzqOaXqjOSm9Z5Lst4fsxAlGUaLhO8sCQh+dO80+wFH978j8thAy0PrUgatrP2IWN/pTyhguQ5DqBpKYaJjAntfCM1sLHD/OO7AxEkaW54xjw1zil+wDcosl5qhXWlmGZQ8caVfhZ3+GGYqePSLO2+LHt+9Y0FSEzIyyuP1ik44ALPB1FjB0P954DvOhArSINHwufQx0w== 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 Thu, Oct 17, 2024 at 08:47:03AM -0700, Kees Cook wrote: > On Thu, Oct 17, 2024 at 08:34:43AM -0600, Tycho Andersen wrote: > > On Mon, Oct 14, 2024 at 02:13:32PM -0700, Kees Cook wrote: > > > On Wed, Oct 09, 2024 at 08:41:31AM -0600, Tycho Andersen wrote: > > > > +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: > > > > Isn't str here a __user? We want a kernel string for setting comm, so > > I guess kaddr+offset? But that's not mapped any more... > > Yes, but it'll be valid __user addr in the new process. (IIUC) Yes, it's valid, but we need a kernel pointer for __set_task_comm(). > > > 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. > > > > What happens if that allocation fails? begin_new_exec() says it is the > > point of no return, so we would just swallow the exec? Or have > > mysteriously inconsistent behavior? > > If we can't alloc a string in begin_new_exec() we're going to have much > later problems, so yeah, I'm fine with it failing there. Ok, cool. But with your notes below, the allocation will still be before begin_new_execexit(), just only in the cases where we actually need it, hopefully that's okay. > > +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); > > To keep this const but not call get_user_arg_ptr() before the fdpath > check, how about externalizing it. See further below... > > > + > > + /* > > + * If this isn't an execveat(), we don't need to fix up the command. > > + */ > > + if (!bprm->fdpath) > > + return 0; > > + > > + /* > > + * In keeping with the logic in do_execveat_common(), we say p == NULL > > + * => "" for comm. > > + */ > > + if (!p) { > > + bprm->argv0 = kstrdup("", GFP_KERNEL); > > Do we want an empty argv0, though? Shouldn't an empty fall back to > fdpath? Yes, sounds good. > > + return 0; > > + } > > + > > + bprm->argv0 = strndup_user(p, MAX_ARG_STRLEN); > > + if (bprm->argv0) > > + return 0; > > + > > + return -EFAULT; > > +} > > + > > static struct linux_binprm *alloc_bprm(int fd, struct filename *filename, int flags) > > { > > struct linux_binprm *bprm; > > @@ -1975,6 +2011,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; > > How about: > > if (unlikely(bprm->fdpath)) { > retval = bprm_add_fixup_comm(bprm, argv); > if (retval != 0) > goto out_free; > } > > with the fdpath removed from bprm_add_fixup_comm()? Yep, this is much clearer, thanks. I will respin with these as a Real Patch. Tycho