Commit f66af88e authored by Andreas Gruenbacher's avatar Andreas Gruenbacher

gfs2: Stop using gfs2_make_fs_ro for withdraw

[   81.372851][ T5532] CPU: 1 PID: 5532 Comm: syz-executor.0 Not tainted 6.2.0-rc1-syzkaller-dirty #0
[   81.382080][ T5532] Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/12/2023
[   81.392343][ T5532] Call Trace:
[   81.395654][ T5532]  <TASK>
[   81.398603][ T5532]  dump_stack_lvl+0x1b1/0x290
[   81.418421][ T5532]  gfs2_assert_warn_i+0x19a/0x2e0
[   81.423480][ T5532]  gfs2_quota_cleanup+0x4c6/0x6b0
[   81.428611][ T5532]  gfs2_make_fs_ro+0x517/0x610
[   81.457802][ T5532]  gfs2_withdraw+0x609/0x1540
[   81.481452][ T5532]  gfs2_inode_refresh+0xb2d/0xf60
[   81.506658][ T5532]  gfs2_instantiate+0x15e/0x220
[   81.511504][ T5532]  gfs2_glock_wait+0x1d9/0x2a0
[   81.516352][ T5532]  do_sync+0x485/0xc80
[   81.554943][ T5532]  gfs2_quota_sync+0x3da/0x8b0
[   81.559738][ T5532]  gfs2_sync_fs+0x49/0xb0
[   81.564063][ T5532]  sync_filesystem+0xe8/0x220
[   81.568740][ T5532]  generic_shutdown_super+0x6b/0x310
[   81.574112][ T5532]  kill_block_super+0x79/0xd0
[   81.578779][ T5532]  deactivate_locked_super+0xa7/0xf0
[   81.584064][ T5532]  cleanup_mnt+0x494/0x520
[   81.593753][ T5532]  task_work_run+0x243/0x300
[   81.608837][ T5532]  exit_to_user_mode_loop+0x124/0x150
[   81.614232][ T5532]  exit_to_user_mode_prepare+0xb2/0x140
[   81.619820][ T5532]  syscall_exit_to_user_mode+0x26/0x60
[   81.625287][ T5532]  do_syscall_64+0x49/0xb0
[   81.629710][ T5532]  entry_SYSCALL_64_after_hwframe+0x63/0xcd

In this backtrace, gfs2_quota_sync() takes quota data references and
then calls do_sync().  Function do_sync() encounters filesystem
corruption and withdraws the filesystem, which (among other things) calls
gfs2_quota_cleanup().  Function gfs2_quota_cleanup() wrongly assumes
that nobody is holding any quota data references anymore, and destroys
all quota data objects.  When gfs2_quota_sync() then resumes and
dereferences the quota data objects it is holding, those objects are no
longer there.

Function gfs2_quota_cleanup() deals with resource deallocation and can
easily be delayed until gfs2_put_super() in the case of a filesystem
withdraw.  In fact, most of the other work gfs2_make_fs_ro() does is
unnecessary during a withdraw as well, so change signal_our_withdraw()
to skip gfs2_make_fs_ro() and perform the necessary steps directly
instead.

Thanks to Edward Adam Davis <eadavis@sina.com> for the initial patches.

Link: https://lore.kernel.org/all/0000000000002b5e2405f14e860f@google.com
Reported-by: syzbot+3f6a670108ce43356017@syzkaller.appspotmail.com
Signed-off-by: default avatarAndreas Gruenbacher <agruenba@redhat.com>
parent a475c5dd
...@@ -580,15 +580,8 @@ void gfs2_make_fs_ro(struct gfs2_sbd *sdp) ...@@ -580,15 +580,8 @@ void gfs2_make_fs_ro(struct gfs2_sbd *sdp)
gfs2_log_is_empty(sdp), gfs2_log_is_empty(sdp),
HZ * 5); HZ * 5);
gfs2_assert_warn(sdp, gfs2_log_is_empty(sdp)); gfs2_assert_warn(sdp, gfs2_log_is_empty(sdp));
} else {
wait_event_timeout(sdp->sd_log_waitq,
gfs2_log_is_empty(sdp),
HZ * 5);
} }
gfs2_quota_cleanup(sdp); gfs2_quota_cleanup(sdp);
if (!log_write_allowed)
sdp->sd_vfs->s_flags |= SB_RDONLY;
} }
/** /**
...@@ -622,6 +615,8 @@ static void gfs2_put_super(struct super_block *sb) ...@@ -622,6 +615,8 @@ static void gfs2_put_super(struct super_block *sb)
if (!sb_rdonly(sb)) { if (!sb_rdonly(sb)) {
gfs2_make_fs_ro(sdp); gfs2_make_fs_ro(sdp);
} }
if (gfs2_withdrawn(sdp))
gfs2_quota_cleanup(sdp);
WARN_ON(gfs2_withdrawing(sdp)); WARN_ON(gfs2_withdrawing(sdp));
/* At this point, we're through modifying the disk */ /* At this point, we're through modifying the disk */
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include <linux/spinlock.h> #include <linux/spinlock.h>
#include <linux/completion.h> #include <linux/completion.h>
#include <linux/buffer_head.h> #include <linux/buffer_head.h>
#include <linux/kthread.h>
#include <linux/crc32.h> #include <linux/crc32.h>
#include <linux/gfs2_ondisk.h> #include <linux/gfs2_ondisk.h>
#include <linux/delay.h> #include <linux/delay.h>
...@@ -150,7 +151,23 @@ static void signal_our_withdraw(struct gfs2_sbd *sdp) ...@@ -150,7 +151,23 @@ static void signal_our_withdraw(struct gfs2_sbd *sdp)
if (!sb_rdonly(sdp->sd_vfs)) { if (!sb_rdonly(sdp->sd_vfs)) {
bool locked = mutex_trylock(&sdp->sd_freeze_mutex); bool locked = mutex_trylock(&sdp->sd_freeze_mutex);
gfs2_make_fs_ro(sdp); if (sdp->sd_quotad_process &&
current != sdp->sd_quotad_process) {
kthread_stop(sdp->sd_quotad_process);
sdp->sd_quotad_process = NULL;
}
if (sdp->sd_logd_process &&
current != sdp->sd_logd_process) {
kthread_stop(sdp->sd_logd_process);
sdp->sd_logd_process = NULL;
}
wait_event_timeout(sdp->sd_log_waitq,
gfs2_log_is_empty(sdp),
HZ * 5);
sdp->sd_vfs->s_flags |= SB_RDONLY;
if (locked) if (locked)
mutex_unlock(&sdp->sd_freeze_mutex); mutex_unlock(&sdp->sd_freeze_mutex);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment