* Re: [PATCH] gfs2: flush withdraw work before freeing gfs2_sbd
[not found] ` <8f270474-7ced-4668-97da-f3d7709a82e7@gmail.com>
@ 2025-11-15 16:12 ` David Hunter
2025-11-16 20:12 ` Andreas Gruenbacher
1 sibling, 0 replies; 3+ messages in thread
From: David Hunter @ 2025-11-15 16:12 UTC (permalink / raw)
To: Nirbhay Sharma, Andreas Gruenbacher
Cc: gfs2, linux-kernel, syzbot+19e0be39cc25dfcb0858, skhan,
linux-kernel-mentees
On 11/13/25 15:24, Nirbhay Sharma wrote:
> Hi Andreas,
>
> I hope this email finds you well.
>
> I'm writing to follow up on the GFS2 patch I submitted regarding the
> ODEBUG warning in free_sbd(). The patch addressed the syzbot report
> where sd_withdraw_work was being freed while still active.
>
> I wanted to check if you've had a chance to review the patch, or if
> there's any feedback or additional information I can provide to help
> with the review process.
>
> I understand maintainers are busy, and I'm happy to make any necessary
> revisions or provide further clarification on the testing that was
> performed.
>
> Best regards,
> Nirbhay Sharma
>
>
Hey Nirbay,
The reply that you write should be below the message you are talking
about. If you put it above, it is called "top-posting", and the kernel
community does not like top-posting.
For long email chains, it becomes difficult to keep track of the
conversations.
What I am doing now is called in-line posting. I am responding below
your message, but there are still other messages below mine.
> On 10/24/25 8:13 PM, Nirbhay Sharma wrote:
>> Syzbot reported an ODEBUG warning where free_sbd() was freeing memory
>> containing an active work_struct (sd_withdraw_work):
>>
>> ODEBUG: free active (active state 0) object: ffff888026c285a0
>> object type: work_struct hint: gfs2_withdraw_func+0x0/0x430
>> WARNING: CPU: 0 PID: 6010 at lib/debugobjects.c:545
>> Call Trace:
>> free_sbd+0x1e4/0x270 fs/gfs2/ops_fstype.c:1308
>>
>> The issue occurs when gfs2_fill_super() fails after initializing
>> sd_withdraw_work at line 1218. Some error paths (fail_lm, fail_debug,
>> etc.) skip the existing flush_work() at the fail_inodes label and jump
>> directly to fail_free, which calls free_sbd() without flushing the
>> potentially pending work.
>>
>> free_sbd() is also called from init_sbd()'s error path before
>> sd_withdraw_work is initialized. Since the structure is allocated with
>> kzalloc(), work.func is NULL in this case.
>>
>> Fix by adding a guarded flush_work() to free_sbd(). Check work.func
>> before flushing to handle both cases: when called after INIT_WORK()
>> (work must be flushed), and when called before INIT_WORK() (work.func
>> is NULL, skip flushing). This avoids the WARN_ON(!work->func) in
>> __flush_work().
>>
>> Note: gfs2_put_super() already calls flush_work() before free_sbd()
>> (line 606), so the flush in free_sbd() will be redundant but harmless
>> for the normal unmount path.
>>
>> Reported-by: syzbot+19e0be39cc25dfcb0858@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=19e0be39cc25dfcb0858
>> Fixes: 8fdd8a28fe5c ("gfs2: Asynchronous withdraw")
>> Signed-off-by: Nirbhay Sharma <nirbhay.lkd@gmail.com>
>> ---
>> Testing performed:
>> - Reproduced original bug with syzbot C reproducer
>> - Verified fix prevents ODEBUG warnings in all error paths
>> - Tested early mount failures (unformatted devices)
>> - Tested all gfs2_fill_super error paths (4 scenarios)
>> - Parallel mount stress test (3 concurrent operations)
>> - Memory leak test (50 mount/unmount cycles, <4MB variance)
>> - Race condition testing passed
>> - Validated with syzbot on linux-next (Oct 22)
>> - All tests completed with zero ODEBUG warnings
>>
>> fs/gfs2/ops_fstype.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
>> index 08502d967e71..6cea03410e57 100644
>> --- a/fs/gfs2/ops_fstype.c
>> +++ b/fs/gfs2/ops_fstype.c
>> @@ -67,6 +67,14 @@ void free_sbd(struct gfs2_sbd *sdp)
>> {
>> struct super_block *sb = sdp->sd_vfs;
>>
>> + /*
>> + * Only flush withdraw work if initialized. Work is initialized in
>> + * gfs2_fill_super() at line 1218, after init_sbd() succeeds.
>> + * Checking func avoids WARN_ON in __flush_work() for early failures.
>> + */
>> + if (sdp->sd_withdraw_work.func)
>> + flush_work(&sdp->sd_withdraw_work);
>> +
>> free_percpu(sdp->sd_lkstats);
>> sb->s_fs_info = NULL;
>> kfree(sdp);
If I respond down here, it is bottom-posting. Both inline and bottom
posting are encouraged. Top posting is to be avoided.
Thanks,
David Hunter
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] gfs2: flush withdraw work before freeing gfs2_sbd
[not found] ` <8f270474-7ced-4668-97da-f3d7709a82e7@gmail.com>
2025-11-15 16:12 ` David Hunter
@ 2025-11-16 20:12 ` Andreas Gruenbacher
1 sibling, 0 replies; 3+ messages in thread
From: Andreas Gruenbacher @ 2025-11-16 20:12 UTC (permalink / raw)
To: Nirbhay Sharma
Cc: Andreas Gruenbacher, gfs2, linux-kernel,
syzbot+19e0be39cc25dfcb0858, skhan, david.hunter.linux,
linux-kernel-mentees
Hi Nirbhay,
On Thu, Nov 13, 2025 at 9:24 PM Nirbhay Sharma <nirbhay.lkd@gmail.com> wrote:
> Hi Andreas,
>
> I hope this email finds you well.
>
> I'm writing to follow up on the GFS2 patch I submitted regarding the ODEBUG warning in free_sbd(). The patch addressed the syzbot report where sd_withdraw_work was being freed while still active.
>
> I wanted to check if you've had a chance to review the patch, or if there's any feedback or additional information I can provide to help with the review process.
>
> I understand maintainers are busy, and I'm happy to make any necessary revisions or provide further clarification on the testing that was performed.
thanks for looking into this issue, but this is already fixed in the
updated version of patch "gfs2: Asynchronous withdraw" which has been on
gfs2 for-next since at least October 23. Below is my isolated fix.
Thanks,
Andreas
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 2d177aa21ffd..c42982bdd4b2 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -1294,7 +1294,6 @@ static int gfs2_fill_super(struct super_block *sb, struct fs_context *fc)
fail_per_node:
init_per_node(sdp, UNDO);
fail_inodes:
- flush_work(&sdp->sd_withdraw_work);
init_inodes(sdp, UNDO);
fail_sb:
if (sdp->sd_root_dir)
diff --git a/fs/gfs2/util.c b/fs/gfs2/util.c
index fff0b0e2e27c..c454bea101de 100644
--- a/fs/gfs2/util.c
+++ b/fs/gfs2/util.c
@@ -345,6 +345,12 @@ void gfs2_withdraw(struct gfs2_sbd *sdp)
} while (unlikely(!try_cmpxchg(&sdp->sd_flags, &old, new)));
dump_stack();
+ /*
+ * There is no need to withdraw when the superblock hasn't been
+ * fully initialized, yet.
+ */
+ if (!(sdp->sd_vfs->s_flags & SB_BORN))
+ return;
fs_err(sdp, "about to withdraw this file system\n");
schedule_work(&sdp->sd_withdraw_work);
}
^ permalink raw reply related [flat|nested] 3+ messages in thread