Skip to contentSkip to conversation

fix s_uuid and f_fsid consistency for cloned filesystems

PATCH In review Anand Jain 3 participants 5 messages Apr 27 – May 26 Btrfs file system

lore
Share & tools
lore

Anand Jain

author
#0 lore raw

v3 cover fix s_uuid and f_fsid consistency for cloned filesystems

v3:
Thanks for the input. The plan to implement a consistent f_fsid across all major filesystems has been dropped for now, so ext4 patch is removed. Please refer to the discussion here [1]:

    [1] https://lore.kernel.org/linux-btrfs/33e8eb64c304a4d42b60f608c26497bf9a2e9e19.1774092915.git.asj@kernel.org/#t

This patch set specifically addresses the issue reported by OverlayFS [2] regarding cloned Btrfs filesystems.

    [2] https://lore.kernel.org/linux-btrfs/[email protected]

v2:
https://lore.kernel.org/linux-btrfs/[email protected]

Anand Jain (2):
btrfs: use on-disk uuid for s_uuid in temp_fsid mounts
btrfs: derive f_fsid from on-disk fsuuid and dev_t

 fs/btrfs/disk-io.c | 11 ++++++++++-
 fs/btrfs/super.c   | 41 +++++++++++++++++++++++++++++++++--------
 2 files changed, 43 insertions(+), 9 deletions(-)
Signature · 1 line
2.43.0
author
#1 lore raw

PATCH v3 1/2 btrfs: use on-disk uuid for s_uuid in temp_fsid mounts

When mounting a cloned filesystem with a temporary fsuuid (temp_fsid), layered modules like overlayfs require a persistent identifier.

While internal in-memory fs_devices->fsid must remain unique to the kernel module, let s_uuid carry the original on-disk UUID.

---
 fs/btrfs/disk-io.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
fs/btrfs/disk-io.c +10 -1
index 2020351e715b..0771903f82ba 100644
@@ -3447,7 +3447,16 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
 	/* Update the values for the current filesystem. */
 	sb->s_blocksize = sectorsize;
 	sb->s_blocksize_bits = blksize_bits(sectorsize);
-	memcpy(&sb->s_uuid, fs_info->fs_devices->fsid, BTRFS_FSID_SIZE);
+	/*
+	 * When temp_fsid is active, fs_devices->fsid is assigned a random UUID
+	 * at mount. This inconsistent UUID causes issues for layered filesystems
+	 * like OverlayFS. Since metadata_uuid may or may not be set, provide the
+	 * on-disk UUID directly from the super_copy.
+	 */
+	if (fs_info->fs_devices->temp_fsid)
+		memcpy(&sb->s_uuid, fs_info->super_copy->fsid, BTRFS_FSID_SIZE);
+	else
+		memcpy(&sb->s_uuid, fs_info->fs_devices->fsid, BTRFS_FSID_SIZE);
 
 	mutex_lock(&fs_info->chunk_mutex);
 	ret = btrfs_read_sys_array(fs_info);
Signature · 1 line
2.43.0
author
#2 lore raw

PATCH v3 2/2 btrfs: derive f_fsid from on-disk fsuuid and dev_t

In reply to Anand Jain

f_fsid depends on fs_devices->fsid and subvol root id.

For cloned devices either same as the source or dynamical generated at mount won't suite because tools like fanotify and ima depends on it.

Switch to a stable derivation using the persistent on-disk fsuuid + root id + devt of the block device for the single device filesystem when temp_fsid is active.
This is consistent as long as the device remains unchanged/replace (excludes btrfs device replace secnario for now).

---
 fs/btrfs/super.c | 41 +++++++++++++++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 8 deletions(-)
fs/btrfs/super.c +33 -8
index a63ad6bd89f6..00eff951917c 100644
@@ -1732,12 +1732,13 @@ static int btrfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	u64 total_free_data = 0;
 	u64 total_free_meta = 0;
 	u32 bits = fs_info->sectorsize_bits;
-	__be32 *fsid = (__be32 *)fs_info->fs_devices->fsid;
+	__be32 *fsid;
 	unsigned factor = 1;
 	struct btrfs_block_rsv *block_rsv = &fs_info->global_block_rsv;
 	int ret;
 	u64 thresh = 0;
 	int mixed = 0;
