linux-ext4 - Re: [PATCH v1] xfs: Fail remount with noattr2 on a v5 xfs with CONFIG_XFS_SUPPORT

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 for Android: free password hash cracker in your pocket

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

Message-ID: <Z_9JmaXJJVJFJ2Yl@infradead.org>
Date: Tue, 15 Apr 2025 23:09:29 -0700
From: Christoph Hellwig <hch@...radead.org>
To: "Nirjhar Roy (IBM)" <nirjhar.roy.lists@...il.com>
Cc: Christoph Hellwig <hch@...radead.org>, linux-xfs@...r.kernel.org,
	linux-ext4@...r.kernel.org, fstests@...r.kernel.org,
	ritesh.list@...il.com, ojaswin@...ux.ibm.com, djwong@...nel.org,
	zlang@...nel.org, david@...morbit.com
Subject: Re: [PATCH v1] xfs: Fail remount with noattr2 on a v5 xfs with
 CONFIG_XFS_SUPPORT_V4=y

On Tue, Apr 15, 2025 at 12:48:39PM +0530, Nirjhar Roy (IBM) wrote:
> condition(noattr2 on v5) is not caught in xfs_fs_validate_params() because
> the superblock isn't read yet and "struct xfs_mount    *mp" is still not
> aware of whether the underlying filesystem is v5 or v4 (i.e, whether crc=0
> or crc=1). So, even if the underlying filesystem is v5, xfs_has_attr2() will
> return false, because the m_features isn't populated yet.

Yes.

> However, once
> xfs_readsb() is done, m_features is populated (mp->m_features |=
> xfs_sb_version_to_features(sbp); called at the end of xfs_readsb()). After
> that, when xfs_finish_flags() is called, the invalid mount option (i.e,
> noattr2 with crc=1) is caught, and the mount fails correctly. So, m_features
> is partially populated while xfs_fs_validate_params() is getting executed, I
> am not sure if that is done intentionally.

As you pointed out above it can't be fully populated becaue we don't
have all the information.  And that can't be fixed because some of the
options are needed to even get to reading the superblock.

So we do need a second pass of verification for everything that depends
on informationtion from the superblock.  The fact that m_features
mixes user options and on-disk feature bits is unfortunately not very
helpful for a clear structure here.