ext4_resize_fs() has an off-by-one bug when checking whether growing of a filesystem will not overflow inode count. As a result it allows a filesystem with 8192 inodes per group to grow to 64TB which overflows inode count to 0 and makes filesystem unusable. Fix it.
CC: stable@vger.kernel.org Fixes: 3f8a6411fbada1fa482276591e037f3b1adcf55b Reported-by: Jaco Kroon jaco@uls.co.za Signed-off-by: Jan Kara jack@suse.cz --- fs/ext4/resize.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c index b6bec270a8e4..d792b7689d92 100644 --- a/fs/ext4/resize.c +++ b/fs/ext4/resize.c @@ -1933,7 +1933,7 @@ int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count) return 0;
n_group = ext4_get_group_number(sb, n_blocks_count - 1); - if (n_group > (0xFFFFFFFFUL / EXT4_INODES_PER_GROUP(sb))) { + if (n_group >= (0xFFFFFFFFUL / EXT4_INODES_PER_GROUP(sb))) { ext4_warning(sb, "resize would cause inodes_count overflow"); return -EINVAL; }
Hi Jan,
To confirm, this would prevent you from resizing the filesystem completely and put a limit on the max size.
Would another approach possibly be to make the additional blocks available, but without allocating additional inodes by way of flex groups?
In other words, accept the resize below, but then lower down in the same function where flex groups gets added, only add until the overflow would occur. Or make it overflow once, set number of inodes to 2^32-1 and then stop adding more? In my (and I'm betting most use-cases) this would en up wasting 1 potential inode.
That way the resize can still proceed.
The check that is then currently tripping up userspace tools remain in place, but will also allow for the special value of 2^32-1 if (and only if) the calculation for what the number of inodes should be ends up being greater than equal to 2^32 (would require special code to avoid the wrap-around condition). For the prevent overflow case the logic may actually end up being marginally more complicated.
Would that make sense? I'd be willing to attempt a patch, but I'll have to check if I've got some way to actually test this somewhere.
If that does not make sense and a hard limit on the ext4 size based on the inode count to size ratio is a given:
On 24/05/2018 13:36, Jan Kara wrote:
ext4_resize_fs() has an off-by-one bug when checking whether growing of a filesystem will not overflow inode count. As a result it allows a filesystem with 8192 inodes per group to grow to 64TB which overflows inode count to 0 and makes filesystem unusable. Fix it.
CC: stable@vger.kernel.org Fixes: 3f8a6411fbada1fa482276591e037f3b1adcf55b Reported-by: Jaco Kroon jaco@uls.co.za Signed-off-by: Jan Kara jack@suse.cz
Acked-By: Jaco Kroon jaco@uls.co.za
fs/ext4/resize.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c index b6bec270a8e4..d792b7689d92 100644 --- a/fs/ext4/resize.c +++ b/fs/ext4/resize.c @@ -1933,7 +1933,7 @@ int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count) return 0; n_group = ext4_get_group_number(sb, n_blocks_count - 1);
- if (n_group > (0xFFFFFFFFUL / EXT4_INODES_PER_GROUP(sb))) {
- if (n_group >= (0xFFFFFFFFUL / EXT4_INODES_PER_GROUP(sb))) { ext4_warning(sb, "resize would cause inodes_count overflow"); return -EINVAL; }
Hi Jaco,
On Thu 24-05-18 15:52:50, Jaco Kroon wrote:
To confirm, this would prevent you from resizing the filesystem completely and put a limit on the max size.
Yes, but the limit was already there. There was just an off-by-one error in the computation... So your filesystem was currently limited to 64TB but with a fixed test it would be limited to 64TB - 128MB.
Would another approach possibly be to make the additional blocks available, but without allocating additional inodes by way of flex groups?
In other words, accept the resize below, but then lower down in the same function where flex groups gets added, only add until the overflow would occur. Or make it overflow once, set number of inodes to 2^32-1 and then stop adding more? In my (and I'm betting most use-cases) this would en up wasting 1 potential inode.
So I don't think this is worth the complexity just to go up from 64TB-128MB to 64TB. It would be reasonably doable and maybe worth it to allow filesystems to grow arbitrarily by making groups that would overflow the s_inodes_count limit just not have any inodes. But it would not be completely trivial as there's quite some code (especially in e2fsprogs) assuming that every group has the same amount of inodes and now we'd have to teach all that code that there can be groups without inodes. So someone would have to be dedicated enough to implement and test this. OTOH the code is not *that* involved so if you wanted to try that, please go ahead.
Honza
On May 24, 2018, at 5:36 AM, Jan Kara jack@suse.cz wrote:
ext4_resize_fs() has an off-by-one bug when checking whether growing of a filesystem will not overflow inode count. As a result it allows a filesystem with 8192 inodes per group to grow to 64TB which overflows inode count to 0 and makes filesystem unusable. Fix it.
CC: stable@vger.kernel.org Fixes: 3f8a6411fbada1fa482276591e037f3b1adcf55b Reported-by: Jaco Kroon jaco@uls.co.za Signed-off-by: Jan Kara jack@suse.cz
This should be enough to stop the filesystem from exploding. We can look at more complex solutions in a separate patch.
Reviewed-by: Andreas Dilger adilger@dilger.ca
fs/ext4/resize.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c index b6bec270a8e4..d792b7689d92 100644 --- a/fs/ext4/resize.c +++ b/fs/ext4/resize.c @@ -1933,7 +1933,7 @@ int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count) return 0;
n_group = ext4_get_group_number(sb, n_blocks_count - 1);
- if (n_group > (0xFFFFFFFFUL / EXT4_INODES_PER_GROUP(sb))) {
- if (n_group >= (0xFFFFFFFFUL / EXT4_INODES_PER_GROUP(sb))) { ext4_warning(sb, "resize would cause inodes_count overflow"); return -EINVAL; }
-- 2.13.6
Cheers, Andreas
On Thu, May 24, 2018 at 01:36:46PM +0200, Jan Kara wrote:
ext4_resize_fs() has an off-by-one bug when checking whether growing of a filesystem will not overflow inode count. As a result it allows a filesystem with 8192 inodes per group to grow to 64TB which overflows inode count to 0 and makes filesystem unusable. Fix it.
CC: stable@vger.kernel.org Fixes: 3f8a6411fbada1fa482276591e037f3b1adcf55b Reported-by: Jaco Kroon jaco@uls.co.za Signed-off-by: Jan Kara jack@suse.cz
Applied, thanks.
- Ted
linux-stable-mirror@lists.linaro.org
-
Andreas Dilger -
Jaco Kroon -
Jan Kara -
Theodore Y. Ts'o