* [PATCH] ext4: fix check to prevent false positive report of incorrect used inodes
@ 2021-03-29 6:19 Zhang Yi
2021-03-29 14:26 ` Jan Kara
0 siblings, 1 reply; 3+ messages in thread
From: Zhang Yi @ 2021-03-29 6:19 UTC (permalink / raw)
To: linux-ext4; +Cc: tytso, adilger.kernel, jack, yi.zhang
Commit <50122847007> ("ext4: fix check to prevent initializing reserved
inodes") check the block group zero and prevent initializing reserved
inodes. But in some special cases, the reserved inode may not all belong
to the group zero, it may exist into the second group if we format
filesystem below.
mkfs.ext4 -b 4096 -g 8192 -N 1024 -I 4096 /dev/sda
So, it will end up triggering a false positive report of a corrupted
file system. This patch fix it by avoid check reserved inodes if no free
inode blocks will be zeroed.
Fixes: 50122847007 ("ext4: fix check to prevent initializing reserved inodes")
Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
---
fs/ext4/ialloc.c | 36 ++++++++++++++++++++----------------
1 file changed, 20 insertions(+), 16 deletions(-)
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 633ae7becd61..2eab813b690b 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -1513,6 +1513,7 @@ int ext4_init_inode_table(struct super_block *sb, ext4_group_t group,
handle_t *handle;
ext4_fsblk_t blk;
int num, ret = 0, used_blks = 0;
+ unsigned long used_inos = 0;
/* This should not happen, but just to be sure check this */
if (sb_rdonly(sb)) {
@@ -1543,22 +1544,25 @@ int ext4_init_inode_table(struct super_block *sb, ext4_group_t group,
* used inodes so we need to skip blocks with used inodes in
* inode table.
*/
- if (!(gdp->bg_flags & cpu_to_le16(EXT4_BG_INODE_UNINIT)))
- used_blks = DIV_ROUND_UP((EXT4_INODES_PER_GROUP(sb) -
- ext4_itable_unused_count(sb, gdp)),
- sbi->s_inodes_per_block);
-
- if ((used_blks < 0) || (used_blks > sbi->s_itb_per_group) ||
- ((group == 0) && ((EXT4_INODES_PER_GROUP(sb) -
- ext4_itable_unused_count(sb, gdp)) <
- EXT4_FIRST_INO(sb)))) {
- ext4_error(sb, "Something is wrong with group %u: "
- "used itable blocks: %d; "
- "itable unused count: %u",
- group, used_blks,
- ext4_itable_unused_count(sb, gdp));
- ret = 1;
- goto err_out;
+ if (!(gdp->bg_flags & cpu_to_le16(EXT4_BG_INODE_UNINIT))) {
+ used_inos = EXT4_INODES_PER_GROUP(sb) -
+ ext4_itable_unused_count(sb, gdp);
+ used_blks = DIV_ROUND_UP(used_inos, sbi->s_inodes_per_block);
+
+ if (used_blks >= 0 && used_blks <= sbi->s_itb_per_group)
+ used_inos += group * EXT4_INODES_PER_GROUP(sb);
+
+ if ((used_blks < 0) || (used_blks > sbi->s_itb_per_group) ||
+ ((used_blks != sbi->s_itb_per_group) &&
+ (used_inos < EXT4_FIRST_INO(sb)))) {
+ ext4_error(sb, "Something is wrong with group %u: "
+ "used itable blocks: %d; "
+ "itable unused count: %u",
+ group, used_blks,
+ ext4_itable_unused_count(sb, gdp));
+ ret = 1;
+ goto err_out;
+ }
}
blk = ext4_inode_table(sb, gdp) + used_blks;
--
2.25.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] ext4: fix check to prevent false positive report of incorrect used inodes
2021-03-29 6:19 [PATCH] ext4: fix check to prevent false positive report of incorrect used inodes Zhang Yi
@ 2021-03-29 14:26 ` Jan Kara
2021-03-31 3:12 ` Zhang Yi
0 siblings, 1 reply; 3+ messages in thread
From: Jan Kara @ 2021-03-29 14:26 UTC (permalink / raw)
To: Zhang Yi; +Cc: linux-ext4, tytso, adilger.kernel, jack
On Mon 29-03-21 14:19:55, Zhang Yi wrote:
> Commit <50122847007> ("ext4: fix check to prevent initializing reserved
> inodes") check the block group zero and prevent initializing reserved
> inodes. But in some special cases, the reserved inode may not all belong
> to the group zero, it may exist into the second group if we format
> filesystem below.
>
> mkfs.ext4 -b 4096 -g 8192 -N 1024 -I 4096 /dev/sda
>
> So, it will end up triggering a false positive report of a corrupted
> file system. This patch fix it by avoid check reserved inodes if no free
> inode blocks will be zeroed.
>
> Fixes: 50122847007 ("ext4: fix check to prevent initializing reserved inodes")
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Thanks! The patch looks correct but maybe the code can be made more
comprehensible like I suggest below?
> @@ -1543,22 +1544,25 @@ int ext4_init_inode_table(struct super_block *sb, ext4_group_t group,
> * used inodes so we need to skip blocks with used inodes in
> * inode table.
> */
> - if (!(gdp->bg_flags & cpu_to_le16(EXT4_BG_INODE_UNINIT)))
> - used_blks = DIV_ROUND_UP((EXT4_INODES_PER_GROUP(sb) -
> - ext4_itable_unused_count(sb, gdp)),
> - sbi->s_inodes_per_block);
> -
> - if ((used_blks < 0) || (used_blks > sbi->s_itb_per_group) ||
> - ((group == 0) && ((EXT4_INODES_PER_GROUP(sb) -
> - ext4_itable_unused_count(sb, gdp)) <
> - EXT4_FIRST_INO(sb)))) {
> - ext4_error(sb, "Something is wrong with group %u: "
> - "used itable blocks: %d; "
> - "itable unused count: %u",
> - group, used_blks,
> - ext4_itable_unused_count(sb, gdp));
> - ret = 1;
> - goto err_out;
> + if (!(gdp->bg_flags & cpu_to_le16(EXT4_BG_INODE_UNINIT))) {
> + used_inos = EXT4_INODES_PER_GROUP(sb) -
> + ext4_itable_unused_count(sb, gdp);
> + used_blks = DIV_ROUND_UP(used_inos, sbi->s_inodes_per_block);
> +
> + if (used_blks >= 0 && used_blks <= sbi->s_itb_per_group)
> + used_inos += group * EXT4_INODES_PER_GROUP(sb);
Maybe if would be more comprehensible like:
/* Bogus inode unused count? */
if (used_blks < 0 || used_blks > sbi->s_itb_per_group) {
ext4_error(...);
ret = 1;
goto err_out;
}
used_inos += EXT4_INODES_PER_GROUP(sb);
/*
* Are there some uninitialized inodes in the inode table
* before the first normal inode?
*/
if (used_blks != sbi->s_itb_per_group &&
used_inos < EXT4_FIRST_INO(sb)) {
ext4_error(...);
ret = 1;
goto err_out;
}
> +
> + if ((used_blks < 0) || (used_blks > sbi->s_itb_per_group) ||
> + ((used_blks != sbi->s_itb_per_group) &&
> + (used_inos < EXT4_FIRST_INO(sb)))) {
> + ext4_error(sb, "Something is wrong with group %u: "
> + "used itable blocks: %d; "
> + "itable unused count: %u",
> + group, used_blks,
> + ext4_itable_unused_count(sb, gdp));
> + ret = 1;
> + goto err_out;
> + }
> }
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ext4: fix check to prevent false positive report of incorrect used inodes
2021-03-29 14:26 ` Jan Kara
@ 2021-03-31 3:12 ` Zhang Yi
0 siblings, 0 replies; 3+ messages in thread
From: Zhang Yi @ 2021-03-31 3:12 UTC (permalink / raw)
To: Jan Kara; +Cc: linux-ext4, tytso, adilger.kernel
On 2021/3/29 22:26, Jan Kara wrote:
> On Mon 29-03-21 14:19:55, Zhang Yi wrote:
>> Commit <50122847007> ("ext4: fix check to prevent initializing reserved
>> inodes") check the block group zero and prevent initializing reserved
>> inodes. But in some special cases, the reserved inode may not all belong
>> to the group zero, it may exist into the second group if we format
>> filesystem below.
>>
>> mkfs.ext4 -b 4096 -g 8192 -N 1024 -I 4096 /dev/sda
>>
>> So, it will end up triggering a false positive report of a corrupted
>> file system. This patch fix it by avoid check reserved inodes if no free
>> inode blocks will be zeroed.
>>
>> Fixes: 50122847007 ("ext4: fix check to prevent initializing reserved inodes")
>> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
>
> Thanks! The patch looks correct but maybe the code can be made more
> comprehensible like I suggest below?
>
>> @@ -1543,22 +1544,25 @@ int ext4_init_inode_table(struct super_block *sb, ext4_group_t group,
>> * used inodes so we need to skip blocks with used inodes in
>> * inode table.
>> */
>> - if (!(gdp->bg_flags & cpu_to_le16(EXT4_BG_INODE_UNINIT)))
>> - used_blks = DIV_ROUND_UP((EXT4_INODES_PER_GROUP(sb) -
>> - ext4_itable_unused_count(sb, gdp)),
>> - sbi->s_inodes_per_block);
>> -
>> - if ((used_blks < 0) || (used_blks > sbi->s_itb_per_group) ||
>> - ((group == 0) && ((EXT4_INODES_PER_GROUP(sb) -
>> - ext4_itable_unused_count(sb, gdp)) <
>> - EXT4_FIRST_INO(sb)))) {
>> - ext4_error(sb, "Something is wrong with group %u: "
>> - "used itable blocks: %d; "
>> - "itable unused count: %u",
>> - group, used_blks,
>> - ext4_itable_unused_count(sb, gdp));
>> - ret = 1;
>> - goto err_out;
>> + if (!(gdp->bg_flags & cpu_to_le16(EXT4_BG_INODE_UNINIT))) {
>> + used_inos = EXT4_INODES_PER_GROUP(sb) -
>> + ext4_itable_unused_count(sb, gdp);
>> + used_blks = DIV_ROUND_UP(used_inos, sbi->s_inodes_per_block);
>> +
>> + if (used_blks >= 0 && used_blks <= sbi->s_itb_per_group)
>> + used_inos += group * EXT4_INODES_PER_GROUP(sb);
>
> Maybe if would be more comprehensible like:
>
> /* Bogus inode unused count? */
> if (used_blks < 0 || used_blks > sbi->s_itb_per_group) {
> ext4_error(...);
> ret = 1;
> goto err_out;
> }
>
> used_inos += EXT4_INODES_PER_GROUP(sb);
> /*
> * Are there some uninitialized inodes in the inode table
> * before the first normal inode?
> */
> if (used_blks != sbi->s_itb_per_group &&
> used_inos < EXT4_FIRST_INO(sb)) {
> ext4_error(...);
> ret = 1;
> goto err_out;
> }
Yes, it looks more comprehensible, I will send v2 as you suggested.
Thanks,
Yi.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-03-31 3:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-29 6:19 [PATCH] ext4: fix check to prevent false positive report of incorrect used inodes Zhang Yi
2021-03-29 14:26 ` Jan Kara
2021-03-31 3:12 ` Zhang Yi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).