+	__kernel_fsid_t f_fsid;
 
 	list_for_each_entry(found, &fs_info->space_info, list) {
 		if (found->flags & BTRFS_BLOCK_GROUP_DATA) {
@@ -1818,14 +1819,38 @@ static int btrfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	buf->f_bsize = fs_info->sectorsize;
 	buf->f_namelen = BTRFS_NAME_LEN;
 
-	/* We treat it as constant endianness (it doesn't matter _which_)
-	   because we want the fsid to come out the same whether mounted
-	   on a big-endian or little-endian host */
-	buf->f_fsid.val[0] = be32_to_cpu(fsid[0]) ^ be32_to_cpu(fsid[2]);
-	buf->f_fsid.val[1] = be32_to_cpu(fsid[1]) ^ be32_to_cpu(fsid[3]);
+	/*
+	 * fs_devices->fsid is dynamically generated when temp_fsid is active
+	 * to support cloned filesystems. Use the original on-disk fsid instead,
+	 * as it remains consistent across mount cycles.
+	 */
+	if (fs_info->fs_devices->temp_fsid)
+		fsid = (__be32 *)fs_info->super_copy->fsid;
+	else
+		fsid = (__be32 *)fs_info->fs_devices->fsid;
+
+	/*
+	 * We treat it as constant endianness (it doesn't matter _which_)
+	 * because we want the fsid to come out the same whether mounted
+	 * on a big-endian or little-endian host.
+	 */
+	f_fsid.val[0] = be32_to_cpu(fsid[0]) ^ be32_to_cpu(fsid[2]);
+	f_fsid.val[1] = be32_to_cpu(fsid[1]) ^ be32_to_cpu(fsid[3]);
+
 	/* Mask in the root object ID too, to disambiguate subvols */
-	buf->f_fsid.val[0] ^= btrfs_root_id(BTRFS_I(d_inode(dentry))->root) >> 32;
-	buf->f_fsid.val[1] ^= btrfs_root_id(BTRFS_I(d_inode(dentry))->root);
+	f_fsid.val[0] ^= btrfs_root_id(BTRFS_I(d_inode(dentry))->root) >> 32;
+	f_fsid.val[1] ^= btrfs_root_id(BTRFS_I(d_inode(dentry))->root);
+
+	/* Hash dev_t to avoid f_fsid collision with cloned filesystems. */
+	if (fs_info->fs_devices->total_devices == 1) {
+		__kernel_fsid_t dev_fsid = \
+	u64_to_fsid(huge_encode_dev(fs_info->fs_devices->latest_dev->bdev->bd_dev));
+
+		f_fsid.val[0] ^= dev_fsid.val[1];
+		f_fsid.val[1] ^= dev_fsid.val[0];
+	}
+
+	memcpy(&buf->f_fsid, &f_fsid, sizeof(f_fsid));
 
 	return 0;
 }
Signature · 1 line
2.43.0
#3 lore raw

Re: [PATCH v3 2/2] btrfs: derive f_fsid from on-disk fsuuid and dev_t

Quoted 11 lines · Anand Jain

f_fsid depends on fs_devices->fsid and subvol root id.

For cloned devices either same as the source or dynamical generated at mount won't suite because tools like fanotify and ima depends on it.

Switch to a stable derivation using the persistent on-disk fsuuid + root id + devt of the block device for the single device filesystem when temp_fsid is active.
This is consistent as long as the device remains unchanged/replace (excludes btrfs device replace secnario for now).

David,

The changelog was inaccurate per Sashiko. I missed updating it to reflect the final implementation.
I have corrected it below to match the current code changes. I hope below change log can be used during merge.

-----------
btrfs: derive f_fsid from on-disk fsid and dev_t

The f_fsid was originally derived from fs_devices->fsid and the subvolume root ID. However, when temp_fsid is active, fs_devices->fsid is randomized, making the standard derivation inconsistent.

Since metadata_uuid is optional, it is not a reliable alternative. This patch instead retrieves the on-disk UUID from fs_info->super_copy->fsid.

To prevent f_fsid collisions between original and cloned filesystems, this implementation hashes the dev_t for single-device btrfs filesystems to ensure uniqueness. This is limited to single-device filesystems as cloned mounts are currently only supported for that configuration. Note that f_fsid will change if the device is replaced. Additionally, since the kernel cannot distinguish between the original and the cloned filesystem, this new f_fsid derivation is applied to both.
---------------

Thanks
Anand

Quoted 70 lines
---
 fs/btrfs/super.c | 41 +++++++++++++++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 8 deletions(-)
fs/btrfs/super.c +33 -8
index a63ad6bd89f6..00eff951917c 100644
@@ -1732,12 +1732,13 @@ static int btrfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	u64 total_free_data = 0;
 	u64 total_free_meta = 0;
 	u32 bits = fs_info->sectorsize_bits;
-	__be32 *fsid = (__be32 *)fs_info->fs_devices->fsid;
+	__be32 *fsid;
 	unsigned factor = 1;
 	struct btrfs_block_rsv *block_rsv = &fs_info->global_block_rsv;
 	int ret;
 	u64 thresh = 0;
 	int mixed = 0;
+	__kernel_fsid_t f_fsid;
 
 	list_for_each_entry(found, &fs_info->space_info, list) {
 		if (found->flags & BTRFS_BLOCK_GROUP_DATA) {
@@ -1818,14 +1819,38 @@ static int btrfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	buf->f_bsize = fs_info->sectorsize;
 	buf->f_namelen = BTRFS_NAME_LEN;
 
-	/* We treat it as constant endianness (it doesn't matter _which_)
-	   because we want the fsid to come out the same whether mounted
-	   on a big-endian or little-endian host */
-	buf->f_fsid.val[0] = be32_to_cpu(fsid[0]) ^ be32_to_cpu(fsid[2]);
-	buf->f_fsid.val[1] = be32_to_cpu(fsid[1]) ^ be32_to_cpu(fsid[3]);
+	/*
+	 * fs_devices->fsid is dynamically generated when temp_fsid is active
+	 * to support cloned filesystems. Use the original on-disk fsid instead,
+	 * as it remains consistent across mount cycles.
+	 */
+	if (fs_info->fs_devices->temp_fsid)
+		fsid = (__be32 *)fs_info->super_copy->fsid;
+	else
+		fsid = (__be32 *)fs_info->fs_devices->fsid;
+
+	/*
+	 * We treat it as constant endianness (it doesn't matter _which_)
+	 * because we want the fsid to come out the same whether mounted
+	 * on a big-endian or little-endian host.
+	 */
+	f_fsid.val[0] = be32_to_cpu(fsid[0]) ^ be32_to_cpu(fsid[2]);
+	f_fsid.val[1] = be32_to_cpu(fsid[1]) ^ be32_to_cpu(fsid[3]);
+
 	/* Mask in the root object ID too, to disambiguate subvols */
-	buf->f_fsid.val[0] ^= btrfs_root_id(BTRFS_I(d_inode(dentry))->root) >> 32;
-	buf->f_fsid.val[1] ^= btrfs_root_id(BTRFS_I(d_inode(dentry))->root);
+	f_fsid.val[0] ^= btrfs_root_id(BTRFS_I(d_inode(dentry))->root) >> 32;
+	f_fsid.val[1] ^= btrfs_root_id(BTRFS_I(d_inode(dentry))->root);
+
+	/* Hash dev_t to avoid f_fsid collision with cloned filesystems. */
+	if (fs_info->fs_devices->total_devices == 1) {
+		__kernel_fsid_t dev_fsid = \
+	u64_to_fsid(huge_encode_dev(fs_info->fs_devices->latest_dev->bdev->bd_dev));
+
+		f_fsid.val[0] ^= dev_fsid.val[1];
+		f_fsid.val[1] ^= dev_fsid.val[0];
+	}
+
+	memcpy(&buf->f_fsid, &f_fsid, sizeof(f_fsid));
 
 	return 0;
 }
#4 lore raw

Re: [PATCH v3 2/2] btrfs: derive f_fsid from on-disk fsuuid and dev_t

Quoted 13 lines · Anand Jain
Quoted 11 lines · Anand Jain

f_fsid depends on fs_devices->fsid and subvol root id.

For cloned devices either same as the source or dynamical generated at mount won't suite because tools like fanotify and ima depends on it.

Switch to a stable derivation using the persistent on-disk fsuuid + root id + devt of the block device for the single device filesystem when temp_fsid is active.
This is consistent as long as the device remains unchanged/replace (excludes btrfs device replace secnario for now).

Quoted 6 lines

David,

The changelog was inaccurate per Sashiko. I missed updating it to reflect the final implementation.
I have corrected it below to match the current code changes. I hope below change log can be used during merge.

Changelog replaced and patches added to for-next. Thanks.