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 2F418EE49A6 for ; Sat, 19 Aug 2023 20:03:45 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 47254900011; Sat, 19 Aug 2023 16:03:44 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3F77D900008; Sat, 19 Aug 2023 16:03:44 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2BEB2900011; Sat, 19 Aug 2023 16:03:44 -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 1CBBB900008 for ; Sat, 19 Aug 2023 16:03:44 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id D27F5160AEE for ; Sat, 19 Aug 2023 20:03:43 +0000 (UTC) X-FDA: 81141929526.19.40609A2 Received: from mail-ot1-f47.google.com (mail-ot1-f47.google.com [209.85.210.47]) by imf06.hostedemail.com (Postfix) with ESMTP id B37A518001A for ; Sat, 19 Aug 2023 20:03:41 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=gmail.com header.s=20221208 header.b=g+rCERem; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf06.hostedemail.com: domain of mjguzik@gmail.com designates 209.85.210.47 as permitted sender) smtp.mailfrom=mjguzik@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1692475421; 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=aJmwmCW2cEQCd04RE4RrxWMNws0AbxA5Q61XDGT3T+Y=; b=0WrAv9aFKB98LAahM8leVOiH7VJ0pur34f/f3qibU2e0C//KAs/AkjLAvmPfdPD8yO1M0I bgQbOTK29vkDxhF19lrH+kEEdf+jsH6pArmij0549FrpZKs6Ntr8dtbPKPXfeIpSQ3qTfV XrV3jBSl7eXUV/Y76zoWbMoyYObCJJo= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=gmail.com header.s=20221208 header.b=g+rCERem; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf06.hostedemail.com: domain of mjguzik@gmail.com designates 209.85.210.47 as permitted sender) smtp.mailfrom=mjguzik@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1692475421; a=rsa-sha256; cv=none; b=MSbPb+lUEB/Vpsb0GQuRAr+e5jinLeRHP4966FkGRP1zVwKnD9t+6czhA414U5yjggpHNd /5FB9qcKDhNVG1R69jfJKM89uiSBYg1GvSJCWy9EPiltB1rr5QtlHQrGtys73Dy8usvYJV Q9+5vvZTvwlUBAJ/ouUZnEfPNj0KskI= Received: by mail-ot1-f47.google.com with SMTP id 46e09a7af769-6bd6a34474cso1565067a34.3 for ; Sat, 19 Aug 2023 13:03:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1692475421; x=1693080221; h=cc:to:subject:message-id:date:from:references:in-reply-to :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=aJmwmCW2cEQCd04RE4RrxWMNws0AbxA5Q61XDGT3T+Y=; b=g+rCERemWikecerpqjEDtuCW3J3jHkNm2Fn5WiZ00kYiXRoMVCXXROJJpfAhc15ysl UWyXlVSZpz35w9DMq98q4cOxdVKV1k7EtA3CBAk8rCr20aJ/nHENESikDfhiZEiJEyFo yTLrMLCL1aTFaD2s021AVlUAsULlYgxSb4homYRDe3mtKQgj76vSGFVxcQd3pRccd42Z SlSgLj/i64i7MMUYTgel3/XsUsGJY9+5Za+GFigyUJTVhq09oXN0yrnplUVftBvbvW92 lJDC2OQrmljAnyqavE9PxuqenvyqAGHnmcXJ2Fq7fuUAF6qD65/hWv45bAEK6auxe91l I3EA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1692475421; x=1693080221; h=cc:to:subject:message-id:date:from:references:in-reply-to :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=aJmwmCW2cEQCd04RE4RrxWMNws0AbxA5Q61XDGT3T+Y=; b=gwpXtvGWVICUf3dJGyo6/ncp+uMt5slweokY7F/zZq33cuuUSe+oDNmvYof/N4DgYy /g4cj/7QZakNKLTzU8GQOyW+JOzJacX0zQ8HVCe5nYRXSuIC2iZalX+UI/eq39dm3V4W PiDG0NG8cNo7IqoaNnLIriL7wDMYCGCBqQhuAHNRapCHroEVm/w63Jv7V/JWyD5Ys1dE LuK8qhG9kaxLP2BKQgEFC3Vudo7o8gJU4D15r5a/KSwMSjsj3ns5VNZu3xKFv5E+7g8k tyAXfcx39hd5qgJ8DdlZbWrLktS2kwo2DGqkIGZsrx2qdkUTkYbVihkuIPARIPrIyupp X0aw== X-Gm-Message-State: AOJu0YxiBRVsZv0+wPRn6yq2UvChIdqkwZ4CSo8YTk5M9TH719YEAjO2 pTNCyEskPb9L2NXHeJ7Vb1qPscLhbJq6e126fs4= X-Google-Smtp-Source: AGHT+IFcQ5JgHH5X+0iR2/3eAmurP/E4fqkFweod4P5W5Y3dD5jml27OqGCNcTMGfxg3F0F72tWMGlj2+wEF2tqx2qQ= X-Received: by 2002:a05:6870:6394:b0:1c1:12dc:70b1 with SMTP id t20-20020a056870639400b001c112dc70b1mr3579234oap.57.1692475420635; Sat, 19 Aug 2023 13:03:40 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:ac9:79d9:0:b0:4f0:1250:dd51 with HTTP; Sat, 19 Aug 2023 13:03:40 -0700 (PDT) In-Reply-To: <20230819-geblendet-energetisch-a90a2886216c@brauner> References: <000000000000c74d44060334d476@google.com> <87o7j471v8.fsf@email.froward.int.ebiederm.org> <202308181030.0DA3FD14@keescook> <20230818191239.3cprv2wncyyy5yxj@f> <20230819-geblendet-energetisch-a90a2886216c@brauner> From: Mateusz Guzik Date: Sat, 19 Aug 2023 22:03:40 +0200 Message-ID: Subject: Re: [syzbot] [ntfs?] WARNING in do_open_execat To: Christian Brauner Cc: Kees Cook , "Eric W. Biederman" , syzbot , anton@tuxera.com, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-ntfs-dev@lists.sourceforge.net, syzkaller-bugs@googlegroups.com, Al Viro , "Theodore Ts'o" Content-Type: text/plain; charset="UTF-8" X-Rspam-User: X-Stat-Signature: y3ec9gs1esz587sym55191xje9og4k4s X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: B37A518001A X-HE-Tag: 1692475421-596593 X-HE-Meta: U2FsdGVkX1//jQxYGTSI+uG8rRwZqQ+sWe+ELyZYw6Xq/PQL0JDNujUfYHK6JggDQFAhBDxSzUCy1Lp3YMiArQmKP1tP6nEgznbRXPR8XZtbViAyOdvzutaZdaoY9zvybzlqIpqkMdgHlilJtBTmJiGwBjqQftYakcqknvC2Aw+e4x+XUkW1qAYgG91CPj+j36dIU5gddhLERLrtNU/bIPmvl7Ix5+7VRKLiHiPJY/4n2CEtK1OorLiECJREuTv/76c+yV6Gzm8cUxVjad8TWb65RVuQcV0FTrD8mz6518Zuj+qS9o8R7iB0wblJdBxiCVk3SNpXA+0ljUcimdpPK8SYjYIM7dncEfpgrLXTjJ045bepgouN5jHh56ODUAktC+OnmkB+OPxMmurKNUc/WrnyQsE2Ks9LMgcs0u4jZwap6pTe9S2QlGA52UixZ1EkPLhOCglcw7uKYvIWgcIUWk5/24fvEgX9u/Nk2drktqQKRRMT6GOHHcEVZfdgd65bs++pbK6HDdrojuhx5ZPdj70YZaUkRWuxB92TCc1XxXE7QoNWbeBpGQLE3FVhPhS2s4wtgtKP7upstVIrLf7C/GjsBPvZ1vYE6FQNdE/tHr/R+Muvacg4v6+CVuAybo8Ee6+TQBVCh23T0lownoPbOdofaoeYt5BycWm3itDeTpzQF4VUBEBE6uP0hgcY8Q10oeR9eysxCbZH8i/Ee9xIOej/yWPypMQjlmSMAvU2TPjht14zvhoGAHpWiZyg9uIxUWw9dZd6oqcI8hfzg9Bf9fHqGl8G4CiGtJxOu00dR3G/VlLDOPDkXjIL25gizlyF3tu5eJS/A8XX0xDGmqLhQ5UVn79O3P4PP7mxCze4fvall6uJiEx45sHwWdK6yKtqOnXogmNUn/q3jKYG6jlvxvxmP0u9Q2mgAanjSg58qVScqZV6VJqVWRx1LEOGd/uHjq0XqHpIMstbpA9VSXN HDCa+DoH zN990HHjmJDw9BYzglwchZlI/IPXN6sk/0wHChKY83dlTOSZQdVoO0zI2oXQXznlpzKEKJaBE+F+v83OLxCNvCS8Glg1+WFyNRO+ow7nOyoqlHsvKEEjc+/rfoVsMxW8ykkdH2UAl5D0MEm6zzTu55n8a+8U6bG5uRpYgTj61uzBsCzwtzVuGk+vFEcMHFwdV6wdBapJZhnrSYFlT2qwzff2HYC0uEXvtk1J5Pv2t7FXOfKdVsUOiNg9TqdFHlZnJEaaFKp2LaUdMVSak7cGyRj/jzhIGErncJtfaytJyGD92M+KoUdARnoHfAFTTj4Do5comx0B7o9V+qM+IHes6X3eyIwBx3MQjieg38px30e4MjmAqTtPvKuoe7IE9DeRH7sLn10G8XNJ7/u2CPgchLC8ZzCSVcUVlcjYxJoHAxQfRpJTqhVgq+fweVVjsMl/3c7DOZ0dT61woECcUCsesYUfe8NU+xjCiJ7BYcKq2WQ688rLXWYJzYRgAe5zHTg8R4HjnOp+3P/QqC6KeXT9wOufo+qnMR6Ld+uNm7PmsLYLL6bNcZ+I7SemhYlA8ARjQlfi3+nEbYrLQw1U66WUfWtkshUtPI+huDFc4A30M778IdENs6/TWe4RmGw== 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: On 8/19/23, Christian Brauner wrote: > On Fri, Aug 18, 2023 at 09:12:39PM +0200, Mateusz Guzik wrote: >> On Fri, Aug 18, 2023 at 10:33:26AM -0700, Kees Cook wrote: >> > This is a double-check I left in place, since it shouldn't have been >> > reachable: >> > >> > /* >> > * 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; >> > >> >> As I mentioned in my other e-mail, the check is racy -- an unlucky >> enough remounting with noexec should trip over it, and probably a chmod >> too. >> >> However, that's not what triggers the warn in this case. >> >> The ntfs image used here is intentionally corrupted and the inode at >> hand has a mode of 777 (as in type not specified). >> >> Then the type check in may_open(): >> switch (inode->i_mode & S_IFMT) { >> >> fails to match anything. >> >> This debug printk: >> diff --git a/fs/namei.c b/fs/namei.c >> index e56ff39a79bc..05652e8a1069 100644 >> --- a/fs/namei.c >> +++ b/fs/namei.c >> @@ -3259,6 +3259,10 @@ static int may_open(struct mnt_idmap *idmap, const >> struct path *path, >> if ((acc_mode & MAY_EXEC) && path_noexec(path)) >> return -EACCES; >> break; >> + default: >> + /* bogus mode! */ >> + printk(KERN_EMERG "got bogus mode inode!\n"); >> + return -EACCES; >> } >> >> error = inode_permission(idmap, inode, MAY_OPEN | acc_mode); >> >> catches it. >> >> All that said, I think adding a WARN_ONCE here is prudent, but I >> don't know if denying literally all opts is the way to go. >> >> Do other filesystems have provisions to prevent inodes like this from >> getting here? > > Bugs reported against the VFS from ntfs/ntfs3 are to be treated with > extreme caution. Frankly, if it isn't reproducible without a corrupted > ntfs/ntfs3 image it is to be dismissed until further notice. > > In this case it simply seems that ntfs is failing at ensuring that its > own inodes it reads from disk have a well-defined type. > > If ntfs fails to validate that its own inodes it puts into the icache > are correctly initialized then the vfs doesn't need to try and taper > over this. > > If ntfs fails at that, there's no guarantee that it doesn't also fail at > setting the correct i_ops for that inode. At which point we can check > the type in may_open() but we already used bogus i_ops the whole time on > some other inodes. > > We're not here to make up for silly bugs like this. That WARN belongs > into ntfs not the vfs. > Given the triggered WARN_ON it seemed to me this would be the operating procedure, I am happy it is not ;) Per your description and the one provided by Theodore I take it filesystems must not ship botched inodes like this one. While in this case this is a clear-cut bug in ntfs, I would argue the entire ordeal exposes a deficiency in VFS -- it should have a debug-only mechanism which catches cases like this early on. For example there could be a mandatory function to call when the filesystem claims it constructed the inode to assert a bunch on it -- it would not catch all possible problems, but would definitely catch this one (and VFS would have to detect the call was not made). Perhaps I should write a separate e-mail about this, but I'm surprised there is no debug-only (as in not present in production kernels) support for asserting the state. To give one example which makes me itchy see inode destruction. There are few checks in clear_inode, but past that there is almost nothing. Similarly there are quite a few comments how the caller is required to hold a given lock, which should have been converted to lockdep asserts years ago. I'm going to write something up later. -- Mateusz Guzik