| Message ID | 20240904020949.1685198-1-joseph.qi@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ocfs2: cancel dqi_sync_work before freeing oinfo | expand |
On 9/4/24 10:09, Joseph Qi wrote: > ocfs2_global_read_info() will schedule dqi_sync_work. So any error > occurs after it, we have to cancel this delayed work first before > freeing oinfo, otherwise it will trigger the following warning with > CONFIG_DEBUG_OBJECTS_* enabled: > > ODEBUG: free active (active state 0) object: 00000000d8b0ce28 object type: timer_list hint: qsync_work_fn+0x0/0x16c > > Link: https://syzkaller.appspot.com/bug?extid=f7af59df5d6b25f0febd > Reported-by: syzbot+f7af59df5d6b25f0febd@syzkaller.appspotmail.com > Tested-by: syzbot+f7af59df5d6b25f0febd@syzkaller.appspotmail.com > Fixes: 171bf93ce11f ("ocfs2: Periodic quota syncing") > Cc: stable@vger.kernel.org > Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com> > --- > fs/ocfs2/quota_local.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/ocfs2/quota_local.c b/fs/ocfs2/quota_local.c > index 8ce462c64c51..ebe0dbc8db4a 100644 > --- a/fs/ocfs2/quota_local.c > +++ b/fs/ocfs2/quota_local.c > @@ -782,6 +782,7 @@ static int ocfs2_local_read_info(struct super_block *sb, int type) > if (locked) > ocfs2_inode_unlock(lqinode, 1); > ocfs2_release_local_quota_bitmaps(&oinfo->dqi_chunk); > + cancel_delayed_work_sync(&oinfo->dqi_sync_work); > kfree(oinfo); > } > brelse(bh); In my view, there is one problem: If ocfs2_global_read_info() returns error before ->dqi_sync_work is initialized, above code will trigger wild pointer error. -Heming
On 9/4/24 10:56 AM, Heming Zhao wrote: > On 9/4/24 10:09, Joseph Qi wrote: >> ocfs2_global_read_info() will schedule dqi_sync_work. So any error >> occurs after it, we have to cancel this delayed work first before >> freeing oinfo, otherwise it will trigger the following warning with >> CONFIG_DEBUG_OBJECTS_* enabled: >> >> ODEBUG: free active (active state 0) object: 00000000d8b0ce28 object type: timer_list hint: qsync_work_fn+0x0/0x16c >> >> Link: https://syzkaller.appspot.com/bug?extid=f7af59df5d6b25f0febd >> Reported-by: syzbot+f7af59df5d6b25f0febd@syzkaller.appspotmail.com >> Tested-by: syzbot+f7af59df5d6b25f0febd@syzkaller.appspotmail.com >> Fixes: 171bf93ce11f ("ocfs2: Periodic quota syncing") >> Cc: stable@vger.kernel.org >> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com> >> --- >> fs/ocfs2/quota_local.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/fs/ocfs2/quota_local.c b/fs/ocfs2/quota_local.c >> index 8ce462c64c51..ebe0dbc8db4a 100644 >> --- a/fs/ocfs2/quota_local.c >> +++ b/fs/ocfs2/quota_local.c >> @@ -782,6 +782,7 @@ static int ocfs2_local_read_info(struct super_block *sb, int type) >> if (locked) >> ocfs2_inode_unlock(lqinode, 1); >> ocfs2_release_local_quota_bitmaps(&oinfo->dqi_chunk); >> + cancel_delayed_work_sync(&oinfo->dqi_sync_work); >> kfree(oinfo); >> } >> brelse(bh); > > In my view, there is one problem: > If ocfs2_global_read_info() returns error before ->dqi_sync_work is > initialized, above code will trigger wild pointer error. > You're right. Will update and send v2. Thanks, Joseph
diff --git a/fs/ocfs2/quota_local.c b/fs/ocfs2/quota_local.c index 8ce462c64c51..ebe0dbc8db4a 100644 --- a/fs/ocfs2/quota_local.c +++ b/fs/ocfs2/quota_local.c @@ -782,6 +782,7 @@ static int ocfs2_local_read_info(struct super_block *sb, int type) if (locked) ocfs2_inode_unlock(lqinode, 1); ocfs2_release_local_quota_bitmaps(&oinfo->dqi_chunk); + cancel_delayed_work_sync(&oinfo->dqi_sync_work); kfree(oinfo); } brelse(bh);