* [PATCH 0/2] fix two issue about quota @ 2023-05-27 1:40 Ye Bin 2023-05-27 1:40 ` [PATCH 1/2] quota: fix null-ptr-deref in ext4_acquire_dquot() Ye Bin 2023-05-27 1:40 ` [PATCH 2/2] quota: fix warning in dqgrab() Ye Bin 0 siblings, 2 replies; 9+ messages in thread From: Ye Bin @ 2023-05-27 1:40 UTC (permalink / raw) To: jack, linux-kernel; +Cc: yebin10 Ye Bin (2): quota: fix null-ptr-deref in ext4_acquire_dquot() quota: fix warning in dqgrab() fs/quota/dquot.c | 2 +- include/linux/quotaops.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) -- 2.31.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] quota: fix null-ptr-deref in ext4_acquire_dquot() 2023-05-27 1:40 [PATCH 0/2] fix two issue about quota Ye Bin @ 2023-05-27 1:40 ` Ye Bin 2023-05-30 9:57 ` Jan Kara 2023-05-27 1:40 ` [PATCH 2/2] quota: fix warning in dqgrab() Ye Bin 1 sibling, 1 reply; 9+ messages in thread From: Ye Bin @ 2023-05-27 1:40 UTC (permalink / raw) To: jack, linux-kernel; +Cc: yebin10, syzbot+e633c79ceaecbf479854 Syzbot found the following issue: Unable to handle kernel paging request at virtual address dfff800000000005 KASAN: null-ptr-deref in range [0x0000000000000028-0x000000000000002f] Mem abort info: ESR = 0x0000000096000006 EC = 0x25: DABT (current EL), IL = 32 bits SET = 0, FnV = 0 EA = 0, S1PTW = 0 FSC = 0x06: level 2 translation fault Data abort info: ISV = 0, ISS = 0x00000006 CM = 0, WnR = 0 [dfff800000000005] address between user and kernel address ranges Internal error: Oops: 0000000096000006 [#1] PREEMPT SMP Modules linked in: 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 sp : ffff80001eb27280 x29: ffff80001eb27280 x28: 1fffe0001c3c01fc x27: ffff800015d705b0 x26: ffff0000dd93c000 x25: ffff0000dd93e000 x24: 1fffe0001c3c021c x23: dfff800000000000 x22: 0000000000000049 x21: 0000000000000028 x20: 0000000000000000 x19: ffff0000e1e00fc0 x18: ffff0001b426cca8 x17: 0000000000000000 x16: ffff8000089669b0 x15: 0000000000000001 x14: 1ffff00002bae0b0 x13: dfff800000000000 x12: 0000000000000001 x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000000000 x8 : 0000000000000005 x7 : ffff800008c11f68 x6 : 0000000000000000 x5 : 0000000000000000 x4 : 0000000000000001 x3 : ffff800012441b4c x2 : 0000000000000001 x1 : 0000000000000001 x0 : 0000000000000003 Call trace: ext4_acquire_dquot+0x1d4/0x398 fs/ext4/super.c:6766 dqget+0x844/0xc48 fs/quota/dquot.c:914 __dquot_initialize+0x2cc/0xb54 fs/quota/dquot.c:1492 dquot_initialize fs/quota/dquot.c:1550 [inline] dquot_file_open+0x90/0xc8 fs/quota/dquot.c:2181 ext4_file_open+0x230/0x590 fs/ext4/file.c:903 do_dentry_open+0x724/0xf90 fs/open.c:920 vfs_open+0x7c/0x90 fs/open.c:1051 do_open fs/namei.c:3560 [inline] path_openat+0x1f2c/0x27f8 fs/namei.c:3715 do_filp_open+0x1bc/0x3cc fs/namei.c:3742 do_sys_openat2+0x128/0x3d8 fs/open.c:1348 do_sys_open fs/open.c:1364 [inline] __do_sys_openat fs/open.c:1380 [inline] __se_sys_openat fs/open.c:1375 [inline] __arm64_sys_openat+0x1f0/0x240 fs/open.c:1375 __invoke_syscall arch/arm64/kernel/syscall.c:38 [inline] invoke_syscall+0x98/0x2c0 arch/arm64/kernel/syscall.c:52 el0_svc_common+0x138/0x258 arch/arm64/kernel/syscall.c:142 do_el0_svc+0x64/0x198 arch/arm64/kernel/syscall.c:193 el0_svc+0x4c/0x15c arch/arm64/kernel/entry-common.c:637 el0t_64_sync_handler+0x84/0xf0 arch/arm64/kernel/entry-common.c:655 el0t_64_sync+0x190/0x194 arch/arm64/kernel/entry.S:591 Code: 97e8a7df f94002a8 9100a115 d343fea8 (38776908) ---[ end trace 0000000000000000 ]--- 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; 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] quota: fix null-ptr-deref in ext4_acquire_dquot() 2023-05-27 1:40 ` [PATCH 1/2] quota: fix null-ptr-deref in ext4_acquire_dquot() Ye Bin @ 2023-05-30 9:57 ` Jan Kara 2023-06-03 6:14 ` yebin (H) 0 siblings, 1 reply; 9+ messages in thread From: Jan Kara @ 2023-05-30 9:57 UTC (permalink / raw) To: Ye Bin; +Cc: jack, linux-kernel, syzbot+e633c79ceaecbf479854 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] quota: fix null-ptr-deref in ext4_acquire_dquot() 2023-05-30 9:57 ` Jan Kara @ 2023-06-03 6:14 ` yebin (H) 2023-06-05 11:51 ` Jan Kara 0 siblings, 1 reply; 9+ messages in thread From: yebin (H) @ 2023-06-03 6:14 UTC (permalink / raw) To: Jan Kara; +Cc: jack, linux-kernel, syzbot+e633c79ceaecbf479854 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. >> 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 >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] quota: fix null-ptr-deref in ext4_acquire_dquot() 2023-06-03 6:14 ` yebin (H) @ 2023-06-05 11:51 ` Jan Kara 2023-06-05 14:08 ` yebin (H) 0 siblings, 1 reply; 9+ messages in thread From: Jan Kara @ 2023-06-05 11:51 UTC (permalink / raw) To: yebin (H); +Cc: Jan Kara, jack, linux-kernel, syzbot+e633c79ceaecbf479854 [-- Attachment #1: Type: text/plain, Size: 3407 bytes --] 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 [-- Attachment #2: 0001-quota-Properly-disable-quotas-when-add_dquot_ref-fai.patch --] [-- Type: text/x-patch, Size: 1300 bytes --] 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] quota: fix null-ptr-deref in ext4_acquire_dquot() 2023-06-05 11:51 ` Jan Kara @ 2023-06-05 14:08 ` yebin (H) 0 siblings, 0 replies; 9+ messages in thread From: yebin (H) @ 2023-06-05 14:08 UTC (permalink / raw) To: Jan Kara; +Cc: jack, linux-kernel, syzbot+e633c79ceaecbf479854 On 2023/6/5 19:51, Jan Kara wrote: > 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 Yes , it's works. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] quota: fix warning in dqgrab() 2023-05-27 1:40 [PATCH 0/2] fix two issue about quota Ye Bin 2023-05-27 1:40 ` [PATCH 1/2] quota: fix null-ptr-deref in ext4_acquire_dquot() Ye Bin @ 2023-05-27 1:40 ` Ye Bin 2023-05-30 10:15 ` Jan Kara 1 sibling, 1 reply; 9+ messages in thread From: Ye Bin @ 2023-05-27 1:40 UTC (permalink / raw) To: jack, linux-kernel; +Cc: yebin10 There's issue as follows when do fault injection: WARNING: CPU: 1 PID: 14870 at include/linux/quotaops.h:51 dquot_disable+0x13b7/0x18c0 Modules linked in: CPU: 1 PID: 14870 Comm: fsconfig Not tainted 6.3.0-next-20230505-00006-g5107a9c821af-dirty #541 RIP: 0010:dquot_disable+0x13b7/0x18c0 RSP: 0018:ffffc9000acc79e0 EFLAGS: 00010246 RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffff88825e41b980 RDX: 0000000000000000 RSI: ffff88825e41b980 RDI: 0000000000000002 RBP: ffff888179f68000 R08: ffffffff82087ca7 R09: 0000000000000000 R10: 0000000000000001 R11: ffffed102f3ed026 R12: ffff888179f68130 R13: ffff888179f68110 R14: dffffc0000000000 R15: ffff888179f68118 FS: 00007f450a073740(0000) GS:ffff88882fc00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007ffe96f2efd8 CR3: 000000025c8ad000 CR4: 00000000000006e0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: <TASK> dquot_load_quota_sb+0xd53/0x1060 dquot_resume+0x172/0x230 ext4_reconfigure+0x1dc6/0x27b0 reconfigure_super+0x515/0xa90 __x64_sys_fsconfig+0xb19/0xd20 do_syscall_64+0x39/0xb0 entry_SYSCALL_64_after_hwframe+0x63/0xcd 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 atomic_inc(&dquot->dq_count); __dquot_initialize __dquot_initialize dqget if (!test_bit(DQ_ACTIVE_B, &dquot->dq_flags)) ext4_acquire_dquot -> Return error DQ_ACTIVE_B flag isn't set dquot_disable invalidate_dquots if (atomic_read(&dquot->dq_count)) dqgrab WARN_ON_ONCE(!test_bit(DQ_ACTIVE_B, &dquot->dq_flags)) -> Trigger warning In the above scenario, 'dquot->dq_flags' has no DQ_ACTIVE_B is normal when dqgrab(). So just remove 'WARN_ON_ONCE(!test_bit(DQ_ACTIVE_B, &dquot->dq_flags))' in dqgrab(). Signed-off-by: Ye Bin <yebin10@huawei.com> --- include/linux/quotaops.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/linux/quotaops.h b/include/linux/quotaops.h index 11a4becff3a9..cb5e4c11e503 100644 --- a/include/linux/quotaops.h +++ b/include/linux/quotaops.h @@ -48,7 +48,6 @@ static inline struct dquot *dqgrab(struct dquot *dquot) { /* Make sure someone else has active reference to dquot */ WARN_ON_ONCE(!atomic_read(&dquot->dq_count)); - WARN_ON_ONCE(!test_bit(DQ_ACTIVE_B, &dquot->dq_flags)); atomic_inc(&dquot->dq_count); return dquot; } -- 2.31.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] quota: fix warning in dqgrab() 2023-05-27 1:40 ` [PATCH 2/2] quota: fix warning in dqgrab() Ye Bin @ 2023-05-30 10:15 ` Jan Kara 2023-06-03 2:57 ` yebin (H) 0 siblings, 1 reply; 9+ messages in thread From: Jan Kara @ 2023-05-30 10:15 UTC (permalink / raw) To: Ye Bin; +Cc: jack, linux-kernel On Sat 27-05-23 09:40:18, Ye Bin wrote: > There's issue as follows when do fault injection: > WARNING: CPU: 1 PID: 14870 at include/linux/quotaops.h:51 dquot_disable+0x13b7/0x18c0 > Modules linked in: > CPU: 1 PID: 14870 Comm: fsconfig Not tainted 6.3.0-next-20230505-00006-g5107a9c821af-dirty #541 > RIP: 0010:dquot_disable+0x13b7/0x18c0 > RSP: 0018:ffffc9000acc79e0 EFLAGS: 00010246 > RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffff88825e41b980 > RDX: 0000000000000000 RSI: ffff88825e41b980 RDI: 0000000000000002 > RBP: ffff888179f68000 R08: ffffffff82087ca7 R09: 0000000000000000 > R10: 0000000000000001 R11: ffffed102f3ed026 R12: ffff888179f68130 > R13: ffff888179f68110 R14: dffffc0000000000 R15: ffff888179f68118 > FS: 00007f450a073740(0000) GS:ffff88882fc00000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 00007ffe96f2efd8 CR3: 000000025c8ad000 CR4: 00000000000006e0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > Call Trace: > <TASK> > dquot_load_quota_sb+0xd53/0x1060 > dquot_resume+0x172/0x230 > ext4_reconfigure+0x1dc6/0x27b0 > reconfigure_super+0x515/0xa90 > __x64_sys_fsconfig+0xb19/0xd20 > do_syscall_64+0x39/0xb0 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > > 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 > atomic_inc(&dquot->dq_count); > > __dquot_initialize > __dquot_initialize > dqget > if (!test_bit(DQ_ACTIVE_B, &dquot->dq_flags)) > ext4_acquire_dquot > -> Return error DQ_ACTIVE_B flag isn't set > dquot_disable > invalidate_dquots > if (atomic_read(&dquot->dq_count)) > dqgrab > WARN_ON_ONCE(!test_bit(DQ_ACTIVE_B, &dquot->dq_flags)) > -> Trigger warning > > In the above scenario, 'dquot->dq_flags' has no DQ_ACTIVE_B is normal when > dqgrab(). > So just remove 'WARN_ON_ONCE(!test_bit(DQ_ACTIVE_B, &dquot->dq_flags))' > in dqgrab(). > > Signed-off-by: Ye Bin <yebin10@huawei.com> Thanks for the patch! Actually rather than deleting the assertion from dqgrab() (which can be and is used by filesystems) I'd replace the dqgrab() use in invalidate_dquots() with plain: atomic_inc(&dquot->dq_count); Honza > --- > include/linux/quotaops.h | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/include/linux/quotaops.h b/include/linux/quotaops.h > index 11a4becff3a9..cb5e4c11e503 100644 > --- a/include/linux/quotaops.h > +++ b/include/linux/quotaops.h > @@ -48,7 +48,6 @@ static inline struct dquot *dqgrab(struct dquot *dquot) > { > /* Make sure someone else has active reference to dquot */ > WARN_ON_ONCE(!atomic_read(&dquot->dq_count)); > - WARN_ON_ONCE(!test_bit(DQ_ACTIVE_B, &dquot->dq_flags)); > atomic_inc(&dquot->dq_count); > return dquot; > } > -- > 2.31.1 > -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] quota: fix warning in dqgrab() 2023-05-30 10:15 ` Jan Kara @ 2023-06-03 2:57 ` yebin (H) 0 siblings, 0 replies; 9+ messages in thread From: yebin (H) @ 2023-06-03 2:57 UTC (permalink / raw) To: Jan Kara; +Cc: jack, linux-kernel On 2023/5/30 18:15, Jan Kara wrote: > On Sat 27-05-23 09:40:18, Ye Bin wrote: >> There's issue as follows when do fault injection: >> WARNING: CPU: 1 PID: 14870 at include/linux/quotaops.h:51 dquot_disable+0x13b7/0x18c0 >> Modules linked in: >> CPU: 1 PID: 14870 Comm: fsconfig Not tainted 6.3.0-next-20230505-00006-g5107a9c821af-dirty #541 >> RIP: 0010:dquot_disable+0x13b7/0x18c0 >> RSP: 0018:ffffc9000acc79e0 EFLAGS: 00010246 >> RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffff88825e41b980 >> RDX: 0000000000000000 RSI: ffff88825e41b980 RDI: 0000000000000002 >> RBP: ffff888179f68000 R08: ffffffff82087ca7 R09: 0000000000000000 >> R10: 0000000000000001 R11: ffffed102f3ed026 R12: ffff888179f68130 >> R13: ffff888179f68110 R14: dffffc0000000000 R15: ffff888179f68118 >> FS: 00007f450a073740(0000) GS:ffff88882fc00000(0000) knlGS:0000000000000000 >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >> CR2: 00007ffe96f2efd8 CR3: 000000025c8ad000 CR4: 00000000000006e0 >> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 >> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 >> Call Trace: >> <TASK> >> dquot_load_quota_sb+0xd53/0x1060 >> dquot_resume+0x172/0x230 >> ext4_reconfigure+0x1dc6/0x27b0 >> reconfigure_super+0x515/0xa90 >> __x64_sys_fsconfig+0xb19/0xd20 >> do_syscall_64+0x39/0xb0 >> entry_SYSCALL_64_after_hwframe+0x63/0xcd >> >> 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 >> atomic_inc(&dquot->dq_count); >> >> __dquot_initialize >> __dquot_initialize >> dqget >> if (!test_bit(DQ_ACTIVE_B, &dquot->dq_flags)) >> ext4_acquire_dquot >> -> Return error DQ_ACTIVE_B flag isn't set >> dquot_disable >> invalidate_dquots >> if (atomic_read(&dquot->dq_count)) >> dqgrab >> WARN_ON_ONCE(!test_bit(DQ_ACTIVE_B, &dquot->dq_flags)) >> -> Trigger warning >> >> In the above scenario, 'dquot->dq_flags' has no DQ_ACTIVE_B is normal when >> dqgrab(). >> So just remove 'WARN_ON_ONCE(!test_bit(DQ_ACTIVE_B, &dquot->dq_flags))' >> in dqgrab(). >> >> Signed-off-by: Ye Bin <yebin10@huawei.com> > Thanks for the patch! Actually rather than deleting the assertion from > dqgrab() (which can be and is used by filesystems) I'd replace the dqgrab() > use in invalidate_dquots() with plain: > > atomic_inc(&dquot->dq_count); > > Honza This is indeed a good idea, I will send a new version. >> --- >> include/linux/quotaops.h | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/include/linux/quotaops.h b/include/linux/quotaops.h >> index 11a4becff3a9..cb5e4c11e503 100644 >> --- a/include/linux/quotaops.h >> +++ b/include/linux/quotaops.h >> @@ -48,7 +48,6 @@ static inline struct dquot *dqgrab(struct dquot *dquot) >> { >> /* Make sure someone else has active reference to dquot */ >> WARN_ON_ONCE(!atomic_read(&dquot->dq_count)); >> - WARN_ON_ONCE(!test_bit(DQ_ACTIVE_B, &dquot->dq_flags)); >> atomic_inc(&dquot->dq_count); >> return dquot; >> } >> -- >> 2.31.1 >> ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-06-05 14:10 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-05-27 1:40 [PATCH 0/2] fix two issue about quota Ye Bin 2023-05-27 1:40 ` [PATCH 1/2] quota: fix null-ptr-deref in ext4_acquire_dquot() Ye Bin 2023-05-30 9:57 ` Jan Kara 2023-06-03 6:14 ` yebin (H) 2023-06-05 11:51 ` Jan Kara 2023-06-05 14:08 ` yebin (H) 2023-05-27 1:40 ` [PATCH 2/2] quota: fix warning in dqgrab() Ye Bin 2023-05-30 10:15 ` Jan Kara 2023-06-03 2:57 ` yebin (H)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox