[RFC-PATCH] xfs: do not update sunit/swidth in the superblock to match those provided during mount

Alex Lyakas
2019-11-21T18:09:33Z

We are hitting the following issue: if XFS is mounted with sunit/swidth different from those
specified during mkfs, then xfs_repair reports false corruption and eventually segfaults.

Example:

# mkfs
mkfs.xfs -f -K -p /etc/zadara/xfs.protofile -d sunit=64,swidth=64 -l sunit=32 /dev/vda

#mount with a different sunit/swidth:
mount -onoatime,sync,nouuid,sunit=32,swidth=32 /dev/vda /mnt/xfs

#umount
umount /mnt/xfs

#xfs_repair
xfs_repair -n /dev/vda
# reports false corruption and eventually segfaults[1]

The root cause seems to be that repair/xfs_repair.c::calc_mkfs() calculates the location of first inode chunk based on the current superblock sunit:
/*
* ditto the location of the first inode chunks in the fs ('/')
*/
if (xfs_sb_version_hasdalign(&mp->m_sb) && do_inoalign) {
first_prealloc_ino = XFS_OFFBNO_TO_AGINO(mp, roundup(fino_bno,
mp->m_sb.sb_unit), 0);
...

and then compares to the value in the superblock:

/*
* now the first 3 inodes in the system
*/
if (mp->m_sb.sb_rootino != first_prealloc_ino) {
do_warn(
_("sb root inode value %" PRIu64 " %sinconsistent with calculated value %u\n"),
mp->m_sb.sb_rootino,
(mp->m_sb.sb_rootino == NULLFSINO ? "(NULLFSINO) ":""),
first_prealloc_ino);

if (!no_modify)
do_warn(
_("resetting superblock root inode pointer to %u\n"),
first_prealloc_ino);
else
do_warn(
_("would reset superblock root inode pointer to %u\n"),
first_prealloc_ino);

/*
* just set the value -- safe since the superblock
* doesn't get flushed out if no_modify is set
*/
mp->m_sb.sb_rootino = first_prealloc_ino;
}

and sets the "correct" value into mp->m_sb.sb_rootino.

And from there xfs_repair uses the wrong value, leading to false corruption reports.

Looking at the kernel code of XFS, there seems to be no need to update the superblock sunit/swidth if the mount-provided sunit/swidth are different.
The superblock values are not used during runtime.

With the suggested patch, xfs repair is working properly also when mount-provided sunit/swidth are different.

However, I am not sure whether this is the proper approach. Otherwise, should we not allow specifying different sunit/swidth during mount?

[1]
Phase 1 - find and verify superblock...
- reporting progress in intervals of 15 minutes
sb root inode value 128 inconsistent with calculated value 96
would reset superblock root inode pointer to 96
sb realtime bitmap inode 129 inconsistent with calculated value 97
would reset superblock realtime bitmap ino pointer to 97
sb realtime summary inode 130 inconsistent with calculated value 98
would reset superblock realtime summary ino pointer to 98
Phase 2 - using internal log
- zero log...
- scan filesystem freespace and inode maps...
- 16:09:57: scanning filesystem freespace - 16 of 16 allocation groups done
root inode chunk not found
avl_insert: Warning! duplicate range [96,160]
add_inode - duplicate inode range
Phase 3 - for each AG...
- scan (but don't clear) agi unlinked lists...
- 16:09:57: scanning agi unlinked lists - 16 of 16 allocation groups done
- process known inodes and perform inode discovery...
- agno = 15
- agno = 0
inode 129 not rt bitmap
bad .. entry in directory inode 128, points to self, would clear inode number
inode 129 not rt bitmap
would fix bad flags.
...
Segmentation fault (core dumped)

Signed-off-by: Alex Lyakas <alex@zadara.com>
---
fs/xfs/xfs_mount.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index ba5b6f3..e8263b4 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -399,19 +399,13 @@
}

/*
- * Update superblock with new values
- * and log changes
+ * If sunit/swidth specified during mount do not match
+ * those in the superblock, use the mount-specified values,
+ * but do not update the superblock.
+ * Otherwise, xfs_repair reports false corruption.
+ * Here, only verify that superblock supports data alignment.
*/
- if (xfs_sb_version_hasdalign(sbp)) {
- if (sbp->sb_unit != mp->m_dalign) {
- sbp->sb_unit = mp->m_dalign;
- mp->m_update_sb = true;
- }
- if (sbp->sb_width != mp->m_swidth) {
- sbp->sb_width = mp->m_swidth;
- mp->m_update_sb = true;
- }
- } else {
+ if (!xfs_sb_version_hasdalign(sbp)) {
xfs_warn(mp,
"cannot change alignment: superblock does not support data alignment");
return -EINVAL;
--
1.9.1