linux-kernel - Re: [PATCH] kernel/fs: last check for exec credentials on NOEXEC mount

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGudoHFOwOaDyLg3Nh=gPvhG6cO+NXf_xqCjqjz9OxP9DLP3kw@mail.gmail.com>
Date: Sat, 3 Aug 2024 08:29:17 +0200
From: Mateusz Guzik <mjguzik@...il.com>
To: Josef Bacik <josef@...icpanda.com>
Cc: Wojciech Gładysz <wojciech.gladysz@...ogain.com>, 
	viro@...iv.linux.org.uk, brauner@...nel.org, jack@...e.cz, 
	ebiederm@...ssion.com, kees@...nel.org, linux-fsdevel@...r.kernel.org, 
	linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kernel/fs: last check for exec credentials on NOEXEC mount

On Fri, Aug 2, 2024 at 5:59 PM Josef Bacik <josef@...icpanda.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>