Messages in this thread Patch in this message | | | Date | Mon, 5 Jun 2023 13:51:43 +0200 | From | Jan Kara <> | Subject | Re: [PATCH 1/2] quota: fix null-ptr-deref in ext4_acquire_dquot() |
| |
On Sat 03-06-23 14:14:11, yebin (H) wrote: > > > On 2023/5/30 17:57, Jan Kara wrote: > > 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 > My reproduction condition is: > mkfs.ext4 -F /dev/sda > tune2fs -Q usrquota /dev/sda > > dquot_disable > ... > if ((flags & DQUOT_USAGE_ENABLED && !(flags & > DQUOT_LIMITS_ENABLED)) > || (flags & DQUOT_SUSPENDED && flags & (DQUOT_LIMITS_ENABLED | > DQUOT_USAGE_ENABLED))) > return -EINVAL; > ... > If without enable DQUOT_LIMITS_ENABLED dquot_disable() will just return > -EINVAL.
Aha, that is the bug! Does attached patch fix your problem?
Honza -- Jan Kara <jack@suse.com> SUSE Labs, CR From 2bb758bff7f9c92a25af7156cdeedc1f39201eba Mon Sep 17 00:00:00 2001 From: Jan Kara <jack@suse.cz> Date: Mon, 5 Jun 2023 13:39:28 +0200 Subject: [PATCH] quota: Properly disable quotas when add_dquot_ref() fails
When add_dquot_ref() fails (usually due to IO error or ENOMEM), we want to disable quotas we are trying to enable. However dquot_disable() call was passed just the flags we are enabling so in case flags == DQUOT_USAGE_ENABLED dquot_disable() call will just fail with EINVAL instead of properly disabling quotas. Fix the problem by always passing DQUOT_LIMITS_ENABLED | DQUOT_USAGE_ENABLED to dquot_disable() in this case.
Reported-by: Ye Bin <yebin10@huawei.com> Reported-by: syzbot+e633c79ceaecbf479854@syzkaller.appspotmail.com Signed-off-by: Jan Kara <jack@suse.cz> --- fs/quota/dquot.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c index 226eb3cba1fb..e42cf387fa78 100644 --- a/fs/quota/dquot.c +++ b/fs/quota/dquot.c @@ -2409,7 +2409,8 @@ int dquot_load_quota_sb(struct super_block *sb, int type, int format_id, error = add_dquot_ref(sb, type); if (error) - dquot_disable(sb, type, flags); + dquot_disable(sb, type, + DQUOT_USAGE_ENABLED | DQUOT_LIMITS_ENABLED); return error; out_fmt: -- 2.35.3
| |