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@xxxxxxxxxxxxxxxxxxx/ > > > > > > > > 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@xxxxxxxxxxxxx
