* [PATCH v2 0/2] ext4: fix quotas leak in __ext4_fill_super()
@ 2023-03-27 2:27 Baokun Li
2023-03-27 2:27 ` [PATCH v2 1/2] ext4: turning quotas off if mount failed after enable quotas Baokun Li
2023-03-27 2:27 ` [PATCH v2 2/2] ext4: refactoring to use the unified helper ext4_quotas_off() Baokun Li
0 siblings, 2 replies; 6+ messages in thread
From: Baokun Li @ 2023-03-27 2:27 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, adilger.kernel, jack, ritesh.list, linux-kernel, yi.zhang,
yangerkun, yukuai3, libaokun1
V1->V2:
Add judgment for CONFIG_QUOTA to avoid warning
"label 'failed_mount9' defined but not used".
(Reported-by: kernel test robot <lkp@intel.com>)
Baokun Li (2):
ext4: turning quotas off if mount failed after enable quotas
ext4: refactoring to use the unified helper ext4_quotas_off()
fs/ext4/super.c | 30 +++++++++++-------------------
1 file changed, 11 insertions(+), 19 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] ext4: turning quotas off if mount failed after enable quotas
2023-03-27 2:27 [PATCH v2 0/2] ext4: fix quotas leak in __ext4_fill_super() Baokun Li
@ 2023-03-27 2:27 ` Baokun Li
2023-03-27 9:05 ` Jan Kara
2023-03-27 2:27 ` [PATCH v2 2/2] ext4: refactoring to use the unified helper ext4_quotas_off() Baokun Li
1 sibling, 1 reply; 6+ messages in thread
From: Baokun Li @ 2023-03-27 2:27 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, adilger.kernel, jack, ritesh.list, linux-kernel, yi.zhang,
yangerkun, yukuai3, libaokun1, stable
Yi found during a review of the patch "ext4: don't BUG on inconsistent
journal feature" that when ext4_mark_recovery_complete() returns an error
value, the error handling path does not turn off the enabled quotas,
which triggers the following kmemleak:
================================================================
unreferenced object 0xffff8cf68678e7c0 (size 64):
comm "mount", pid 746, jiffies 4294871231 (age 11.540s)
hex dump (first 32 bytes):
00 90 ef 82 f6 8c ff ff 00 00 00 00 41 01 00 00 ............A...
c7 00 00 00 bd 00 00 00 0a 00 00 00 48 00 00 00 ............H...
backtrace:
[<00000000c561ef24>] __kmem_cache_alloc_node+0x4d4/0x880
[<00000000d4e621d7>] kmalloc_trace+0x39/0x140
[<00000000837eee74>] v2_read_file_info+0x18a/0x3a0
[<0000000088f6c877>] dquot_load_quota_sb+0x2ed/0x770
[<00000000340a4782>] dquot_load_quota_inode+0xc6/0x1c0
[<0000000089a18bd5>] ext4_enable_quotas+0x17e/0x3a0 [ext4]
[<000000003a0268fa>] __ext4_fill_super+0x3448/0x3910 [ext4]
[<00000000b0f2a8a8>] ext4_fill_super+0x13d/0x340 [ext4]
[<000000004a9489c4>] get_tree_bdev+0x1dc/0x370
[<000000006e723bf1>] ext4_get_tree+0x1d/0x30 [ext4]
[<00000000c7cb663d>] vfs_get_tree+0x31/0x160
[<00000000320e1bed>] do_new_mount+0x1d5/0x480
[<00000000c074654c>] path_mount+0x22e/0xbe0
[<0000000003e97a8e>] do_mount+0x95/0xc0
[<000000002f3d3736>] __x64_sys_mount+0xc4/0x160
[<0000000027d2140c>] do_syscall_64+0x3f/0x90
================================================================
To solve this problem, we add a "failed_mount10" tag, and call
ext4_quota_off_umount() in this tag to release the enabled qoutas.
Fixes: 11215630aada ("ext4: don't BUG on inconsistent journal feature")
Cc: stable@kernel.org
Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
V1->V2:
Add judgment for CONFIG_QUOTA to avoid warning
"label 'failed_mount9' defined but not used".
(Reported-by: kernel test robot <lkp@intel.com>)
fs/ext4/super.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index e6d84c1e34a4..bd1ca1c3022e 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5520,7 +5520,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
ext4_msg(sb, KERN_INFO, "recovery complete");
err = ext4_mark_recovery_complete(sb, es);
if (err)
- goto failed_mount9;
+ goto failed_mount10;
}
if (test_opt(sb, DISCARD) && !bdev_max_discard_sectors(sb->s_bdev))
@@ -5539,7 +5539,11 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
return 0;
+failed_mount10:
+#ifdef CONFIG_QUOTA
+ ext4_quota_off_umount(sb);
failed_mount9:
+#endif /* CONFIG_QUOTA */
ext4_release_orphan_info(sb);
failed_mount8:
ext4_unregister_sysfs(sb);
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] ext4: refactoring to use the unified helper ext4_quotas_off()
2023-03-27 2:27 [PATCH v2 0/2] ext4: fix quotas leak in __ext4_fill_super() Baokun Li
2023-03-27 2:27 ` [PATCH v2 1/2] ext4: turning quotas off if mount failed after enable quotas Baokun Li
@ 2023-03-27 2:27 ` Baokun Li
2023-03-27 9:21 ` Jan Kara
1 sibling, 1 reply; 6+ messages in thread
From: Baokun Li @ 2023-03-27 2:27 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, adilger.kernel, jack, ritesh.list, linux-kernel, yi.zhang,
yangerkun, yukuai3, libaokun1
Rename ext4_quota_off_umount() to ext4_quotas_off(), and add type
parameter to replace open code in ext4_enable_quotas().
Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
V1->V2:
Adapting to the changes in PATCH 1/2.
fs/ext4/super.c | 26 +++++++-------------------
1 file changed, 7 insertions(+), 19 deletions(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index bd1ca1c3022e..59c5dd4dbe5a 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1157,12 +1157,12 @@ static void dump_orphan_list(struct super_block *sb, struct ext4_sb_info *sbi)
#ifdef CONFIG_QUOTA
static int ext4_quota_off(struct super_block *sb, int type);
-static inline void ext4_quota_off_umount(struct super_block *sb)
+static inline void ext4_quotas_off(struct super_block *sb, int type)
{
- int type;
+ BUG_ON(type > EXT4_MAXQUOTAS);
/* Use our quota_off function to clear inode flags etc. */
- for (type = 0; type < EXT4_MAXQUOTAS; type++)
+ for (type--; type >= 0; type--)
ext4_quota_off(sb, type);
}
@@ -1178,7 +1178,7 @@ static inline char *get_qf_name(struct super_block *sb,
lockdep_is_held(&sb->s_umount));
}
#else
-static inline void ext4_quota_off_umount(struct super_block *sb)
+static inline void ext4_quotas_off(struct super_block *sb, int type)
{
}
#endif
@@ -1209,7 +1209,7 @@ static void ext4_put_super(struct super_block *sb)
&sb->s_uuid);
ext4_unregister_li_request(sb);
- ext4_quota_off_umount(sb);
+ ext4_quotas_off(sb, EXT4_MAXQUOTAS);
flush_work(&sbi->s_error_work);
destroy_workqueue(sbi->rsv_conversion_wq);
@@ -5541,7 +5541,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
failed_mount10:
#ifdef CONFIG_QUOTA
- ext4_quota_off_umount(sb);
+ ext4_quotas_off(sb, EXT4_MAXQUOTAS);
failed_mount9:
#endif /* CONFIG_QUOTA */
ext4_release_orphan_info(sb);
@@ -7014,20 +7014,8 @@ int ext4_enable_quotas(struct super_block *sb)
"(type=%d, err=%d, ino=%lu). "
"Please run e2fsck to fix.", type,
err, qf_inums[type]);
- for (type--; type >= 0; type--) {
- struct inode *inode;
-
- inode = sb_dqopt(sb)->files[type];
- if (inode)
- inode = igrab(inode);
- dquot_quota_off(sb, type);
- if (inode) {
- lockdep_set_quota_inode(inode,
- I_DATA_SEM_NORMAL);
- iput(inode);
- }
- }
+ ext4_quotas_off(sb, type);
return err;
}
}
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] ext4: turning quotas off if mount failed after enable quotas
2023-03-27 2:27 ` [PATCH v2 1/2] ext4: turning quotas off if mount failed after enable quotas Baokun Li
@ 2023-03-27 9:05 ` Jan Kara
2023-03-27 12:06 ` Baokun Li
0 siblings, 1 reply; 6+ messages in thread
From: Jan Kara @ 2023-03-27 9:05 UTC (permalink / raw)
To: Baokun Li
Cc: linux-ext4, tytso, adilger.kernel, jack, ritesh.list,
linux-kernel, yi.zhang, yangerkun, yukuai3, stable
On Mon 27-03-23 10:27:02, Baokun Li wrote:
> Yi found during a review of the patch "ext4: don't BUG on inconsistent
> journal feature" that when ext4_mark_recovery_complete() returns an error
> value, the error handling path does not turn off the enabled quotas,
> which triggers the following kmemleak:
>
> ================================================================
> unreferenced object 0xffff8cf68678e7c0 (size 64):
> comm "mount", pid 746, jiffies 4294871231 (age 11.540s)
> hex dump (first 32 bytes):
> 00 90 ef 82 f6 8c ff ff 00 00 00 00 41 01 00 00 ............A...
> c7 00 00 00 bd 00 00 00 0a 00 00 00 48 00 00 00 ............H...
> backtrace:
> [<00000000c561ef24>] __kmem_cache_alloc_node+0x4d4/0x880
> [<00000000d4e621d7>] kmalloc_trace+0x39/0x140
> [<00000000837eee74>] v2_read_file_info+0x18a/0x3a0
> [<0000000088f6c877>] dquot_load_quota_sb+0x2ed/0x770
> [<00000000340a4782>] dquot_load_quota_inode+0xc6/0x1c0
> [<0000000089a18bd5>] ext4_enable_quotas+0x17e/0x3a0 [ext4]
> [<000000003a0268fa>] __ext4_fill_super+0x3448/0x3910 [ext4]
> [<00000000b0f2a8a8>] ext4_fill_super+0x13d/0x340 [ext4]
> [<000000004a9489c4>] get_tree_bdev+0x1dc/0x370
> [<000000006e723bf1>] ext4_get_tree+0x1d/0x30 [ext4]
> [<00000000c7cb663d>] vfs_get_tree+0x31/0x160
> [<00000000320e1bed>] do_new_mount+0x1d5/0x480
> [<00000000c074654c>] path_mount+0x22e/0xbe0
> [<0000000003e97a8e>] do_mount+0x95/0xc0
> [<000000002f3d3736>] __x64_sys_mount+0xc4/0x160
> [<0000000027d2140c>] do_syscall_64+0x3f/0x90
> ================================================================
>
> To solve this problem, we add a "failed_mount10" tag, and call
> ext4_quota_off_umount() in this tag to release the enabled qoutas.
>
> Fixes: 11215630aada ("ext4: don't BUG on inconsistent journal feature")
> Cc: stable@kernel.org
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> Signed-off-by: Baokun Li <libaokun1@huawei.com>
Looks good. Just one comment:
> +failed_mount10:
> +#ifdef CONFIG_QUOTA
> + ext4_quota_off_umount(sb);
> failed_mount9:
> +#endif /* CONFIG_QUOTA */
How about dealing with this using __maybe_unused attribute instead. Like:
failed_mount9: __maybe_unused
That would be much easier to read...
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] ext4: refactoring to use the unified helper ext4_quotas_off()
2023-03-27 2:27 ` [PATCH v2 2/2] ext4: refactoring to use the unified helper ext4_quotas_off() Baokun Li
@ 2023-03-27 9:21 ` Jan Kara
0 siblings, 0 replies; 6+ messages in thread
From: Jan Kara @ 2023-03-27 9:21 UTC (permalink / raw)
To: Baokun Li
Cc: linux-ext4, tytso, adilger.kernel, jack, ritesh.list,
linux-kernel, yi.zhang, yangerkun, yukuai3
On Mon 27-03-23 10:27:03, Baokun Li wrote:
> Rename ext4_quota_off_umount() to ext4_quotas_off(), and add type
> parameter to replace open code in ext4_enable_quotas().
>
> Signed-off-by: Baokun Li <libaokun1@huawei.com>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> V1->V2:
> Adapting to the changes in PATCH 1/2.
>
> fs/ext4/super.c | 26 +++++++-------------------
> 1 file changed, 7 insertions(+), 19 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index bd1ca1c3022e..59c5dd4dbe5a 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1157,12 +1157,12 @@ static void dump_orphan_list(struct super_block *sb, struct ext4_sb_info *sbi)
> #ifdef CONFIG_QUOTA
> static int ext4_quota_off(struct super_block *sb, int type);
>
> -static inline void ext4_quota_off_umount(struct super_block *sb)
> +static inline void ext4_quotas_off(struct super_block *sb, int type)
> {
> - int type;
> + BUG_ON(type > EXT4_MAXQUOTAS);
>
> /* Use our quota_off function to clear inode flags etc. */
> - for (type = 0; type < EXT4_MAXQUOTAS; type++)
> + for (type--; type >= 0; type--)
> ext4_quota_off(sb, type);
> }
>
> @@ -1178,7 +1178,7 @@ static inline char *get_qf_name(struct super_block *sb,
> lockdep_is_held(&sb->s_umount));
> }
> #else
> -static inline void ext4_quota_off_umount(struct super_block *sb)
> +static inline void ext4_quotas_off(struct super_block *sb, int type)
> {
> }
> #endif
> @@ -1209,7 +1209,7 @@ static void ext4_put_super(struct super_block *sb)
> &sb->s_uuid);
>
> ext4_unregister_li_request(sb);
> - ext4_quota_off_umount(sb);
> + ext4_quotas_off(sb, EXT4_MAXQUOTAS);
>
> flush_work(&sbi->s_error_work);
> destroy_workqueue(sbi->rsv_conversion_wq);
> @@ -5541,7 +5541,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
>
> failed_mount10:
> #ifdef CONFIG_QUOTA
> - ext4_quota_off_umount(sb);
> + ext4_quotas_off(sb, EXT4_MAXQUOTAS);
> failed_mount9:
> #endif /* CONFIG_QUOTA */
> ext4_release_orphan_info(sb);
> @@ -7014,20 +7014,8 @@ int ext4_enable_quotas(struct super_block *sb)
> "(type=%d, err=%d, ino=%lu). "
> "Please run e2fsck to fix.", type,
> err, qf_inums[type]);
> - for (type--; type >= 0; type--) {
> - struct inode *inode;
> -
> - inode = sb_dqopt(sb)->files[type];
> - if (inode)
> - inode = igrab(inode);
> - dquot_quota_off(sb, type);
> - if (inode) {
> - lockdep_set_quota_inode(inode,
> - I_DATA_SEM_NORMAL);
> - iput(inode);
> - }
> - }
>
> + ext4_quotas_off(sb, type);
> return err;
> }
> }
> --
> 2.31.1
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] ext4: turning quotas off if mount failed after enable quotas
2023-03-27 9:05 ` Jan Kara
@ 2023-03-27 12:06 ` Baokun Li
0 siblings, 0 replies; 6+ messages in thread
From: Baokun Li @ 2023-03-27 12:06 UTC (permalink / raw)
To: Jan Kara
Cc: linux-ext4, tytso, adilger.kernel, ritesh.list, linux-kernel,
yi.zhang, yangerkun, yukuai3, stable, Baokun Li
On 2023/3/27 17:05, Jan Kara wrote:
> On Mon 27-03-23 10:27:02, Baokun Li wrote:
>> Yi found during a review of the patch "ext4: don't BUG on inconsistent
>> journal feature" that when ext4_mark_recovery_complete() returns an error
>> value, the error handling path does not turn off the enabled quotas,
>> which triggers the following kmemleak:
>>
>> ================================================================
>> unreferenced object 0xffff8cf68678e7c0 (size 64):
>> comm "mount", pid 746, jiffies 4294871231 (age 11.540s)
>> hex dump (first 32 bytes):
>> 00 90 ef 82 f6 8c ff ff 00 00 00 00 41 01 00 00 ............A...
>> c7 00 00 00 bd 00 00 00 0a 00 00 00 48 00 00 00 ............H...
>> backtrace:
>> [<00000000c561ef24>] __kmem_cache_alloc_node+0x4d4/0x880
>> [<00000000d4e621d7>] kmalloc_trace+0x39/0x140
>> [<00000000837eee74>] v2_read_file_info+0x18a/0x3a0
>> [<0000000088f6c877>] dquot_load_quota_sb+0x2ed/0x770
>> [<00000000340a4782>] dquot_load_quota_inode+0xc6/0x1c0
>> [<0000000089a18bd5>] ext4_enable_quotas+0x17e/0x3a0 [ext4]
>> [<000000003a0268fa>] __ext4_fill_super+0x3448/0x3910 [ext4]
>> [<00000000b0f2a8a8>] ext4_fill_super+0x13d/0x340 [ext4]
>> [<000000004a9489c4>] get_tree_bdev+0x1dc/0x370
>> [<000000006e723bf1>] ext4_get_tree+0x1d/0x30 [ext4]
>> [<00000000c7cb663d>] vfs_get_tree+0x31/0x160
>> [<00000000320e1bed>] do_new_mount+0x1d5/0x480
>> [<00000000c074654c>] path_mount+0x22e/0xbe0
>> [<0000000003e97a8e>] do_mount+0x95/0xc0
>> [<000000002f3d3736>] __x64_sys_mount+0xc4/0x160
>> [<0000000027d2140c>] do_syscall_64+0x3f/0x90
>> ================================================================
>>
>> To solve this problem, we add a "failed_mount10" tag, and call
>> ext4_quota_off_umount() in this tag to release the enabled qoutas.
>>
>> Fixes: 11215630aada ("ext4: don't BUG on inconsistent journal feature")
>> Cc: stable@kernel.org
>> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
>> Signed-off-by: Baokun Li <libaokun1@huawei.com>
> Looks good. Just one comment:
>
>> +failed_mount10:
>> +#ifdef CONFIG_QUOTA
>> + ext4_quota_off_umount(sb);
>> failed_mount9:
>> +#endif /* CONFIG_QUOTA */
> How about dealing with this using __maybe_unused attribute instead. Like:
>
> failed_mount9: __maybe_unused
>
> That would be much easier to read...
>
> Honza
Indeed!
Thank you very much for the review!
I will send a patch V3 with the changes suggested by you.
--
With Best Regards,
Baokun Li
.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-03-27 12:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-27 2:27 [PATCH v2 0/2] ext4: fix quotas leak in __ext4_fill_super() Baokun Li
2023-03-27 2:27 ` [PATCH v2 1/2] ext4: turning quotas off if mount failed after enable quotas Baokun Li
2023-03-27 9:05 ` Jan Kara
2023-03-27 12:06 ` Baokun Li
2023-03-27 2:27 ` [PATCH v2 2/2] ext4: refactoring to use the unified helper ext4_quotas_off() Baokun Li
2023-03-27 9:21 ` Jan Kara
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.