Skip to content

Commit d4fab7b

Browse files
committed
ext4: clean up error handling in __ext4_fill_super()
There were two ways to return an error code; one was via setting the 'err' variable, and the second, if err was zero, was via the 'ret' variable. This was both confusing and fragile, and when code was factored out of __ext4_fill_super(), some of the error codes returned by the original code was replaced by -EINVAL, and in one case, the error code was placed by 0, triggering a kernel null pointer dereference. Clean this up by removing the 'ret' variable, leaving only one way to set the error code to be returned, and restore the errno codes that were returned via the the mount system call as they were before we started refactoring __ext4_fill_super(). Signed-off-by: Theodore Ts'o <tytso@mit.edu> Reviewed-by: Jason Yan <yanaijie@huawei.com>
1 parent 3b50d50 commit d4fab7b

1 file changed

Lines changed: 29 additions & 22 deletions

File tree

fs/ext4/super.c

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5196,9 +5196,8 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
51965196
struct ext4_sb_info *sbi = EXT4_SB(sb);
51975197
ext4_fsblk_t logical_sb_block;
51985198
struct inode *root;
5199-
int ret = -ENOMEM;
52005199
int needs_recovery;
5201-
int err = 0;
5200+
int err;
52025201
ext4_group_t first_not_zeroed;
52035202
struct ext4_fs_context *ctx = fc->fs_private;
52045203
int silent = fc->sb_flags & SB_SILENT;
@@ -5211,8 +5210,6 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
52115210
sbi->s_sectors_written_start =
52125211
part_stat_read(sb->s_bdev, sectors[STAT_WRITE]);
52135212

5214-
/* -EINVAL is default */
5215-
ret = -EINVAL;
52165213
err = ext4_load_super(sb, &logical_sb_block, silent);
52175214
if (err)
52185215
goto out_fail;
@@ -5238,7 +5235,8 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
52385235
*/
52395236
sbi->s_li_wait_mult = EXT4_DEF_LI_WAIT_MULT;
52405237

5241-
if (ext4_inode_info_init(sb, es))
5238+
err = ext4_inode_info_init(sb, es);
5239+
if (err)
52425240
goto failed_mount;
52435241

52445242
err = parse_apply_sb_mount_options(sb, ctx);
@@ -5254,10 +5252,12 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
52545252

52555253
ext4_apply_options(fc, sb);
52565254

5257-
if (ext4_encoding_init(sb, es))
5255+
err = ext4_encoding_init(sb, es);
5256+
if (err)
52585257
goto failed_mount;
52595258

5260-
if (ext4_check_journal_data_mode(sb))
5259+
err = ext4_check_journal_data_mode(sb);
5260+
if (err)
52615261
goto failed_mount;
52625262

52635263
sb->s_flags = (sb->s_flags & ~SB_POSIXACL) |
@@ -5266,18 +5266,22 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
52665266
/* i_version is always enabled now */
52675267
sb->s_flags |= SB_I_VERSION;
52685268

5269-
if (ext4_check_feature_compatibility(sb, es, silent))
5269+
err = ext4_check_feature_compatibility(sb, es, silent);
5270+
if (err)
52705271
goto failed_mount;
52715272

5272-
if (ext4_block_group_meta_init(sb, silent))
5273+
err = ext4_block_group_meta_init(sb, silent);
5274+
if (err)
52735275
goto failed_mount;
52745276

52755277
ext4_hash_info_init(sb);
52765278

5277-
if (ext4_handle_clustersize(sb))
5279+
err = ext4_handle_clustersize(sb);
5280+
if (err)
52785281
goto failed_mount;
52795282

5280-
if (ext4_check_geometry(sb, es))
5283+
err = ext4_check_geometry(sb, es);
5284+
if (err)
52815285
goto failed_mount;
52825286

52835287
timer_setup(&sbi->s_err_report, print_daily_error_info, 0);
@@ -5288,8 +5292,8 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
52885292
if (err)
52895293
goto failed_mount3;
52905294

