CVE-2025-21830: landlock: Handle weird files

[Dave Chinner]

On Mon, Mar 10, 2025 at 10:09:22PM -0400, Kent Overstreet wrote:
> On Tue, Mar 11, 2025 at 10:42:41AM +1100, Dave Chinner wrote:
> > [cc linux-fsdevel]
> > 
> > On Mon, Mar 10, 2025 at 03:36:04PM +0100, Greg Kroah-Hartman wrote:
> > > On Mon, Mar 10, 2025 at 01:00:50PM +0100, Mickaël Salaün wrote:
> > > > Hi Greg,
> > > > 
> > > > FYI, I don't think this patch fixes a security issue.  If attackers can
> > > > corrupt a filesystem, then they should already be able to harm the whole
> > > > system.
> > > > 
> > > > The commit description might be a bit confusing, but from an access
> > > > control point of view, the filesystem on which we spotted this issue
> > > > (bcachefs) does not allow to open weird files (but they are still
> > > > visible, hence this patch) and I guess it would be the same for other
> > > > filesystems, right?  I'm not sure how a weird file could be used by user
> > > > space.  See
> > > > https://lore.kernel.org/all/Zpc46HEacI%2Fwd7Rg@dread.disaster.area/
> > > > 
> > > > The goal of this fix was mainly to not warn about a bcachefs issue (and
> > > > avoid related syzkaller report for Landlock), and to harden Landlock in
> > > > case other filesystems have this kind of bug.
> > > 
> > > It was issue a CVE because the reviewers thought that it was a way to
> > > circumvent the landlock permission checks, based on the changelog text
> > > (note, creating a "corrupted filesystem" is quite easy to get many Linux
> > > systems to auto-mount it, so those types of issues do get assigned
> > > CVEs.)
> > 
> > That's an argument straight from the security theatre.
> > 
> > > If you all do not think this meets the definition of a vulnerability as
> > > defined by CVE.org as:
> > > 	An instance of one or more weaknesses in a Product that can be
> > > 	exploited, causing a negative impact to confidentiality, integrity, or
> > > 	availability; a set of conditions or behaviors that allows the
> > > 	violation of an explicit or implicit security policy.
> > 
> > Yes, so shall we follow this reasoning based on untrusted user
> > auto-mounts of untrusted devices to it's logical conclusion?
> > 
> > If an untrusted user is in control of the filesystem image, then
> > they don't need to corrupt the filesystem image to subvert the
> > system. They can just change the permissions on files, change ACLs,
> > change security xattrs (selinux, landlock, smack, etc),
> > replace the contents of file data (e.g. trojan executables), etc.
> 
> If user mounts are enabled, that comes with UID mapping, and device
> nodes disabled - no?

Not necessarily. Those security mechanisms are all optional mount
options under userspace control....

> Out of curiosity, what's keeping us from saying "user mounts are
> generally expected to be safe" for XFS?

What does "generally expected to be safe" actually mean?

If be "safe" you mean "won't crash the kernel if the structure has
been altered in detectable ways with", then we already largely tick
that box. However, there are whole classes of DOS attacks that are
very difficult to detect without rigorous, expensive runtime
checking (e.g. loops in btree pointers).

Hence while we catch almost all the the obvious out-of-bounds
corruptions within an object, detecting corruptions that require
spanning a largely unbound number of objects to detect are not
handled at all. I can corrupt a filesystem to induce an endless
btree search loop like this pretty easily with a little bit of
xfs_db magic. Yup, we even provide the tools to make doing stuff
like this easy...

If by "safe" you mean "can detect all cases where a metadata field
or file data has been tampered with", then XFS is completely unsafe
and should not be used.

We can't detect that a malicious actor has changed something like a
file permission field or the contents of a security xattr.  To do
that requires cryptographically secure signatures of metadata
objects and file data. We do not have that sort of feature in the
on-disk format. We expect users that need protection from such
tampering will use an envrypted block device to prevent malicious
actors from being able to mutate the filesystem structure in this
way.

> Obviously, that does expose a massive attack surface, so saying that for
> a C codebase that wasn't initially designed for it has a high pucker
> factor.
>
> But I've been impressed with syzbot's ability to find bugs, so barring
> architectural issues which I assume you'd know about it seems it's not
> nearly as crazy a thought as it used to be - for XFS, as you guys have
> been the most rigorous about hardening so I expect that's about as good
> as it's going to get until we start rewriting our filesystems in Rust.

The concerns I have about malicious actors are not mitigated by the
language the filesystem is implemented in.

It has everything to do with the fact that a filesystem like XFS or
ext4 cannot detect someone changing permissions on a file to, say,
add a setuid bit to the permissions field and then hide the
modification by recalculating the correct CRC for the metdata block.

Solving that problem requires a fundamentally different fs/device
trust model (i.e. the device is *never* trusted) and an on-disk
format that is based around "trust nothing" rather than "trust
everything".

-Dave.
-- 
Dave Chinner
david at fromorbit.com