From: Eric Biggers <ebiggers@kernel.org>
To: Theodore Ts'o <tytso@mit.edu>
Cc: Matthew Wilcox <willy@infradead.org>,
Gabriel Krisman Bertazi <krisman@suse.de>,
viro@zeniv.linux.org.uk, brauner@kernel.org, jaegeuk@kernel.org,
linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org,
linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [PATCH v5 01/10] fs: Expose helper to check if a directory needs casefolding
Date: Mon, 14 Aug 2023 10:22:44 -0700 [thread overview]
Message-ID: <20230814172244.GA1171@sol.localdomain> (raw)
In-Reply-To: <20230814113852.GD2247938@mit.edu>
On Mon, Aug 14, 2023 at 07:38:52AM -0400, Theodore Ts'o wrote:
> On Sat, Aug 12, 2023 at 09:30:22PM -0700, Eric Biggers wrote:
> > Well, one thing that the kernel community can do to make things better is
> > identify when a large number of bug reports are caused by a single issue
> > ("userspace can write to mounted block devices"), and do something about that
> > underlying issue (https://lore.kernel.org/r/20230704122727.17096-1-jack@suse.cz)
> > instead of trying to "fix" large numbers of individual "bugs". We can have 1000
> > bugs or 1 bug, it is actually our choice in this case.
>
> That's assuming the syzbot folks are willing to enable the config in
> Jan's patch. The syzbot folks refused to enable it, unless the config
> was gated on CONFIG_INSECURE, which I object to, because that's
> presuming a threat model that we have not all agreed is valid.
>
> Or rather, if it *is* valid some community members (or cough, cough,
> **companies**) need to step up and supply patches. As the saying
> goes, "patches gratefully accepted". It is *not* the maintainer's
> responsibility to grant every single person whining about a feature
> request, or even a bug fix.
They did disable CONFIG_XFS_SUPPORT_V4. Yes, there was some pushback, but they
are very understandably (and correctly) concerned about ending up in a situation
where buggy code is disabled for syzkaller but enabled for everyone else. They
eventually did accept the proposal to disable CONFIG_XFS_SUPPORT_V4, for reasons
including the fact that there is a concrete deprecation plan. (FWIW, the XFS
maintainers had also pushed back strongly when I suggested adding a kconfig
option for XFS v4 support back in 2018. Sometimes these things just take time.)
Anyway, syzkaller is just the messenger that is reminding us of a problem. The
actual problem here is that "make filesystems robust against concurrent changes
to block device's page cache" is effectively unsolvable. *Every* memory access
to the cache would need to use READ_ONCE/WRITE_ONCE, and *every* re-read of
every field would need to be fully re-validated. PageChecked would need to go
away for metadata, as it would be invalid to cache the checked status at all.
There's basically zero chance of getting this correct; filesystems struggle to
validate even the metadata read from disk correctly, so how are they going to
successfully continually revalidate all cached metadata in memory?
I don't understand why we would waste time trying to do that instead of focusing
on an actual solution. Sure, probably The Linux Filesystem Maintainers(TM)
don't have time to help with migrating legacy use cases of writing to mounted
block devices, but that just means that someone needs to step up to do it. It
doesn't mean that they need to instead waste time on pointless "fixes".
Keep in mind, the syzkaller team isn't asking for these pointless "fixes"
either. They'd very much prefer 1 fix to 1000 fixes. I think some confusion
might be arising from the very different types of problems that syzkaller finds.
Sometimes 1 syzkaller report == 1 bug == 1 high-priority "must fix" bug == 1
vulnerability == 1 fix needed. But in general syzkaller is just letting kernel
developers know about a problem, and it is up to them to decide what to do about
it. In this case there is one underlying issue that needs to be fixed, and the
individual syzkaller reports that result from that issue are not important.
- Eric
next prev parent reply other threads:[~2023-08-14 17:23 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-12 0:41 [PATCH v5 00/10] Support negative dentries on case-insensitive ext4 and f2fs Gabriel Krisman Bertazi
2023-08-12 0:41 ` [PATCH v5 01/10] fs: Expose helper to check if a directory needs casefolding Gabriel Krisman Bertazi
2023-08-12 1:59 ` Eric Biggers
2023-08-12 23:06 ` Theodore Ts'o
2023-08-13 0:12 ` Eric Biggers
2023-08-13 3:08 ` Matthew Wilcox
2023-08-13 4:30 ` Eric Biggers
2023-08-14 11:38 ` Theodore Ts'o
2023-08-14 17:22 ` Eric Biggers [this message]
2023-08-15 3:59 ` Theodore Ts'o
2023-08-14 15:02 ` Gabriel Krisman Bertazi
2023-08-14 19:14 ` Eric Biggers
2023-08-14 19:26 ` Gabriel Krisman Bertazi
2023-08-12 0:41 ` [PATCH v5 02/10] ecryptfs: Reject casefold directory inodes Gabriel Krisman Bertazi
2023-08-12 0:41 ` [PATCH v5 03/10] 9p: Split ->weak_revalidate from ->revalidate Gabriel Krisman Bertazi
2023-08-12 0:41 ` [PATCH v5 04/10] fs: Expose name under lookup to d_revalidate hooks Gabriel Krisman Bertazi
2023-08-12 2:15 ` Eric Biggers
2023-08-17 7:00 ` kernel test robot
2023-08-17 9:12 ` kernel test robot
2023-08-12 0:41 ` [PATCH v5 05/10] fs: Add DCACHE_CASEFOLDED_NAME flag Gabriel Krisman Bertazi
2023-08-12 2:17 ` Eric Biggers
2023-08-14 15:03 ` Gabriel Krisman Bertazi
2023-08-12 0:41 ` [PATCH v5 06/10] libfs: Validate negative dentries in case-insensitive directories Gabriel Krisman Bertazi
2023-08-12 2:41 ` Eric Biggers
2023-08-14 14:50 ` Gabriel Krisman Bertazi
2023-08-14 18:42 ` Eric Biggers
2023-08-14 19:21 ` Gabriel Krisman Bertazi
2023-08-14 19:26 ` Eric Biggers
2023-08-12 0:41 ` [PATCH v5 07/10] libfs: Chain encryption checks after case-insensitive revalidation Gabriel Krisman Bertazi
2023-08-12 0:41 ` [PATCH v5 08/10] libfs: Merge encrypted_ci_dentry_ops and ci_dentry_ops Gabriel Krisman Bertazi
2023-08-12 0:41 ` [PATCH v5 09/10] ext4: Enable negative dentries on case-insensitive lookup Gabriel Krisman Bertazi
2023-08-12 0:41 ` [PATCH v5 10/10] f2fs: " Gabriel Krisman Bertazi
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=20230814172244.GA1171@sol.localdomain \
--to=ebiggers@kernel.org \
--cc=brauner@kernel.org \
--cc=jaegeuk@kernel.org \
--cc=krisman@suse.de \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-fsdevel@vger.kernel.org \
--cc=tytso@mit.edu \
--cc=viro@zeniv.linux.org.uk \
--cc=willy@infradead.org \
/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;
as well as URLs for NNTP newsgroup(s).