From: Mateusz Guzik <mjguzik@gmail.com>
To: Josef Bacik <josef@toxicpanda.com>
Cc: "Wojciech Gładysz" <wojciech.gladysz@infogain.com>,
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
Date: Sat, 3 Aug 2024 08:29:17 +0200 [thread overview]
Message-ID: <CAGudoHFOwOaDyLg3Nh=gPvhG6cO+NXf_xqCjqjz9OxP9DLP3kw@mail.gmail.com> (raw)
In-Reply-To: <20240802155859.GB6306@perftesting>
On Fri, Aug 2, 2024 at 5:59 PM Josef Bacik <josef@toxicpanda.com> wrote:
>
> On Thu, Aug 01, 2024 at 05:15:06PM +0200, Mateusz Guzik wrote:
> > 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,
>
To my understanding the submitter ran into the thing tripping over the
racy check, so this check did not find a real bug elsewhere in this
instance.
The only case that I know of where this fired and found a real problem
was after ntfs constructed a bogus inode:
https://lore.kernel.org/linux-fsdevel/20230818191239.3cprv2wncyyy5yxj@f/
But that is a deficiency in debug facilities in the vfs layer -- this
only tripped over because syzkaller tried to exec a sufficiently bogus
inode, while the vfs layer should have prevented that from happening
to begin with.
There should be well-defined spot where the filesystem claims the
inode in fully constructed at which the vfs layer verifies its state
(thus in particular i_mode). If implemented it would have caught the
problem before the inode escaped ntfs and presumably would find some
other problems which the kernel as is does not correctly report. This
in part depends on someone(tm) implementing VFS_* debug macros first,
preferably in a way which can dump inode info on assertion failure.
I have this at the bottom on a TODO list for a rainy day.
However, it's not my call to make as to what to do here. I outlined my
$0,04 and I'm buggering off.
--
Mateusz Guzik <mjguzik gmail.com>
next prev parent reply other threads:[~2024-08-03 6:29 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-01 12:07 Wojciech Gładysz
2024-08-01 14:07 ` Josef Bacik
2024-08-01 15:15 ` Mateusz Guzik
2024-08-02 15:58 ` Josef Bacik
2024-08-03 6:29 ` Mateusz Guzik [this message]
2024-08-05 9:26 ` Christian Brauner
2024-08-05 13:17 ` [PATCH] exec: drop a racy path_noexec check Mateusz Guzik
2024-08-05 15:35 ` Christian Brauner
2024-08-05 20:21 ` Kees Cook
2024-08-05 23:38 ` Al Viro
2024-08-05 23:41 ` Al Viro
2024-08-06 7:06 ` Christian Brauner
2024-08-02 3:28 ` [PATCH] kernel/fs: last check for exec credentials on NOEXEC mount Kees Cook
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAGudoHFOwOaDyLg3Nh=gPvhG6cO+NXf_xqCjqjz9OxP9DLP3kw@mail.gmail.com' \
--to=mjguzik@gmail.com \
--cc=brauner@kernel.org \
--cc=ebiederm@xmission.com \
--cc=jack@suse.cz \
--cc=josef@toxicpanda.com \
--cc=kees@kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=viro@zeniv.linux.org.uk \
--cc=wojciech.gladysz@infogain.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox