From 1dedde690303c05ef732b7c5c8356fdf60a4ade3 Mon Sep 17 00:00:00 2001 From: Zhihao Cheng Date: Tue, 21 Mar 2023 09:37:21 +0800 Subject: [PATCH 1/6] ext4: fix i_disksize exceeding i_size problem in paritally written case It is possible for i_disksize can exceed i_size, triggering a warning. generic_perform_write copied = iov_iter_copy_from_user_atomic(len) // copied < len ext4_da_write_end | ext4_update_i_disksize | new_i_size = pos + copied; | WRITE_ONCE(EXT4_I(inode)->i_disksize, newsize) // update i_disksize | generic_write_end | copied = block_write_end(copied, len) // copied = 0 | if (unlikely(copied < len)) | if (!PageUptodate(page)) | copied = 0; | if (pos + copied > inode->i_size) // return false if (unlikely(copied == 0)) goto again; if (unlikely(iov_iter_fault_in_readable(i, bytes))) { status = -EFAULT; break; } We get i_disksize greater than i_size here, which could trigger WARNING check 'i_size_read(inode) < EXT4_I(inode)->i_disksize' while doing dio: ext4_dio_write_iter iomap_dio_rw __iomap_dio_rw // return err, length is not aligned to 512 ext4_handle_inode_extension WARN_ON_ONCE(i_size_read(inode) < EXT4_I(inode)->i_disksize) // Oops WARNING: CPU: 2 PID: 2609 at fs/ext4/file.c:319 CPU: 2 PID: 2609 Comm: aa Not tainted 6.3.0-rc2 RIP: 0010:ext4_file_write_iter+0xbc7 Call Trace: vfs_write+0x3b1 ksys_write+0x77 do_syscall_64+0x39 Fix it by updating 'copied' value before updating i_disksize just like ext4_write_inline_data_end() does. A reproducer can be found in the buganizer link below. Link: https://bugzilla.kernel.org/show_bug.cgi?id=217209 Fixes: 64769240bd07 ("ext4: Add delayed allocation support in data=writeback mode") Signed-off-by: Zhihao Cheng Reviewed-by: Jan Kara Link: https://lore.kernel.org/r/20230321013721.89818-1-chengzhihao1@huawei.com Signed-off-by: Theodore Ts'o --- fs/ext4/inode.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index 8dbd352e3986..1cfbb929c694 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -2982,6 +2982,9 @@ static int ext4_da_write_end(struct file *file, ext4_has_inline_data(inode)) return ext4_write_inline_data_end(inode, pos, len, copied, page); + if (unlikely(copied < len) && !PageUptodate(page)) + copied = 0; + start = pos & (PAGE_SIZE - 1); end = start + copied - 1; From 835659598c67907b98cd2aa57bb951dfaf675c69 Mon Sep 17 00:00:00 2001 From: Ye Bin Date: Thu, 6 Apr 2023 11:16:27 +0000 Subject: [PATCH 2/6] ext4: fix use-after-free read in ext4_find_extent for bigalloc + inline Syzbot found the following issue: loop0: detected capacity change from 0 to 2048 EXT4-fs (loop0): mounted filesystem 00000000-0000-0000-0000-000000000000 without journal. Quota mode: none. ================================================================== BUG: KASAN: use-after-free in ext4_ext_binsearch_idx fs/ext4/extents.c:768 [inline] BUG: KASAN: use-after-free in ext4_find_extent+0x76e/0xd90 fs/ext4/extents.c:931 Read of size 4 at addr ffff888073644750 by task syz-executor420/5067 CPU: 0 PID: 5067 Comm: syz-executor420 Not tainted 6.2.0-rc1-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/26/2022 Call Trace: __dump_stack lib/dump_stack.c:88 [inline] dump_stack_lvl+0x1b1/0x290 lib/dump_stack.c:106 print_address_description+0x74/0x340 mm/kasan/report.c:306 print_report+0x107/0x1f0 mm/kasan/report.c:417 kasan_report+0xcd/0x100 mm/kasan/report.c:517 ext4_ext_binsearch_idx fs/ext4/extents.c:768 [inline] ext4_find_extent+0x76e/0xd90 fs/ext4/extents.c:931 ext4_clu_mapped+0x117/0x970 fs/ext4/extents.c:5809 ext4_insert_delayed_block fs/ext4/inode.c:1696 [inline] ext4_da_map_blocks fs/ext4/inode.c:1806 [inline] ext4_da_get_block_prep+0x9e8/0x13c0 fs/ext4/inode.c:1870 ext4_block_write_begin+0x6a8/0x2290 fs/ext4/inode.c:1098 ext4_da_write_begin+0x539/0x760 fs/ext4/inode.c:3082 generic_perform_write+0x2e4/0x5e0 mm/filemap.c:3772 ext4_buffered_write_iter+0x122/0x3a0 fs/ext4/file.c:285 ext4_file_write_iter+0x1d0/0x18f0 call_write_iter include/linux/fs.h:2186 [inline] new_sync_write fs/read_write.c:491 [inline] vfs_write+0x7dc/0xc50 fs/read_write.c:584 ksys_write+0x177/0x2a0 fs/read_write.c:637 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x3d/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x63/0xcd RIP: 0033:0x7f4b7a9737b9 RSP: 002b:00007ffc5cac3668 EFLAGS: 00000246 ORIG_RAX: 0000000000000001 RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f4b7a9737b9 RDX: 00000000175d9003 RSI: 0000000020000200 RDI: 0000000000000004 RBP: 00007f4b7a933050 R08: 0000000000000000 R09: 0000000000000000 R10: 000000000000079f R11: 0000000000000246 R12: 00007f4b7a9330e0 R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 Above issue is happens when enable bigalloc and inline data feature. As commit 131294c35ed6 fixed delayed allocation bug in ext4_clu_mapped for bigalloc + inline. But it only resolved issue when has inline data, if inline data has been converted to extent(ext4_da_convert_inline_data_to_extent) before writepages, there is no EXT4_STATE_MAY_INLINE_DATA flag. However i_data is still store inline data in this scene. Then will trigger UAF when find extent. To resolve above issue, there is need to add judge "ext4_has_inline_data(inode)" in ext4_clu_mapped(). Fixes: 131294c35ed6 ("ext4: fix delayed allocation bug in ext4_clu_mapped for bigalloc + inline") Reported-by: syzbot+bf4bb7731ef73b83a3b4@syzkaller.appspotmail.com Reviewed-by: Jan Kara Reviewed-by: Ye Bin Reviewed-by: Tudor Ambarus Tested-by: Tudor Ambarus Link: https://lore.kernel.org/r/20230406111627.1916759-1-tudor.ambarus@linaro.org Signed-off-by: Theodore Ts'o --- fs/ext4/extents.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index e79c767cc5e0..35703dce23a3 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -5795,7 +5795,8 @@ int ext4_clu_mapped(struct inode *inode, ext4_lblk_t lclu) * mapped - no physical clusters have been allocated, and the * file has no extents */ - if (ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA)) + if (ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA) || + ext4_has_inline_data(inode)) return 0; /* search for the extent closest to the first block in the cluster */ From 856dd6c5981260b4d1aa84b78373ad54a203db48 Mon Sep 17 00:00:00 2001 From: Nathan Chancellor Date: Thu, 20 Apr 2023 09:51:24 -0700 Subject: [PATCH 3/6] ext4: fix unused iterator variable warnings When CONFIG_QUOTA is disabled, there are warnings around unused iterator variables: fs/ext4/super.c: In function 'ext4_put_super': fs/ext4/super.c:1262:13: error: unused variable 'i' [-Werror=unused-variable] 1262 | int i, err; | ^ fs/ext4/super.c: In function '__ext4_fill_super': fs/ext4/super.c:5200:22: error: unused variable 'i' [-Werror=unused-variable] 5200 | unsigned int i; | ^ cc1: all warnings being treated as errors The kernel has updated to GNU11, allowing the variables to be declared within the for loop. Do so to clear up the warnings. Fixes: dcbf87589d90 ("ext4: factor out ext4_flex_groups_free()") Signed-off-by: Nathan Chancellor Reviewed-by: Geert Uytterhoeven Reviewed-by: Jan Kara Reviewed-by: Jason Yan Link: https://lore.kernel.org/r/20230420-ext4-unused-variables-super-c-v1-1-138b6db6c21c@kernel.org Signed-off-by: Theodore Ts'o --- fs/ext4/super.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 403cc0e6cd65..f16492b8c98d 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -1259,7 +1259,7 @@ static void ext4_put_super(struct super_block *sb) struct ext4_sb_info *sbi = EXT4_SB(sb); struct ext4_super_block *es = sbi->s_es; int aborted = 0; - int i, err; + int err; /* * Unregister sysfs before destroying jbd2 journal. @@ -1311,7 +1311,7 @@ static void ext4_put_super(struct super_block *sb) ext4_flex_groups_free(sbi); ext4_percpu_param_destroy(sbi); #ifdef CONFIG_QUOTA - for (i = 0; i < EXT4_MAXQUOTAS; i++) + for (int i = 0; i < EXT4_MAXQUOTAS; i++) kfree(get_qf_name(sb, sbi, i)); #endif @@ -5197,7 +5197,6 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb) ext4_fsblk_t logical_sb_block; struct inode *root; int ret = -ENOMEM; - unsigned int i; int needs_recovery; int err = 0; ext4_group_t first_not_zeroed; @@ -5628,7 +5627,7 @@ failed_mount: #endif #ifdef CONFIG_QUOTA - for (i = 0; i < EXT4_MAXQUOTAS; i++) + for (unsigned int i = 0; i < EXT4_MAXQUOTAS; i++) kfree(get_qf_name(sb, sbi, i)); #endif fscrypt_free_dummy_policy(&sbi->s_dummy_enc_policy); From d5e72c4e3256335d6fb75c2e321144f93141f4f5 Mon Sep 17 00:00:00 2001 From: Theodore Ts'o Date: Thu, 27 Apr 2023 19:18:01 -0400 Subject: [PATCH 4/6] ext4: fix lost error code reporting in __ext4_fill_super() When code was factored out of __ext4_fill_super() into ext4_percpu_param_init() the error return was discarded. This meant that it was possible for __ext4_fill_super() to return zero, indicating success, without the struct super getting completely filled in, leading to a potential NULL pointer dereference. Reported-by: syzbot+bbf0f9a213c94f283a5c@syzkaller.appspotmail.com Fixes: 1f79467c8a6b ("ext4: factor out ext4_percpu_param_init() ...") Link: https://syzkaller.appspot.com/bug?id=6dac47d5e58af770c0055f680369586ec32e144c Signed-off-by: Theodore Ts'o Reviewed-by: Jason Yan --- fs/ext4/super.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/ext4/super.c b/fs/ext4/super.c index f16492b8c98d..8ed6450effaa 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -5502,7 +5502,8 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb) sbi->s_journal->j_commit_callback = ext4_journal_commit_callback; - if (ext4_percpu_param_init(sbi)) + err = ext4_percpu_param_init(sbi); + if (err) goto failed_mount6; if (ext4_has_feature_flex_bg(sb)) From 3b50d5018ed06a647bb26c44bb5ae74e59c903c7 Mon Sep 17 00:00:00 2001 From: Theodore Ts'o Date: Thu, 27 Apr 2023 22:49:34 -0400 Subject: [PATCH 5/6] ext4: reflect error codes from ext4_multi_mount_protect() to its callers This will allow more fine-grained errno codes to be returned by the mount system call. Cc: Andreas Dilger Signed-off-by: Theodore Ts'o --- fs/ext4/mmp.c | 9 ++++++++- fs/ext4/super.c | 16 +++++++++------- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c index 4681fff6665f..4022bc713421 100644 --- a/fs/ext4/mmp.c +++ b/fs/ext4/mmp.c @@ -282,6 +282,7 @@ int ext4_multi_mount_protect(struct super_block *sb, if (mmp_block < le32_to_cpu(es->s_first_data_block) || mmp_block >= ext4_blocks_count(es)) { ext4_warning(sb, "Invalid MMP block in superblock"); + retval = -EINVAL; goto failed; } @@ -307,6 +308,7 @@ int ext4_multi_mount_protect(struct super_block *sb, if (seq == EXT4_MMP_SEQ_FSCK) { dump_mmp_msg(sb, mmp, "fsck is running on the filesystem"); + retval = -EBUSY; goto failed; } @@ -320,6 +322,7 @@ int ext4_multi_mount_protect(struct super_block *sb, if (schedule_timeout_interruptible(HZ * wait_time) != 0) { ext4_warning(sb, "MMP startup interrupted, failing mount\n"); + retval = -ETIMEDOUT; goto failed; } @@ -330,6 +333,7 @@ int ext4_multi_mount_protect(struct super_block *sb, if (seq != le32_to_cpu(mmp->mmp_seq)) { dump_mmp_msg(sb, mmp, "Device is already active on another node."); + retval = -EBUSY; goto failed; } @@ -349,6 +353,7 @@ skip: */ if (schedule_timeout_interruptible(HZ * wait_time) != 0) { ext4_warning(sb, "MMP startup interrupted, failing mount"); + retval = -ETIMEDOUT; goto failed; } @@ -359,6 +364,7 @@ skip: if (seq != le32_to_cpu(mmp->mmp_seq)) { dump_mmp_msg(sb, mmp, "Device is already active on another node."); + retval = -EBUSY; goto failed; } @@ -378,6 +384,7 @@ skip: EXT4_SB(sb)->s_mmp_tsk = NULL; ext4_warning(sb, "Unable to create kmmpd thread for %s.", sb->s_id); + retval = -ENOMEM; goto failed; } @@ -385,5 +392,5 @@ skip: failed: brelse(bh); - return 1; + return retval; } diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 8ed6450effaa..c8ae6b7e8615 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -5328,9 +5328,11 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb) ext4_has_feature_orphan_present(sb) || ext4_has_feature_journal_needs_recovery(sb)); - if (ext4_has_feature_mmp(sb) && !sb_rdonly(sb)) - if (ext4_multi_mount_protect(sb, le64_to_cpu(es->s_mmp_block))) + if (ext4_has_feature_mmp(sb) && !sb_rdonly(sb)) { + err = ext4_multi_mount_protect(sb, le64_to_cpu(es->s_mmp_block)); + if (err) goto failed_mount3a; + } /* * The first inode we look at is the journal inode. Don't try @@ -6565,12 +6567,12 @@ static int __ext4_remount(struct fs_context *fc, struct super_block *sb) goto restore_opts; sb->s_flags &= ~SB_RDONLY; - if (ext4_has_feature_mmp(sb)) - if (ext4_multi_mount_protect(sb, - le64_to_cpu(es->s_mmp_block))) { - err = -EROFS; + if (ext4_has_feature_mmp(sb)) { + err = ext4_multi_mount_protect(sb, + le64_to_cpu(es->s_mmp_block)); + if (err) goto restore_opts; - } + } #ifdef CONFIG_QUOTA enable_quota = 1; #endif From d4fab7b28e2f5d74790d47a8d298da0abfb5132f Mon Sep 17 00:00:00 2001 From: Theodore Ts'o Date: Thu, 27 Apr 2023 23:01:41 -0400 Subject: [PATCH 6/6] 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 Reviewed-by: Jason Yan --- fs/ext4/super.c | 51 ++++++++++++++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/fs/ext4/super.c b/fs/ext4/super.c index c8ae6b7e8615..39f00f05f981 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -5196,9 +5196,8 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb) struct ext4_sb_info *sbi = EXT4_SB(sb); ext4_fsblk_t logical_sb_block; struct inode *root; - int ret = -ENOMEM; int needs_recovery; - int err = 0; + int err; ext4_group_t first_not_zeroed; struct ext4_fs_context *ctx = fc->fs_private; int silent = fc->sb_flags & SB_SILENT; @@ -5211,8 +5210,6 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb) sbi->s_sectors_written_start = part_stat_read(sb->s_bdev, sectors[STAT_WRITE]); - /* -EINVAL is default */ - ret = -EINVAL; err = ext4_load_super(sb, &logical_sb_block, silent); if (err) goto out_fail; @@ -5238,7 +5235,8 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb) */ sbi->s_li_wait_mult = EXT4_DEF_LI_WAIT_MULT; - if (ext4_inode_info_init(sb, es)) + err = ext4_inode_info_init(sb, es); + if (err) goto failed_mount; 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) ext4_apply_options(fc, sb); - if (ext4_encoding_init(sb, es)) + err = ext4_encoding_init(sb, es); + if (err) goto failed_mount; - if (ext4_check_journal_data_mode(sb)) + err = ext4_check_journal_data_mode(sb); + if (err) goto failed_mount; 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) /* i_version is always enabled now */ sb->s_flags |= SB_I_VERSION; - if (ext4_check_feature_compatibility(sb, es, silent)) + err = ext4_check_feature_compatibility(sb, es, silent); + if (err) goto failed_mount; - if (ext4_block_group_meta_init(sb, silent)) + err = ext4_block_group_meta_init(sb, silent); + if (err) goto failed_mount; ext4_hash_info_init(sb); - if (ext4_handle_clustersize(sb)) + err = ext4_handle_clustersize(sb); + if (err) goto failed_mount; - if (ext4_check_geometry(sb, es)) + err = ext4_check_geometry(sb, es); + if (err) goto failed_mount; 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) if (err) goto failed_mount3; - /* Register extent status tree shrinker */ - if (ext4_es_register_shrinker(sbi)) + err = ext4_es_register_shrinker(sbi); + if (err) goto failed_mount3; 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) goto failed_mount3a; } + err = -EINVAL; /* * The first inode we look at is the journal inode. Don't try * 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) if (!sbi->s_ea_block_cache) { ext4_msg(sb, KERN_ERR, "Failed to create ea_block_cache"); + err = -EINVAL; goto failed_mount_wq; } @@ -5393,6 +5399,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb) if (!sbi->s_ea_inode_cache) { ext4_msg(sb, KERN_ERR, "Failed to create ea_inode_cache"); + err = -EINVAL; goto failed_mount_wq; } } @@ -5427,7 +5434,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb) alloc_workqueue("ext4-rsv-conversion", WQ_MEM_RECLAIM | WQ_UNBOUND, 1); if (!EXT4_SB(sb)->rsv_conversion_wq) { printk(KERN_ERR "EXT4-fs: failed to create workqueue\n"); - ret = -ENOMEM; + err = -ENOMEM; goto failed_mount4; } @@ -5439,28 +5446,28 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb) root = ext4_iget(sb, EXT4_ROOT_INO, EXT4_IGET_SPECIAL); if (IS_ERR(root)) { ext4_msg(sb, KERN_ERR, "get root inode failed"); - ret = PTR_ERR(root); + err = PTR_ERR(root); root = NULL; goto failed_mount4; } if (!S_ISDIR(root->i_mode) || !root->i_blocks || !root->i_size) { ext4_msg(sb, KERN_ERR, "corrupt root inode, run e2fsck"); iput(root); + err = -EFSCORRUPTED; goto failed_mount4; } sb->s_root = d_make_root(root); if (!sb->s_root) { ext4_msg(sb, KERN_ERR, "get root dentry failed"); - ret = -ENOMEM; + err = -ENOMEM; goto failed_mount4; } - ret = ext4_setup_super(sb, es, sb_rdonly(sb)); - if (ret == -EROFS) { + err = ext4_setup_super(sb, es, sb_rdonly(sb)); + if (err == -EROFS) { sb->s_flags |= SB_RDONLY; - ret = 0; - } else if (ret) + } else if (err) goto failed_mount4a; ext4_set_resv_clusters(sb); @@ -5513,7 +5520,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb) ext4_msg(sb, KERN_ERR, "unable to initialize " "flex_bg meta info!"); - ret = -ENOMEM; + err = -ENOMEM; goto failed_mount6; } @@ -5639,7 +5646,7 @@ failed_mount: ext4_blkdev_remove(sbi); out_fail: sb->s_fs_info = NULL; - return err ? err : ret; + return err; } static int ext4_fill_super(struct super_block *sb, struct fs_context *fc)