* [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
* 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 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
* [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 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
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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox