* [PATCH v4] xfs: fix ag count overflow during growfs
@ 2023-06-02 2:18 Long Li
2023-06-02 14:27 ` Darrick J. Wong
0 siblings, 1 reply; 2+ messages in thread
From: Long Li @ 2023-06-02 2:18 UTC (permalink / raw)
To: djwong; +Cc: david, linux-xfs, houtao1, yi.zhang, guoxuenan
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)
+ 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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v4] xfs: fix ag count overflow during growfs
2023-06-02 2:18 [PATCH v4] xfs: fix ag count overflow during growfs Long Li
@ 2023-06-02 14:27 ` Darrick J. Wong
0 siblings, 0 replies; 2+ messages in thread
From: Darrick J. Wong @ 2023-06-02 14:27 UTC (permalink / raw)
To: Long Li; +Cc: david, linux-xfs, houtao1, yi.zhang, guoxuenan
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
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-06-02 14:27 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-02 2:18 [PATCH v4] xfs: fix ag count overflow during growfs Long Li
2023-06-02 14:27 ` Darrick J. Wong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox