Re: [PATCH] gfs2: flush withdraw work before freeing gfs2_sbd

[David Hunter <david.hunter.linux@gmail.com>]

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