Messages in this thread Patch in this message | | | Date | Tue, 30 May 2023 11:57:26 +0200 | From | Jan Kara <> | Subject | Re: [PATCH 1/2] quota: fix null-ptr-deref in ext4_acquire_dquot() |
| |
On Sat 27-05-23 09:40:17, Ye Bin wrote: > Syzbot found the following issue: > Unable to handle kernel paging request at virtual address dfff800000000005 > KASAN: null-ptr-deref in range [0x0000000000000028-0x000000000000002f]
... > CPU: 0 PID: 6080 Comm: syz-executor747 Not tainted 6.3.0-rc7-syzkaller-g14f8db1c0f9a #0 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/14/2023 > pstate: 80400005 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > pc : ext4_acquire_dquot+0x1d4/0x398 fs/ext4/super.c:6766 > lr : dquot_to_inode fs/ext4/super.c:6740 [inline] > lr : ext4_acquire_dquot+0x1ac/0x398 fs/ext4/super.c:6766
OK, this is bad...
> Above issue may happens as follows: > ProcessA ProcessB ProcessC > sys_fsconfig > vfs_fsconfig_locked > reconfigure_super > ext4_remount > dquot_suspend -> suspend all type quota > > sys_fsconfig > vfs_fsconfig_locked > reconfigure_super > ext4_remount > dquot_resume > ret = dquot_load_quota_sb > add_dquot_ref > do_open -> open file O_RDWR > vfs_open > do_dentry_open > get_write_access > atomic_inc_unless_negative(&inode->i_writecount) > ext4_file_open > dquot_file_open > dquot_initialize > __dquot_initialize > dqget > if (!test_bit(DQ_ACTIVE_B, &dquot->dq_flags)) > > __dquot_initialize > __dquot_initialize > dqget > if (!test_bit(DQ_ACTIVE_B, &dquot->dq_flags)) > ext4_acquire_dquot -> Return error > if (ret < 0) > vfs_cleanup_quota_inode > dqopt->files[type] = NULL;
But I don't see how this can happen. The code in dquot_load_quota_sb() looks like:
error = add_dquot_ref(sb, type); if (error) dquot_disable(sb, type, flags);
So if an error happens in add_dquot_ref(), we'll call dquot_disable(). dquot_disable() then does:
drop_dquot_ref(sb, cnt); invalidate_dquots(sb, cnt);
and invalidate_dquots() waits for reference count of all dquots to drop to 0. Hence if dqget() returned a dquot pointer to ProcessC, then ProcessB should wait until ProcessC drops the dquot reference (hence ext4_acquire_dquot() is done).
What am I missing?
Honza
> > ext4_acquire_dquot > -->dquot_to_inode(dquot) is NULL > > To solve above issue, if quota has been loaded, there's unneed to cleaup quota > inode if dquot_load_quota_sb() return failed when do dquot_resume(); > > Reported-by: syzbot+e633c79ceaecbf479854@syzkaller.appspotmail.com > Signed-off-by: Ye Bin <yebin10@huawei.com> > --- > fs/quota/dquot.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c > index ffd40dc3e4e9..4b913faa48ec 100644 > --- a/fs/quota/dquot.c > +++ b/fs/quota/dquot.c > @@ -2476,7 +2476,7 @@ int dquot_resume(struct super_block *sb, int type) > flags = dquot_generic_flag(flags, cnt); > ret = dquot_load_quota_sb(sb, cnt, dqopt->info[cnt].dqi_fmt_id, > flags); > - if (ret < 0) > + if (ret < 0 && !sb_has_quota_loaded(sb, cnt)) > vfs_cleanup_quota_inode(sb, cnt); > } > > -- > 2.31.1 > -- Jan Kara <jack@suse.com> SUSE Labs, CR
| |