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
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 <[email protected]>
---
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
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 <[email protected]>
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 <[email protected]>
SUSE Labs, CR
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 <[email protected]>
> 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
>>