5291-
/* Register extent status tree shrinker */
5292-
if (ext4_es_register_shrinker(sbi))
5295+
err = ext4_es_register_shrinker(sbi);
5296+
if (err)
52935297
goto failed_mount3;
52945298

52955299
sbi->s_stripe = ext4_get_stripe_size(sbi);
@@ -5334,6 +5338,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
53345338
goto failed_mount3a;
53355339
}
53365340

5341+
err = -EINVAL;
53375342
/*
53385343
* The first inode we look at is the journal inode. Don't try
53395344
* root first: it may be modified in the journal!
@@ -5385,6 +5390,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
53855390
if (!sbi->s_ea_block_cache) {
53865391
ext4_msg(sb, KERN_ERR,
53875392
"Failed to create ea_block_cache");
5393+
err = -EINVAL;
53885394
goto failed_mount_wq;
53895395
}
53905396

@@ -5393,6 +5399,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
53935399
if (!sbi->s_ea_inode_cache) {
53945400
ext4_msg(sb, KERN_ERR,
53955401
"Failed to create ea_inode_cache");
5402+
err = -EINVAL;
53965403
goto failed_mount_wq;
53975404
}
53985405
}
@@ -5427,7 +5434,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
54275434
alloc_workqueue("ext4-rsv-conversion", WQ_MEM_RECLAIM | WQ_UNBOUND, 1);
54285435
if (!EXT4_SB(sb)->rsv_conversion_wq) {
54295436
printk(KERN_ERR "EXT4-fs: failed to create workqueue\n");
5430-
ret = -ENOMEM;
5437+
err = -ENOMEM;
54315438
goto failed_mount4;
54325439
}
54335440

@@ -5439,28 +5446,28 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
54395446
root = ext4_iget(sb, EXT4_ROOT_INO, EXT4_IGET_SPECIAL);
54405447
if (IS_ERR(root)) {
54415448
ext4_msg(sb, KERN_ERR, "get root inode failed");
5442-
ret = PTR_ERR(root);
5449+
err = PTR_ERR(root);
54435450
root = NULL;
54445451
goto failed_mount4;
54455452
}
54465453
if (!S_ISDIR(root->i_mode) || !root->i_blocks || !root->i_size) {
54475454
ext4_msg(sb, KERN_ERR, "corrupt root inode, run e2fsck");
54485455
iput(root);
5456+
err = -EFSCORRUPTED;
54495457
goto failed_mount4;
54505458
}
54515459

54525460
sb->s_root = d_make_root(root);
54535461
if (!sb->s_root) {
54545462
ext4_msg(sb, KERN_ERR, "get root dentry failed");
5455-
ret = -ENOMEM;
5463+
err = -ENOMEM;
54565464
goto failed_mount4;
54575465
}
54585466

5459-
ret = ext4_setup_super(sb, es, sb_rdonly(sb));
5460-
if (ret == -EROFS) {
5467+
err = ext4_setup_super(sb, es, sb_rdonly(sb));
5468+
if (err == -EROFS) {
54615469
sb->s_flags |= SB_RDONLY;
5462-
ret = 0;
5463-
} else if (ret)
5470+
} else if (err)
54645471
goto failed_mount4a;
54655472

54665473
ext4_set_resv_clusters(sb);
@@ -5513,7 +5520,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
55135520
ext4_msg(sb, KERN_ERR,
55145521
"unable to initialize "
55155522
"flex_bg meta info!");
5516-
ret = -ENOMEM;
5523+
err = -ENOMEM;
55175524
goto failed_mount6;
55185525
}
55195526

@@ -5639,7 +5646,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
56395646
ext4_blkdev_remove(sbi);
56405647
out_fail:
56415648
sb->s_fs_info = NULL;
5642-
return err ? err : ret;
5649+
return err;
56435650
}
56445651

56455652
static int ext4_fill_super(struct super_block *sb, struct fs_context *fc)

0 commit comments

Comments
 (0)