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 9378DC3ABC3 for ; Tue, 13 May 2025 22:17:31 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E9F646B00D7; Tue, 13 May 2025 18:17:29 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E4E356B00D8; Tue, 13 May 2025 18:17:29 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CF2616B00D9; Tue, 13 May 2025 18:17:29 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id AF6106B00D7 for ; Tue, 13 May 2025 18:17:29 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 57EF380AFC for ; Tue, 13 May 2025 22:17:30 +0000 (UTC) X-FDA: 83439297060.10.87C0D7A Received: from out03.mta.xmission.com (out03.mta.xmission.com [166.70.13.233]) by imf21.hostedemail.com (Postfix) with ESMTP id ED1651C0002 for ; Tue, 13 May 2025 22:17:27 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=none; spf=pass (imf21.hostedemail.com: domain of ebiederm@xmission.com designates 166.70.13.233 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=1747174648; 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=FenSnESJlzLM9/ZhMLNocsKbc8rhoGPS3Ubz2BMa5IA=; b=uLZa8wumWn1DE7Hf1FSncGoTlOkwsiYwx3C8oLVTLpDHz3utHZSjBiUJtPv4ysrd2yDorv 8B5VV0TyAJF0TFsrzBhDD5X2c/kz6i6q9hd/zUXbYpw79zOTh6xWVxmQZnnRrdh+ucapk5 5d1eKDCcAFdSIPddWdRLLmrSzLmwPq8= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=none; spf=pass (imf21.hostedemail.com: domain of ebiederm@xmission.com designates 166.70.13.233 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=1747174648; a=rsa-sha256; cv=none; b=vbxE6KiGYcGuhx0OXBQYs9o9Tyi/Ms81EMdOA+oLG4zy2xnuGtO/2O8p5eTRjLhy32AcCn oXzT0e88E3X/z2tfYspfdAlrrzUjmM4utu9SITjyMABgjMOTnXSw7h1DqoW4TYI0FK3X8L xBtY3mTM9iVa0cX47FVlOY6b/m/NSdY= Received: from in01.mta.xmission.com ([166.70.13.51]:44070) by out03.mta.xmission.com with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1uExwB-005sqC-Le; Tue, 13 May 2025 16:17:23 -0600 Received: from ip72-198-198-28.om.om.cox.net ([72.198.198.28]:34512 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 1uExwA-004yCk-FO; Tue, 13 May 2025 16:17:23 -0600 From: "Eric W. Biederman" To: Jann Horn Cc: Kees Cook , Mateusz Guzik , Kees Cook , Christian Brauner , Jorge Merlino , Alexander Viro , Thomas Gleixner , Andy Lutomirski , Sebastian Andrzej Siewior , Andrew Morton , linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, John Johansen , Paul Moore , James Morris , "Serge E. Hallyn" , Stephen Smalley , Eric Paris , Richard Haines , Casey Schaufler , Xin Long , "David S. Miller" , Todd Kjos , Ondrej Mosnacek , Prashanth Prahlad , Micah Morton , Fenghua Yu , Andrei Vagin , linux-kernel@vger.kernel.org, apparmor@lists.ubuntu.com, linux-security-module@vger.kernel.org, selinux@vger.kernel.org, linux-hardening@vger.kernel.org, oleg@redhat.com References: <20221006082735.1321612-1-keescook@chromium.org> <20221006082735.1321612-2-keescook@chromium.org> <20221006090506.paqjf537cox7lqrq@wittgenstein> <86CE201B-5632-4BB7-BCF6-7CB2C2895409@chromium.org> Date: Tue, 13 May 2025 17:16:49 -0500 In-Reply-To: (Jann Horn's message of "Tue, 13 May 2025 23:09:48 +0200") Message-ID: <871pss17hq.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=1uExwA-004yCk-FO;;;mid=<871pss17hq.fsf@email.froward.int.ebiederm.org>;;;hst=in01.mta.xmission.com;;;ip=72.198.198.28;;;frm=ebiederm@xmission.com;;;spf=pass X-XM-AID: U2FsdGVkX19MKpBmCOdib17JedfBy1DoBc7s2t+3dns= Subject: Re: [PATCH 1/2] fs/exec: Explicitly unshare fs_struct on exec X-SA-Exim-Connect-IP: 166.70.13.51 X-SA-Exim-Rcpt-To: too long (recipient list exceeded maximum allowed size of 512 bytes) X-SA-Exim-Mail-From: ebiederm@xmission.com X-SA-Exim-Scanned: No (on out03.mta.xmission.com); SAEximRunCond expanded to false X-Rspam-User: X-Rspamd-Queue-Id: ED1651C0002 X-Rspamd-Server: rspam09 X-Stat-Signature: 84z99kydre5yk8ouzjg41huwmorchr8s X-HE-Tag: 1747174647-893596 X-HE-Meta: U2FsdGVkX1/tDQNp1bHa0apnO0oeN4XF/w7SpEU1mqTR9ngc54zGSvatw8q/6NiNzOp+aFz+JjRGGa3C+DvnHAMVyR5OMj9nt7pTnE4SESZ/I8D80L4MjjHBzWDpypPEZYEEe+Y3Fz6ICgF/pqOwypbdg/0hgWvDdkDS7aMAxA1NQbO3YXXdlFASS1bo2hOJOQ9SYA477SjsR0qr0iF9TpjKoI3g0UwC4nWXeCn9s1sn1WxXOE0qaEzPg+qvexOBZSoVr/tPuwnUWo1RXfXQ6Rxzw16+XHup+6J47+4+0MFiCaxjADzaeLVz50FZ9ITPQqy+Eta0N4Ni38KllVT9riciccelorrMXABtazFccHtv5/EyDKZSGjKPOK7tfA/oW2k5a4Exe+7wD98Y1g/96K5FJ3kkJdvuMhCabRWfyES9YYb3SuDl0hBLwgJ6G8JpXw7idO8U7c5C9XmwjdJIWiKbxX55wSddJvx+5fpbp33mPIlH9ns0qgou9GeuwdozOHkSUIenfhDs4+tZBi1jmdtNugBeOIwtdcKMPnhTNeNSuARYP1jizbzLrZOxZpCDjdM6w+VKdCg8JR4W4XtJuDeEcDeajcPrxnOe+w18vNdor0O6nSClnQmHjmlxr4R3O3IBu0BbHPvlcN11w/uASmAaDXvd/xlDja8IEzAF6lsV8uH0KHfj6v562hvokzUJnb2l8ZvjjBDaXZMEM3G0wMwlbuoOKf3G7uQFKYztoc4atUbajQTEBwWCi1EPj1a4L9xJNCHnCJFPZPhNkhCgetJMwae35gs388mSGFlScbsKo7nxlP0OT7IM7C0vx4mUEY8ly18PVzgQhIULwiVcuyV4OjBzYwqNhw04eSYIg/MaoQDdMmC/MRb80hFoZSSchFPsTN5Oe8vTRn5adJTt1J0grgzqdf/vXFcQjDJt4LySJYYQiU2DZQgDcQvpiv9FfIqXL3T/5biQGOp7dRH N6jjEt5j OOobKgcZLzZe+Wfb37u1bvEJuDdTzbL8IhT7d4dVoddHiirB3KoSJ2+CkolKj3co/RgPJAIizvo27VQDSJGOfRdwY7AQNcJhu7ZbdcedvAPszURICYj4qhy5wWKHVRnkjAaVJUd1aiZRQ/zz1CJtqcNGX1KeLCD8xB5ulwgwXpIDvE0nAWBv8Q6gVZheEABJT5YYv0lk6s+GFHqtADeJMDbjV/5tF5DccWUy7AtDkFKP5Jhb5pqL6xepcvRcndS2OoR4I 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: Jann Horn writes: > On Tue, May 13, 2025 at 10:57=E2=80=AFPM Kees Cook wrot= e: >> On May 13, 2025 6:05:45 AM PDT, Mateusz Guzik wrote: >> >Here is my proposal: *deny* exec of suid/sgid binaries if fs_struct is >> >shared. This will have to be checked for after the execing proc becomes >> >single-threaded ofc. >> >> Unfortunately the above Chrome helper is setuid and uses CLONE_FS. > > Chrome first launches a setuid helper, and then the setuid helper does > CLONE_FS. Mateusz's proposal would not impact this usecase. > > Mateusz is proposing to block the case where a process first does > CLONE_FS, and *then* one of the processes sharing the fs_struct does a > setuid execve(). Linux already downgrades such an execve() to be > non-setuid, which probably means anyone trying to do this will get > hard-to-understand problems. Mateusz' proposal would just turn this > hard-to-debug edgecase, which already doesn't really work, into a > clean error; I think that is a nice improvement even just from the > UAPI standpoint. > > If this change makes it possible to clean up the kernel code a bit, even = better. What has brought this to everyone's attention just now? This is the second mention of this code path I have seen this week. AKA: security/commoncap.c:cap_bprm_creds_from_file(...) > ... > /* Don't let someone trace a set[ug]id/setpcap binary with the revised > * credentials unless they have the appropriate permit. > * > * In addition, if NO_NEW_PRIVS, then ensure we get no new privs. > */ > is_setid =3D __is_setuid(new, old) || __is_setgid(new, old); >=20 > if ((is_setid || __cap_gained(permitted, new, old)) && > ((bprm->unsafe & ~LSM_UNSAFE_PTRACE) || > !ptracer_capable(current, new->user_ns))) { > /* downgrade; they get no more than they had, and maybe less */ > if (!ns_capable(new->user_ns, CAP_SETUID) || > (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS)) { > new->euid =3D new->uid; > new->egid =3D new->gid; > } > new->cap_permitted =3D cap_intersect(new->cap_permitted, > old->cap_permitted); > } The actual downgrade is because a ptrace'd executable also takes this path. I have seen it argued rather forcefully that continuing rather than simply failing seems better in the ptrace case. In general I think it can be said this policy is "safe". AKA we don't let a shared fs struct confuse privileged applications. So nothing to panic about. It looks like most of the lsm's also test bprm->unsafe. So I imagine someone could very carefully separate the non-ptrace case from the ptrace case but *shrug*. Perhaps: if ((is_setid || __cap_gained(permitted, new_old)) && ((bprm->unsafe & ~LSM_UNSAFE_PTRACE) || !ptracer_capable(current, new->user_ns))) { + if (!(bprm->unsafe & LSM_UNSAFE_PTRACE)) { + return -EPERM; + } /* downgrade; they get no more than they had, and maybe less */ if (!ns_capable(new->user_ns, CAP_SETUID) || (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS)) { new->euid =3D new->uid; new->egid =3D new->gid; } new->cap_permitted =3D cap_intersect(new->cap_permitted, old->cap_permitted); } If that is what you want that doesn't look to scary. I don't think it simplifies anything about fs->in_exec. As fs->in_exec is set when the processing calling exec is the only process that owns the fs_struct. With fs->in_exec just being a flag that doesn't allow another thread to call fork and start sharing the fs_struct during exec. *Shrug* I don't see why anyone would care. It is just a very silly corner case. Eric