| Message ID | 20230602021844.GA3150998@ceph-admin (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v4] xfs: fix ag count overflow during growfs | expand |
On Fri, Jun 02, 2023 at 10:18:44AM +0800, Long Li wrote: > I found a corruption during growfs: > > XFS (loop0): Internal error agbno >= mp->m_sb.sb_agblocks at line 3661 of > file fs/xfs/libxfs/xfs_alloc.c. Caller __xfs_free_extent+0x28e/0x3c0 > CPU: 0 PID: 573 Comm: xfs_growfs Not tainted 6.3.0-rc7-next-20230420-00001-gda8c95746257 > Call Trace: > <TASK> > dump_stack_lvl+0x50/0x70 > xfs_corruption_error+0x134/0x150 > __xfs_free_extent+0x2c1/0x3c0 > xfs_ag_extend_space+0x291/0x3e0 > xfs_growfs_data+0xd72/0xe90 > xfs_file_ioctl+0x5f9/0x14a0 > __x64_sys_ioctl+0x13e/0x1c0 > do_syscall_64+0x39/0x80 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > XFS (loop0): Corruption detected. Unmount and run xfs_repair > XFS (loop0): Internal error xfs_trans_cancel at line 1097 of file > fs/xfs/xfs_trans.c. Caller xfs_growfs_data+0x691/0xe90 > CPU: 0 PID: 573 Comm: xfs_growfs Not tainted 6.3.0-rc7-next-20230420-00001-gda8c95746257 > Call Trace: > <TASK> > dump_stack_lvl+0x50/0x70 > xfs_error_report+0x93/0xc0 > xfs_trans_cancel+0x2c0/0x350 > xfs_growfs_data+0x691/0xe90 > xfs_file_ioctl+0x5f9/0x14a0 > __x64_sys_ioctl+0x13e/0x1c0 > do_syscall_64+0x39/0x80 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > RIP: 0033:0x7f2d86706577 > > The bug can be reproduced with the following sequence: > > # truncate -s 1073741824 xfs_test.img > # mkfs.xfs -f -b size=1024 -d agcount=4 xfs_test.img > # truncate -s 2305843009213693952 xfs_test.img > # mount -o loop xfs_test.img /mnt/test > # xfs_growfs -D 1125899907891200 /mnt/test > > The root cause is that during growfs, user space passed in a large value > of newblcoks to xfs_growfs_data_private(), due to current sb_agblocks is > too small, new AG count will exceed UINT_MAX. Because of AG number type > is unsigned int and it would overflow, that caused nagcount much smaller > than the actual value. During AG extent space, delta blocks in > xfs_resizefs_init_new_ags() will much larger than the actual value due to > incorrect nagcount, even exceed UINT_MAX. This will cause corruption and > be detected in __xfs_free_extent. Fix it by growing the filesystem to up > to the maximally allowed AGs and not return EINVAL when new AG count > overflow. > > Signed-off-by: Long Li <leo.lilong@huawei.com> > --- > v3: > - Ensure that the performance is consisent before and after the modification > when nagcount just overflows and 0 < nb_mod < XFS_MIN_AG_BLOCKS. > - Based on Darrick's advice, growing the filesystem to up to the maximally > allowed AGs when new AG count overflow. > v4: > - Define max ag number follow the definition in xfsprogs. > > fs/xfs/libxfs/xfs_fs.h | 2 ++ > fs/xfs/xfs_fsops.c | 13 +++++++++---- > 2 files changed, 11 insertions(+), 4 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h > index 1cfd5bc6520a..9c60ebb328b4 100644 > --- a/fs/xfs/libxfs/xfs_fs.h > +++ b/fs/xfs/libxfs/xfs_fs.h > @@ -257,6 +257,8 @@ typedef struct xfs_fsop_resblks { > #define XFS_MAX_AG_BLOCKS (XFS_MAX_AG_BYTES / XFS_MIN_BLOCKSIZE) > #define XFS_MAX_CRC_AG_BLOCKS (XFS_MAX_AG_BYTES / XFS_MIN_CRC_BLOCKSIZE) > > +#define XFS_MAX_AGNUMBER ((xfs_agnumber_t)(NULLAGNUMBER - 1)) > + > /* keep the maximum size under 2^31 by a small amount */ > #define XFS_MAX_LOG_BYTES \ > ((2 * 1024 * 1024 * 1024ULL) - XFS_MIN_LOG_BYTES) > diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c > index 13851c0d640b..f03b6cd317a6 100644 > --- a/fs/xfs/xfs_fsops.c > +++ b/fs/xfs/xfs_fsops.c > @@ -115,11 +115,16 @@ xfs_growfs_data_private( > > nb_div = nb; > nb_mod = do_div(nb_div, mp->m_sb.sb_agblocks); > - nagcount = nb_div + (nb_mod != 0); > - if (nb_mod && nb_mod < XFS_MIN_AG_BLOCKS) { > - nagcount--; > - nb = (xfs_rfsblock_t)nagcount * mp->m_sb.sb_agblocks; > + if (nb_mod && nb_mod >= XFS_MIN_AG_BLOCKS) I suspect Dave (or whoever he delegates) is going to change this to XFS_AG_MIN_BLOCKS soon, but we'll figure out how to weave these pieces together. For now, this looks correct to me, so Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > + nb_div++; > + else if (nb_mod) > + nb = nb_div * mp->m_sb.sb_agblocks; > + > + if (nb_div > XFS_MAX_AGNUMBER + 1) { > + nb_div = XFS_MAX_AGNUMBER + 1; > + nb = nb_div * mp->m_sb.sb_agblocks; > } > + nagcount = nb_div; > delta = nb - mp->m_sb.sb_dblocks; > /* > * Reject filesystems with a single AG because they are not > -- > 2.31.1 >
diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h index 1cfd5bc6520a..9c60ebb328b4 100644 --- a/fs/xfs/libxfs/xfs_fs.h +++ b/fs/xfs/libxfs/xfs_fs.h @@ -257,6 +257,8 @@ typedef struct xfs_fsop_resblks { #define XFS_MAX_AG_BLOCKS (XFS_MAX_AG_BYTES / XFS_MIN_BLOCKSIZE) #define XFS_MAX_CRC_AG_BLOCKS (XFS_MAX_AG_BYTES / XFS_MIN_CRC_BLOCKSIZE) +#define XFS_MAX_AGNUMBER ((xfs_agnumber_t)(NULLAGNUMBER - 1)) + /* keep the maximum size under 2^31 by a small amount */ #define XFS_MAX_LOG_BYTES \ ((2 * 1024 * 1024 * 1024ULL) - XFS_MIN_LOG_BYTES) diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c index 13851c0d640b..f03b6cd317a6 100644 --- a/fs/xfs/xfs_fsops.c +++ b/fs/xfs/xfs_fsops.c @@ -115,11 +115,16 @@ xfs_growfs_data_private( nb_div = nb; nb_mod = do_div(nb_div, mp->m_sb.sb_agblocks); - nagcount = nb_div + (nb_mod != 0); - if (nb_mod && nb_mod < XFS_MIN_AG_BLOCKS) { - nagcount--; - nb = (xfs_rfsblock_t)nagcount * mp->m_sb.sb_agblocks; + if (nb_mod && nb_mod >= XFS_MIN_AG_BLOCKS) + nb_div++; + else if (nb_mod) + nb = nb_div * mp->m_sb.sb_agblocks; + + if (nb_div > XFS_MAX_AGNUMBER + 1) { + nb_div = XFS_MAX_AGNUMBER + 1; + nb = nb_div * mp->m_sb.sb_agblocks; } + nagcount = nb_div; delta = nb - mp->m_sb.sb_dblocks; /* * Reject filesystems with a single AG because they are not
I found a corruption during growfs: XFS (loop0): Internal error agbno >= mp->m_sb.sb_agblocks at line 3661 of file fs/xfs/libxfs/xfs_alloc.c. Caller __xfs_free_extent+0x28e/0x3c0 CPU: 0 PID: 573 Comm: xfs_growfs Not tainted 6.3.0-rc7-next-20230420-00001-gda8c95746257 Call Trace: <TASK> dump_stack_lvl+0x50/0x70 xfs_corruption_error+0x134/0x150 __xfs_free_extent+0x2c1/0x3c0 xfs_ag_extend_space+0x291/0x3e0 xfs_growfs_data+0xd72/0xe90 xfs_file_ioctl+0x5f9/0x14a0 __x64_sys_ioctl+0x13e/0x1c0 do_syscall_64+0x39/0x80 entry_SYSCALL_64_after_hwframe+0x63/0xcd XFS (loop0): Corruption detected. Unmount and run xfs_repair XFS (loop0): Internal error xfs_trans_cancel at line 1097 of file fs/xfs/xfs_trans.c. Caller xfs_growfs_data+0x691/0xe90 CPU: 0 PID: 573 Comm: xfs_growfs Not tainted 6.3.0-rc7-next-20230420-00001-gda8c95746257 Call Trace: <TASK> dump_stack_lvl+0x50/0x70 xfs_error_report+0x93/0xc0 xfs_trans_cancel+0x2c0/0x350 xfs_growfs_data+0x691/0xe90 xfs_file_ioctl+0x5f9/0x14a0 __x64_sys_ioctl+0x13e/0x1c0 do_syscall_64+0x39/0x80 entry_SYSCALL_64_after_hwframe+0x63/0xcd RIP: 0033:0x7f2d86706577 The bug can be reproduced with the following sequence: # truncate -s 1073741824 xfs_test.img # mkfs.xfs -f -b size=1024 -d agcount=4 xfs_test.img # truncate -s 2305843009213693952 xfs_test.img # mount -o loop xfs_test.img /mnt/test # xfs_growfs -D 1125899907891200 /mnt/test The root cause is that during growfs, user space passed in a large value of newblcoks to xfs_growfs_data_private(), due to current sb_agblocks is too small, new AG count will exceed UINT_MAX. Because of AG number type is unsigned int and it would overflow, that caused nagcount much smaller than the actual value. During AG extent space, delta blocks in xfs_resizefs_init_new_ags() will much larger than the actual value due to incorrect nagcount, even exceed UINT_MAX. This will cause corruption and be detected in __xfs_free_extent. Fix it by growing the filesystem to up to the maximally allowed AGs and not return EINVAL when new AG count overflow. Signed-off-by: Long Li <leo.lilong@huawei.com> --- v3: - Ensure that the performance is consisent before and after the modification when nagcount just overflows and 0 < nb_mod < XFS_MIN_AG_BLOCKS. - Based on Darrick's advice, growing the filesystem to up to the maximally allowed AGs when new AG count overflow. v4: - Define max ag number follow the definition in xfsprogs. fs/xfs/libxfs/xfs_fs.h | 2 ++ fs/xfs/xfs_fsops.c | 13 +++++++++---- 2 files changed, 11 insertions(+), 4 deletions(-)