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 88333C3DA4A for ; Fri, 2 Aug 2024 15:59:06 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1807E6B00A6; Fri, 2 Aug 2024 11:59:06 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 12FF06B00A8; Fri, 2 Aug 2024 11:59:06 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F12276B00A9; Fri, 2 Aug 2024 11:59:05 -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 D3A416B00A6 for ; Fri, 2 Aug 2024 11:59:05 -0400 (EDT) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 8C1B51A12C3 for ; Fri, 2 Aug 2024 15:59:05 +0000 (UTC) X-FDA: 82407764250.25.2A9B56A Received: from mail-yw1-f169.google.com (mail-yw1-f169.google.com [209.85.128.169]) by imf27.hostedemail.com (Postfix) with ESMTP id 100D840007 for ; Fri, 2 Aug 2024 15:59:01 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=toxicpanda-com.20230601.gappssmtp.com header.s=20230601 header.b=MT6R5D9O; spf=none (imf27.hostedemail.com: domain of josef@toxicpanda.com has no SPF policy when checking 209.85.128.169) smtp.mailfrom=josef@toxicpanda.com; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1722614313; 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=411U8k9mMC3IZL6s6SMQtlI1/hffpCMVW4TK9o1PCQs=; b=iN3g/2d7klmDx3IBlwGmylZ4W5/ZZAySk7xFUXlW3Okqql8dUaq3gRDn4aeQxyvIisrPdH 1UCIcz/znE4nqmt/XTbNnYCub74hBZXVpoGVxit2OrwRykh4dRepmicF3v/rNHhijpW6h5 +HOE+6JIxchkJxjqeT3vmiOUKGl/pe8= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=toxicpanda-com.20230601.gappssmtp.com header.s=20230601 header.b=MT6R5D9O; spf=none (imf27.hostedemail.com: domain of josef@toxicpanda.com has no SPF policy when checking 209.85.128.169) smtp.mailfrom=josef@toxicpanda.com; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1722614313; a=rsa-sha256; cv=none; b=owtIpwSh6N+zlVDxpHUAjhxvc83uznKVPgTWPaY7RhwUpDH14DBPkTpCVGgoT2b5Z4rU0P j9P29tDULJQQMqN5hYMbf/LEQYSloduDAdvlRqoWe2uVYkNokPgxrx0OVg0RctRa7FJxZA eW+wP0UAjafGp/LePN3cpqWeFzvN8uw= Received: by mail-yw1-f169.google.com with SMTP id 00721157ae682-67682149265so70360377b3.2 for ; Fri, 02 Aug 2024 08:59:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=toxicpanda-com.20230601.gappssmtp.com; s=20230601; t=1722614341; x=1723219141; darn=kvack.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=411U8k9mMC3IZL6s6SMQtlI1/hffpCMVW4TK9o1PCQs=; b=MT6R5D9OE9Ak9oCXHu6Efkv7TQhdaNm7Sf4PqrHTsJD3o0jeKpxK8dbXgRuc/AQF0O /nRB3YwO7QLFRtHWOgaa50Jecp6fbKionDQB1dofMRehcTOJmGCQhivHXluyJYeVPE4g wxvf8SP6k04IUx+YyPHdprWwywXxmJuFtsDR7zB3BPSG6UeC6UNq9UuXtpthsMfBfdO5 DC3fvQzXKPDdnt1evkqovpsKZz1Sw7YZqG0ifZsP0Xl6L6SP5WRWkH83xfZfFR7eNGJZ Ci5hJFR78sPcElY7dWEm0CESV7gtSQPOtCu1tNnd2ya9F8mFFbN16DZc9QEHOYXLavPY cinw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722614341; x=1723219141; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=411U8k9mMC3IZL6s6SMQtlI1/hffpCMVW4TK9o1PCQs=; b=bMKb1K56OPE8QEDSHpUqnUy8atY8SfzJL650c65LyKFR5iyqcPv3kpPyLS2sHu1lzd sOVhXbGyhXb1N/aZF3QHkGoT7EPl/3PxSksKUeejOPcgM+HAUfW44KPhGPjLpYisS1sK tdca/xJG/WAltDtqwHZtW7Hhyidva3ZUvJl6aD0B9rARSeUql03IQNrhj+TkM6mRlHsj KZ8y8odrj71fIjewiQtQabzo/vEy6RKwmXF0genzwevNPPi954iABfOZZ1L8dRXbWV3H rteCBy5pYW2MI3SmWeTI4OfV9qopHM5NSxyvbLjTwi0wQqxo5ypxM8xXVgXp0Z5TbZUL H01Q== X-Forwarded-Encrypted: i=1; AJvYcCW02TlFhpoMELlKOXfM3ksDbjJro+Rqpym4G+FwPiKmhPDCfnkPuJDfzq4/anRw6D2mr3fUr0gGaQCqRs7O/A84TTY= X-Gm-Message-State: AOJu0YxN4c7QIxAIvSHSSBLMbbOdUbQr82V9tkeizPuGnehCJmN6lCHM PsH7CRrcFgadXZorWch+1tcOKcmOGKiMD6SIW4nPYhx48CCKyAVCRsmwiMGMUIg= X-Google-Smtp-Source: AGHT+IFGZiI6AUAkN7eJwQnP8DpPrdR8tAAe1cjOdO9L4E8zWgjvbtVtl9NTqmwHz+5c4nciO1RMng== X-Received: by 2002:a81:c242:0:b0:65f:dfd9:b672 with SMTP id 00721157ae682-6895f9e5cdamr42031227b3.11.1722614340987; Fri, 02 Aug 2024 08:59:00 -0700 (PDT) Received: from localhost (syn-076-182-020-124.res.spectrum.com. [76.182.20.124]) by smtp.gmail.com with ESMTPSA id 00721157ae682-68a140613afsm2935237b3.136.2024.08.02.08.59.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 02 Aug 2024 08:59:00 -0700 (PDT) Date: Fri, 2 Aug 2024 11:58:59 -0400 From: Josef Bacik To: Mateusz Guzik Cc: Wojciech =?utf-8?Q?G=C5=82adysz?= , viro@zeniv.linux.org.uk, brauner@kernel.org, jack@suse.cz, ebiederm@xmission.com, kees@kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] kernel/fs: last check for exec credentials on NOEXEC mount Message-ID: <20240802155859.GB6306@perftesting> References: <20240801120745.13318-1-wojciech.gladysz@infogain.com> <20240801140739.GA4186762@perftesting> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Server: rspam03 X-Rspam-User: X-Rspamd-Queue-Id: 100D840007 X-Stat-Signature: 7nwwnobpym8g98wde9dek7i3dpzfqe86 X-HE-Tag: 1722614341-791676 X-HE-Meta: U2FsdGVkX18yjahdm3LI86n71l/7bNkwJPgU/IB7UVc7Ku2DAegy1kdCO3Ntn2VNDlZ7X5qrMNnUYrylGIb+tt9tDbqvmxqM81qawUrtQnKerbkGNCrLfJBrCIJdp2ZjBH7zCx7+ilSMh/xsZaTFxUzuzhhj6Ip5gEcK2nO10fqka61WHKCoX8ARmO+KJ9ZpX8COolJm5SlQaXGaj99SsAGF7V3IKRyNVFBiTQGkurC5pqnfdn2JCl0fLVkEgySBzbqYo122npFlOSREI38y4gI4+kRWG5+6scM7REp93SwJYBdNaTf9Z/hMNvGQBaBCkk9XL5p1rtlyyUL0NMzlx/hMqS79QOs8sizBC8UwXNX3lW00bD0r3CTUcNYvMgEL+YBse27/xjvmn9+On0FZeR6t5CsYNNC7FphXsRO4VgHgBd5+wRy/KxwjEovcOGv+iYS3WNuFItxoOBLumidZlnIYpw5F7tVrw8SBghcVVtTFGv7ofGI0Qy1k8dAQA4VDuG9ufJsJ6dDUEpUobhkD8F2HVVCx2ym0DWCxMKihEau3rcFAxFmx0wbNznMexnQCNYvDYEb9okfUPUIKnFWceMm7aMLG0//TUkfBW1Y7rG4LWAohVAWXGzNhaARE7rwDCYOr1PHtjA4ky9Zs7yaoVM1A6Siy94neBQas5++J+nhtZ/PL4ja4VufxynBHbteTyQ+z02MxmfAjYycGTPSWGHSeykImRNN1K6NrSXy7ZPo5G5jzBbd9wWpkdXfXWTZr0/+bdJvhshmnfPpZ/Gns2MWiC3UK11qERoNdf33X5LSzw+ZNkSiP/z8EBEefH8uC63g6my9uqS/BlF0qpcX+S2v+Bqq1NEfOd3CUjOwchkuLTMnsAGV2R455yzjBe0RZcblPQIEgU5RkmJG1hwUlAGiMHIE/pHNy9mGaGk3dxPFF/vGiTtaClWF3J2pMIsvllmaTMJnBg9Igr284aEV JSzp+c3M xgs8tUiAhVw3bZU7fhSdsruMK/f5EZ7AUvm8EHyM6PYKqgXa0X9cOZCbRMrKnX33qlVXSrfrQdJbTPt/uQw/2yYLj8oq20T67og2i6lfouMcWi5/+MD8JRfBpmOs6/n6yxjwin7XYZQFSMgJAb5MpYswi3JZXDTazD3BMJCbcAWm2cxJ3cFqXapJ1gByj5bfMpjIjHy/c3KfnccX5VbQtmpbe8t4P4dRCCTO/Pcm411P8RHa+e/BhplUsdfuNJy7FjOfoU899yR1StjR99q4ybpznVcnkdaKUT/ZNLnczBa0IEz8xJLk22+IaVaof+3ST4o1j3k4MUUsWaaAD916a70AEuo2zwqbHSX/LXNsx+dO5sVvtgGJSQBW3shE/Fck/f1R0D7Q4spbjD23WaLVXi0LPuKk45+NCR0d1 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, Aug 01, 2024 at 05:15:06PM +0200, Mateusz Guzik wrote: > On Thu, Aug 01, 2024 at 10:07:39AM -0400, Josef Bacik wrote: > > On Thu, Aug 01, 2024 at 02:07:45PM +0200, Wojciech Gładysz wrote: > > > Test case: thread mounts NOEXEC fuse to a file being executed. > > > WARN_ON_ONCE is triggered yielding panic for some config. > > > Add a check to security_bprm_creds_for_exec(bprm). > > > > > > > Need more detail here, a script or something to describe the series of events > > that gets us here, I can't quite figure out how to do this. > > > > > Stack trace: > > > ------------[ cut here ]------------ > > > WARNING: CPU: 0 PID: 2736 at fs/exec.c:933 do_open_execat+0x311/0x710 fs/exec.c:932 > > > Modules linked in: > > > CPU: 0 PID: 2736 Comm: syz-executor384 Not tainted 5.10.0-syzkaller #0 > > > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014 > > > RIP: 0010:do_open_execat+0x311/0x710 fs/exec.c:932 > > > Code: 89 de e8 02 b1 a1 ff 31 ff 89 de e8 f9 b0 a1 ff 45 84 ff 75 2e 45 85 ed 0f 8f ed 03 00 00 e8 56 ae a1 ff eb bd e8 4f ae a1 ff <0f> 0b 48 c7 c3 f3 ff ff ff 4c 89 f7 e8 9e cb fe ff 49 89 de e9 2d > > > RSP: 0018:ffffc90008e07c20 EFLAGS: 00010293 > > > RAX: ffffffff82131ac6 RBX: 0000000000000004 RCX: ffff88801a6611c0 > > > RDX: 0000000000000000 RSI: 0000000000000004 RDI: 0000000000000000 > > > RBP: ffffc90008e07cf0 R08: ffffffff8213173f R09: ffffc90008e07aa0 > > > R10: 0000000000000000 R11: dffffc0000000001 R12: ffff8880115810e0 > > > R13: dffffc0000000000 R14: ffff88801122c040 R15: ffffc90008e07c60 > > > FS: 00007f9e283ce6c0(0000) GS:ffff888058a00000(0000) knlGS:0000000000000000 > > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > > CR2: 00007f9e2848600a CR3: 00000000139de000 CR4: 0000000000352ef0 > > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > > > Call Trace: > > > bprm_execve+0x60b/0x1c40 fs/exec.c:1939 > > > do_execveat_common+0x5a6/0x770 fs/exec.c:2077 > > > do_execve fs/exec.c:2147 [inline] > > > __do_sys_execve fs/exec.c:2223 [inline] > > > __se_sys_execve fs/exec.c:2218 [inline] > > > __x64_sys_execve+0x92/0xb0 fs/exec.c:2218 > > > do_syscall_64+0x6d/0xa0 arch/x86/entry/common.c:62 > > > entry_SYSCALL_64_after_hwframe+0x61/0xcb > > > RIP: 0033:0x7f9e2842f299 > > > Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 b1 18 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48 > > > RSP: 002b:00007f9e283ce218 EFLAGS: 00000246 ORIG_RAX: 000000000000003b > > > RAX: ffffffffffffffda RBX: 00007f9e284bd3f8 RCX: 00007f9e2842f299 > > > RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000020000400 > > > RBP: 00007f9e284bd3f0 R08: 0000000000000000 R09: 0000000000000000 > > > R10: 0000000000000000 R11: 0000000000000246 R12: 00007f9e2848a134 > > > R13: 0030656c69662f2e R14: 00007ffc819a23d0 R15: 00007f9e28488130 > > > > > > Signed-off-by: Wojciech Gładysz > > > --- > > > fs/exec.c | 42 +++++++++++++++++++----------------------- > > > 1 file changed, 19 insertions(+), 23 deletions(-) > > > > > > diff --git a/fs/exec.c b/fs/exec.c > > > index a126e3d1cacb..0cc6a7d033a1 100644 > > > --- a/fs/exec.c > > > +++ b/fs/exec.c > > > @@ -953,8 +953,6 @@ EXPORT_SYMBOL(transfer_args_to_stack); > > > */ > > > static struct file *do_open_execat(int fd, struct filename *name, int flags) > > > { > > > - struct file *file; > > > - int err; > > > struct open_flags open_exec_flags = { > > > .open_flag = O_LARGEFILE | O_RDONLY | __FMODE_EXEC, > > > .acc_mode = MAY_EXEC, > > > @@ -969,26 +967,7 @@ static struct file *do_open_execat(int fd, struct filename *name, int flags) > > > if (flags & AT_EMPTY_PATH) > > > open_exec_flags.lookup_flags |= LOOKUP_EMPTY; > > > > > > - file = do_filp_open(fd, name, &open_exec_flags); > > > - if (IS_ERR(file)) > > > - goto out; > > > - > > > - /* > > > - * may_open() has already checked for this, so it should be > > > - * impossible to trip now. But we need to be extra cautious > > > - * and check again at the very end too. > > > - */ > > > - err = -EACCES; > > > - if (WARN_ON_ONCE(!S_ISREG(file_inode(file)->i_mode) || > > > - path_noexec(&file->f_path))) > > > - goto exit; > > > - > > > > This still needs to be left here to catch any bad actors in the future. Thanks, > > > > This check is fundamentally racy. > > path_noexec expands to the following: > return (path->mnt->mnt_flags & MNT_NOEXEC) || > (path->mnt->mnt_sb->s_iflags & SB_I_NOEXEC); > > An exec racing against remount setting the noexec flag can correctly > conclude the file can be execed and then trip over the check later if > the flag showed up in the meantime. > > This is not fuse-specific and I disagree with the posted patch as well. > > The snippet here tries to validate that permissions were correctly checked > at some point, but it fails that goal in 2 ways: > - the inode + fs combo might just happen to be fine for exec, even if > may_open *was not issued* > - there is the aforementioned race > > If this thing here is supposed to stay, it instead needs to be > reimplemented with may_open setting a marker "checking for exec was > performed and execing is allowed" somewhere in struct file. This sounds like a reasonable alternative solution. > > I'm not confident this is particularly valuable, but if it is, it > probably should hide behind some debug flags. I'm still going to disagree here, putting it behind a debug flag means it'll never get caught, and it obviously proved valuable because we're discussing this particular case. Is it racy? Yup sure. I think that your solution is the right way to fix it, and then we can have a WARN_ON(!(file->f_mode & FMODE_NO_EXEC_CHECKED)); or however we choose to flag the file, that way we are no longer racing with the mount flags and only validating that a check that should have already occurred has in fact occurred. Thanks, Josef