public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xfs: fix integer overflow in xlog_grant_head_check
@ 2024-12-10 11:54 cem
  2024-12-10 22:55 ` Dave Chinner
  2024-12-11 22:33 ` Carlos Maiolino
  0 siblings, 2 replies; 5+ messages in thread
From: cem @ 2024-12-10 11:54 UTC (permalink / raw)
  To: linux-xfs; +Cc: cem, hch, djwong, dchinner

From: Carlos Maiolino <cmaiolino@redhat.com>

I tripped over an integer overflow when using a big journal size.

Essentially I can reliably reproduce it using:

mkfs.xfs -f -lsize=393216b -f -b size=4096 -m crc=1,reflink=1,rmapbt=1, \
-i sparse=1 /dev/vdb2 > /dev/null
mount -o usrquota,grpquota,prjquota /dev/vdb2 /mnt
xfs_io -x -c 'shutdown -f' /mnt
umount /mnt
mount -o usrquota,grpquota,prjquota /dev/vdb2 /mnt

The last mount command get stuck on the following path:

[<0>] xlog_grant_head_wait+0x5d/0x2a0 [xfs]
[<0>] xlog_grant_head_check+0x112/0x180 [xfs]
[<0>] xfs_log_reserve+0xe3/0x260 [xfs]
[<0>] xfs_trans_reserve+0x179/0x250 [xfs]
[<0>] xfs_trans_alloc+0x101/0x260 [xfs]
[<0>] xfs_sync_sb+0x3f/0x80 [xfs]
[<0>] xfs_qm_mount_quotas+0xe3/0x2f0 [xfs]
[<0>] xfs_mountfs+0x7ad/0xc20 [xfs]
[<0>] xfs_fs_fill_super+0x762/0xa50 [xfs]
[<0>] get_tree_bdev_flags+0x131/0x1d0
[<0>] vfs_get_tree+0x26/0xd0
[<0>] vfs_cmd_create+0x59/0xe0
[<0>] __do_sys_fsconfig+0x4e3/0x6b0
[<0>] do_syscall_64+0x82/0x160
[<0>] entry_SYSCALL_64_after_hwframe+0x76/0x7e

By investigating it a bit, I noticed that xlog_grant_head_check (called
from xfs_log_reserve), defines free_bytes as an integer, which in turn
is used to store the value from xlog_grant_space_left().
xlog_grant_space_left() however, does return a uint64_t, and, giving a
big enough journal size, it can overflow the free_bytes in
xlog_grant_head_check(), resulting int the conditional:

else if (free_bytes < *need_bytes) {

in xlog_grant_head_check() to evaluate to true and cause xfsaild to try
to flush the log indefinitely, which seems to be causing xfs to get
stuck in xlog_grant_head_wait() indefinitely.

I'm adding a fixes tag as a suggestion from hch, giving that after the
aforementioned patch, all xlog_grant_space_left() callers should store
the return value on a 64bit type.

Fixes: c1220522ef40 ("xfs: grant heads track byte counts, not LSNs")
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---

I'd like to add a caveat here, because I don't properly understand the
journal code/mechanism yet. It does seem to me that it is feasible to
have the reserve grant head to go to a big number and indeed cause the
overflow, but I'm not completely sure that what I'm fixing is a real bug
or if just the symptom of something else (or maybe a bug that triggeded
another overflow bug :)


 fs/xfs/xfs_log.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 05daad8a8d34..a799821393b5 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -222,7 +222,7 @@ STATIC bool
 xlog_grant_head_wake(
 	struct xlog		*log,
 	struct xlog_grant_head	*head,
-	int			*free_bytes)
+	uint64_t		*free_bytes)
 {
 	struct xlog_ticket	*tic;
 	int			need_bytes;
@@ -302,7 +302,7 @@ xlog_grant_head_check(
 	struct xlog_ticket	*tic,
 	int			*need_bytes)
 {
-	int			free_bytes;
+	uint64_t		free_bytes;
 	int			error = 0;
 
 	ASSERT(!xlog_in_recovery(log));
@@ -1088,7 +1088,7 @@ xfs_log_space_wake(
 	struct xfs_mount	*mp)
 {
 	struct xlog		*log = mp->m_log;
-	int			free_bytes;
+	uint64_t		free_bytes;
 
 	if (xlog_is_shutdown(log))
 		return;
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-12-11 22:33 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-10 11:54 [PATCH] xfs: fix integer overflow in xlog_grant_head_check cem
2024-12-10 22:55 ` Dave Chinner
2024-12-11 11:55   ` Carlos Maiolino
2024-12-11 22:24     ` Dave Chinner
2024-12-11 22:33 ` Carlos Maiolino

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox