On Mon 15-04-19 17:19:45, Barret Rhoden wrote:
When remounting with debug_want_extra_isize, we were not performing the same checks that we do during a normal mount. That allowed us to set a value for s_want_extra_isize that reached outside the s_inode_size.
Reported-by: syzbot+f584efa0ac7213c226b7@syzkaller.appspotmail.com Signed-off-by: Barret Rhoden brho@google.com Cc: stable@vger.kernel.org
The patch looks good to me. You can add:
Reviewed-by: Jan Kara jack@suse.cz
Honza
In the current code, it looks like someone could mount with want_extra_isize with some value > 0 but less than the minimums in the s_es. If that's a bug, I can submit a follow-on patch.
Similarly, on a failed remount, sbi->s_want_extra_isize is changed to the remounted value. I can fix that too if it's a problem.
Is it OK to remount with a smaller s_want_extra_isize than the previous mount? I thought it was, but figured I'd ask while I'm looking at it.
fs/ext4/super.c | 58 +++++++++++++++++++++++++++++-------------------- 1 file changed, 34 insertions(+), 24 deletions(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 6ed4eb81e674..184944d4d8d1 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -3513,6 +3513,37 @@ int ext4_calculate_overhead(struct super_block *sb) return 0; } +static void ext4_clamp_want_extra_isize(struct super_block *sb) +{
- struct ext4_sb_info *sbi = EXT4_SB(sb);
- struct ext4_super_block *es = sbi->s_es;
- /* determine the minimum size of new large inodes, if present */
- if (sbi->s_inode_size > EXT4_GOOD_OLD_INODE_SIZE &&
sbi->s_want_extra_isize == 0) {sbi->s_want_extra_isize = sizeof(struct ext4_inode) -EXT4_GOOD_OLD_INODE_SIZE;if (ext4_has_feature_extra_isize(sb)) {if (sbi->s_want_extra_isize <le16_to_cpu(es->s_want_extra_isize))sbi->s_want_extra_isize =le16_to_cpu(es->s_want_extra_isize);if (sbi->s_want_extra_isize <le16_to_cpu(es->s_min_extra_isize))sbi->s_want_extra_isize =le16_to_cpu(es->s_min_extra_isize);}- }
- /* Check if enough inode space is available */
- if (EXT4_GOOD_OLD_INODE_SIZE + sbi->s_want_extra_isize >
sbi->s_inode_size) {sbi->s_want_extra_isize = sizeof(struct ext4_inode) -EXT4_GOOD_OLD_INODE_SIZE;ext4_msg(sb, KERN_INFO,"required extra inode space not available");- }
+}
static void ext4_set_resv_clusters(struct super_block *sb) { ext4_fsblk_t resv_clusters; @@ -4387,30 +4418,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) } else if (ret) goto failed_mount4a;
- /* determine the minimum size of new large inodes, if present */
- if (sbi->s_inode_size > EXT4_GOOD_OLD_INODE_SIZE &&
sbi->s_want_extra_isize == 0) {sbi->s_want_extra_isize = sizeof(struct ext4_inode) -EXT4_GOOD_OLD_INODE_SIZE;if (ext4_has_feature_extra_isize(sb)) {if (sbi->s_want_extra_isize <le16_to_cpu(es->s_want_extra_isize))sbi->s_want_extra_isize =le16_to_cpu(es->s_want_extra_isize);if (sbi->s_want_extra_isize <le16_to_cpu(es->s_min_extra_isize))sbi->s_want_extra_isize =le16_to_cpu(es->s_min_extra_isize);}- }
- /* Check if enough inode space is available */
- if (EXT4_GOOD_OLD_INODE_SIZE + sbi->s_want_extra_isize >
sbi->s_inode_size) {sbi->s_want_extra_isize = sizeof(struct ext4_inode) -EXT4_GOOD_OLD_INODE_SIZE;ext4_msg(sb, KERN_INFO, "required extra inode space not""available");- }
- ext4_clamp_want_extra_isize(sb);
ext4_set_resv_clusters(sb); @@ -5194,6 +5202,8 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data) goto restore_opts; }
- ext4_clamp_want_extra_isize(sb);
- if ((old_opts.s_mount_opt & EXT4_MOUNT_JOURNAL_CHECKSUM) ^ test_opt(sb, JOURNAL_CHECKSUM)) { ext4_msg(sb, KERN_ERR, "changing journal_checksum "
-- 2.21.0.392.gf8f6787159e-goog