public inbox for linux-kernel-mentees@lists.linux-foundation.org
 help / color / mirror / Atom feed
* [PATCH] gfs2: flush withdraw work before freeing gfs2_sbd
       [not found] <68f6a48f.050a0220.91a22.0451.GAE@google.com>
@ 2025-10-24 14:43 ` Nirbhay Sharma
       [not found]   ` <8f270474-7ced-4668-97da-f3d7709a82e7@gmail.com>
  0 siblings, 1 reply; 3+ messages in thread
From: Nirbhay Sharma @ 2025-10-24 14:43 UTC (permalink / raw)
  To: Andreas Gruenbacher
  Cc: gfs2, linux-kernel, syzbot+19e0be39cc25dfcb0858, skhan,
	david.hunter.linux, linux-kernel-mentees, Nirbhay Sharma

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);
-- 
2.48.1


^ permalink raw reply related	[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: 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

end of thread, other threads:[~2025-11-16 20:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <68f6a48f.050a0220.91a22.0451.GAE@google.com>
2025-10-24 14:43 ` [PATCH] gfs2: flush withdraw work before freeing gfs2_sbd Nirbhay Sharma
     [not found]   ` <8f270474-7ced-4668-97da-f3d7709a82e7@gmail.com>
2025-11-15 16:12     ` David Hunter
2025-11-16 20:12     ` Andreas Gruenbacher

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