linux-ext4 - [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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <7c4202348f67788db55c7ec445cbe3f2d587daf2.1744394929.git.nirjhar.roy.lists@gmail.com>
Date: Fri, 11 Apr 2025 23:44:52 +0530
From: "Nirjhar Roy (IBM)" <nirjhar.roy.lists@...il.com>
To: linux-xfs@...r.kernel.org
Cc: 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,
	nirjhar.roy.lists@...il.com
Subject: [PATCH v1] xfs: Fail remount with noattr2 on a v5 xfs with CONFIG_XFS_SUPPORT_V4=y

Bug: When we compile the kernel with CONFIG_XFS_SUPPORT_V4=y, remount
with "-o remount,noattr2" on a v5 XFS does not fail explicitly.

Reproduction:
mkfs.xfs -f /dev/loop0
mount /dev/loop0 /mnt/scratch
mount -o remount,noattr2 /dev/loop0 /mnt/scratch # This should fail but it doesn't

However, with CONFIG_XFS_SUPPORT_V4=n, the remount correctly fails explicitly.
This is because the way the following 2 functions are defined:

static inline bool xfs_has_attr2 (struct xfs_mount *mp)
{
	return !IS_ENABLED(CONFIG_XFS_SUPPORT_V4) ||
		(mp->m_features & XFS_FEAT_ATTR2);
}
static inline bool xfs_has_noattr2 (const struct xfs_mount *mp)
{
	return mp->m_features & XFS_FEAT_NOATTR2;
}

xfs_has_attr2() returns true when CONFIG_XFS_SUPPORT_V4=n and hence, the
the following if condition in xfs_fs_validate_params() succeeds and returns -EINVAL:

/*
 * We have not read the superblock at this point, so only the attr2
 * mount option can set the attr2 feature by this stage.
 */

if (xfs_has_attr2(mp) && xfs_has_noattr2(mp)) {
	xfs_warn(mp, "attr2 and noattr2 cannot both be specified.");
	return -EINVAL;
}
With CONFIG_XFS_SUPPORT_V4=y, xfs_has_attr2() always return false and hence no error
is returned.

Fix: Check if the existing mount is has crc enabled(i.e, of type v5 and has attr2 enabled)
and the remount has noattr2, if yes, return -EINVAL.

I have tested xfs/{189,539} in fstests with v4 and v5 XFS with both CONFIG_XFS_SUPPORT_V4=y/n
and they both behave as expected.

This patch also fixes remount from noattr2 -> attr2 (on a v4 xfs).

Related discussion in [1]

[1] https://lore.kernel.org/all/Z65o6nWxT00MaUrW@dread.disaster.area/

Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@...il.com>
---
 fs/xfs/xfs_super.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index b2dd0c0bf509..fd72c8fcd3a7 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -2114,6 +2114,23 @@ xfs_fs_reconfigure(
 	if (error)
 		return error;
 
+	/* attr2 -> noattr2 */
+	if (xfs_has_noattr2(new_mp)) {
+		if (xfs_has_crc(mp)) {
+			xfs_warn(mp, "attr2 and noattr2 cannot both be specified.");
+			return -EINVAL;
+		}
+		else {
+			mp->m_features &= ~XFS_FEAT_ATTR2;
+			mp->m_features |= XFS_FEAT_NOATTR2;
+		}
+
+	} else if (xfs_has_attr2(new_mp)) {
+			/* noattr2 -> attr2 */
+			mp->m_features &= ~XFS_FEAT_NOATTR2;
+			mp->m_features |= XFS_FEAT_ATTR2;
+	}
+
 	/* inode32 -> inode64 */
 	if (xfs_has_small_inums(mp) && !xfs_has_small_inums(new_mp)) {
 		mp->m_features &= ~XFS_FEAT_SMALL_INUMS;
-- 
2.43.5