From e03d3b1b924cbaac91ddf066e4d14a2c4d3ed1d1 Mon Sep 17 00:00:00 2001 From: Jiangshan Yi Date: Fri, 19 Aug 2022 15:52:40 +0800 Subject: fs/reiserfs: replace ternary operator with min() and min_t() Fix the following coccicheck warning: fs/reiserfs/prints.c:459: WARNING opportunity for min(). fs/reiserfs/resize.c:100: WARNING opportunity for min(). fs/reiserfs/super.c:2508: WARNING opportunity for min(). fs/reiserfs/super.c:2557: WARNING opportunity for min(). min() and min_t() macro is defined in include/linux/minmax.h. It avoids multiple evaluations of the arguments when non-constant and performs strict type-checking. Signed-off-by: Jiangshan Yi Signed-off-by: Jan Kara Link: https://lore.kernel.org/r/20220819075240.3199477-1-13667453960@163.com --- fs/reiserfs/prints.c | 2 +- fs/reiserfs/resize.c | 2 +- fs/reiserfs/super.c | 7 ++----- 3 files changed, 4 insertions(+), 7 deletions(-) (limited to 'fs') diff --git a/fs/reiserfs/prints.c b/fs/reiserfs/prints.c index 30319dc33c18..84a194b77f19 100644 --- a/fs/reiserfs/prints.c +++ b/fs/reiserfs/prints.c @@ -456,7 +456,7 @@ static int print_internal(struct buffer_head *bh, int first, int last) to = B_NR_ITEMS(bh); } else { from = first; - to = last < B_NR_ITEMS(bh) ? last : B_NR_ITEMS(bh); + to = min_t(int, last, B_NR_ITEMS(bh)); } reiserfs_printk("INTERNAL NODE (%ld) contains %z\n", bh->b_blocknr, bh); diff --git a/fs/reiserfs/resize.c b/fs/reiserfs/resize.c index 8096c74c38ac..7b498a0d060b 100644 --- a/fs/reiserfs/resize.c +++ b/fs/reiserfs/resize.c @@ -97,7 +97,7 @@ int reiserfs_resize(struct super_block *s, unsigned long block_count_new) * using the copy_size var below allows this code to work for * both shrinking and expanding the FS. */ - copy_size = bmap_nr_new < bmap_nr ? bmap_nr_new : bmap_nr; + copy_size = min(bmap_nr_new, bmap_nr); copy_size = copy_size * sizeof(struct reiserfs_list_bitmap_node *); for (i = 0; i < JOURNAL_NUM_BITMAPS; i++) { diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c index c88cd2ce0665..da1e72494e30 100644 --- a/fs/reiserfs/super.c +++ b/fs/reiserfs/super.c @@ -2504,9 +2504,7 @@ static ssize_t reiserfs_quota_read(struct super_block *sb, int type, char *data, len = i_size - off; toread = len; while (toread > 0) { - tocopy = - sb->s_blocksize - offset < - toread ? sb->s_blocksize - offset : toread; + tocopy = min_t(unsigned long, sb->s_blocksize - offset, toread); tmp_bh.b_state = 0; /* * Quota files are without tails so we can safely @@ -2554,8 +2552,7 @@ static ssize_t reiserfs_quota_write(struct super_block *sb, int type, return -EIO; } while (towrite > 0) { - tocopy = sb->s_blocksize - offset < towrite ? - sb->s_blocksize - offset : towrite; + tocopy = min_t(unsigned long, sb->s_blocksize - offset, towrite); tmp_bh.b_state = 0; reiserfs_write_lock(sb); err = reiserfs_get_block(inode, blk, &tmp_bh, GET_BLOCK_CREATE); -- cgit v1.2.3 From d4d361ad00bac10701a8c14d8e1a2967bd2e2813 Mon Sep 17 00:00:00 2001 From: Minghao Chi Date: Fri, 19 Aug 2022 08:14:20 +0000 Subject: isofs: delete unnecessary checks before brelse() The brelse() function tests whether its argument is NULL and then returns immediately. Thus remove the tests which are not needed around the shown calls. Reported-by: Zeal Robot Signed-off-by: Minghao Chi Signed-off-by: Jan Kara Link: https://lore.kernel.org/r/20220819081420.96209-1-chi.minghao@zte.com.cn --- fs/isofs/inode.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'fs') diff --git a/fs/isofs/inode.c b/fs/isofs/inode.c index 88bf20303466..df9d70588b60 100644 --- a/fs/isofs/inode.c +++ b/fs/isofs/inode.c @@ -1277,13 +1277,11 @@ static int isofs_read_level3_size(struct inode *inode) } while (more_entries); out: kfree(tmpde); - if (bh) - brelse(bh); + brelse(bh); return 0; out_nomem: - if (bh) - brelse(bh); + brelse(bh); return -ENOMEM; out_noread: @@ -1486,8 +1484,7 @@ static int isofs_read_inode(struct inode *inode, int relocated) ret = 0; out: kfree(tmpde); - if (bh) - brelse(bh); + brelse(bh); return ret; out_badread: -- cgit v1.2.3 From 6c78973da0b11255d2668518ac1baba4c581811a Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Mon, 12 Sep 2022 12:25:29 +0200 Subject: udf: Support splicing to file Add explicit support for splicing from pipe to file through iter_file_splice_write(). Commit 36e2c7421f02 ("fs: don't allow splice read/write without explicit ops") removed the default .splice_write operation which effectively removed UDF support for splicing from pipe. Fixes: 36e2c7421f02 ("fs: don't allow splice read/write without explicit ops") Reported-by: kernel test robot Link: https://lore.kernel.org/r/202209081443.593ab12-yujie.liu@intel.com Signed-off-by: Jan Kara --- fs/udf/file.c | 1 + 1 file changed, 1 insertion(+) (limited to 'fs') diff --git a/fs/udf/file.c b/fs/udf/file.c index 09aef77269fe..5c659e23e578 100644 --- a/fs/udf/file.c +++ b/fs/udf/file.c @@ -252,6 +252,7 @@ const struct file_operations udf_file_operations = { .release = udf_release_file, .fsync = generic_file_fsync, .splice_read = generic_file_splice_read, + .splice_write = iter_file_splice_write, .llseek = generic_file_llseek, }; -- cgit v1.2.3 From d766f2d1e3e3bd44024a7f971ffcf8b8fbb7c5d2 Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Wed, 14 Sep 2022 17:24:42 +0200 Subject: ext2: Add sanity checks for group and filesystem size Add sanity check that filesystem size does not exceed the underlying device size and that group size is big enough so that metadata can fit into it. This avoid trying to mount some crafted filesystems with extremely large group counts. Reported-by: syzbot+0f2f7e65a3007d39539f@syzkaller.appspotmail.com Reported-by: kernel test robot # Test fixup CC: stable@vger.kernel.org Signed-off-by: Jan Kara --- fs/ext2/super.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) (limited to 'fs') diff --git a/fs/ext2/super.c b/fs/ext2/super.c index 252c742379cf..afb31af9302d 100644 --- a/fs/ext2/super.c +++ b/fs/ext2/super.c @@ -1052,6 +1052,13 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent) sbi->s_blocks_per_group); goto failed_mount; } + /* At least inode table, bitmaps, and sb have to fit in one group */ + if (sbi->s_blocks_per_group <= sbi->s_itb_per_group + 3) { + ext2_msg(sb, KERN_ERR, + "error: #blocks per group smaller than metadata size: %lu <= %lu", + sbi->s_blocks_per_group, sbi->s_inodes_per_group + 3); + goto failed_mount; + } if (sbi->s_frags_per_group > sb->s_blocksize * 8) { ext2_msg(sb, KERN_ERR, "error: #fragments per group too big: %lu", @@ -1065,9 +1072,14 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent) sbi->s_inodes_per_group); goto failed_mount; } + if (sb_bdev_nr_blocks(sb) < le32_to_cpu(es->s_blocks_count)) { + ext2_msg(sb, KERN_ERR, + "bad geometry: block count %u exceeds size of device (%u blocks)", + le32_to_cpu(es->s_blocks_count), + (unsigned)sb_bdev_nr_blocks(sb)); + goto failed_mount; + } - if (EXT2_BLOCKS_PER_GROUP(sb) == 0) - goto cantfind_ext2; sbi->s_groups_count = ((le32_to_cpu(es->s_blocks_count) - le32_to_cpu(es->s_first_data_block) - 1) / EXT2_BLOCKS_PER_GROUP(sb)) + 1; -- cgit v1.2.3 From e7c7fbb9a8574ebd89cc05db49d806c7476863ad Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Wed, 14 Sep 2022 17:29:33 +0200 Subject: ext2: Use kvmalloc() for group descriptor array Array of group descriptor block buffers can get rather large. In theory in can reach 1MB for perfectly valid filesystem and even more for maliciously crafted ones. Use kvmalloc() to allocate the array to avoid straining memory allocator with large order allocations unnecessarily. Reported-by: syzbot+0f2f7e65a3007d39539f@syzkaller.appspotmail.com Signed-off-by: Jan Kara --- fs/ext2/super.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'fs') diff --git a/fs/ext2/super.c b/fs/ext2/super.c index afb31af9302d..03f2af98b1b4 100644 --- a/fs/ext2/super.c +++ b/fs/ext2/super.c @@ -163,7 +163,7 @@ static void ext2_put_super (struct super_block * sb) db_count = sbi->s_gdb_count; for (i = 0; i < db_count; i++) brelse(sbi->s_group_desc[i]); - kfree(sbi->s_group_desc); + kvfree(sbi->s_group_desc); kfree(sbi->s_debts); percpu_counter_destroy(&sbi->s_freeblocks_counter); percpu_counter_destroy(&sbi->s_freeinodes_counter); @@ -1092,7 +1092,7 @@ static int ext2_fill_super(struct super_block *sb, void *data, int silent) } db_count = (sbi->s_groups_count + EXT2_DESC_PER_BLOCK(sb) - 1) / EXT2_DESC_PER_BLOCK(sb); - sbi->s_group_desc = kmalloc_array(db_count, + sbi->s_group_desc = kvmalloc_array(db_count, sizeof(struct buffer_head *), GFP_KERNEL); if (sbi->s_group_desc == NULL) { @@ -1218,7 +1218,7 @@ failed_mount2: for (i = 0; i < db_count; i++) brelse(sbi->s_group_desc[i]); failed_mount_group_desc: - kfree(sbi->s_group_desc); + kvfree(sbi->s_group_desc); kfree(sbi->s_debts); failed_mount: brelse(bh); -- cgit v1.2.3 From 6c8ea8b8cd4722efd419f91ca46a2dc81b7d89a3 Mon Sep 17 00:00:00 2001 From: Zhihao Cheng Date: Fri, 23 Sep 2022 21:45:52 +0800 Subject: quota: Check next/prev free block number after reading from quota file Following process: Init: v2_read_file_info: <3> dqi_free_blk 0 dqi_free_entry 5 dqi_blks 6 Step 1. chown bin f_a -> dquot_acquire -> v2_write_dquot: qtree_write_dquot do_insert_tree find_free_dqentry get_free_dqblk write_blk(info->dqi_blocks) // info->dqi_blocks = 6, failure. The content in physical block (corresponding to blk 6) is random. Step 2. chown root f_a -> dquot_transfer -> dqput_all -> dqput -> ext4_release_dquot -> v2_release_dquot -> qtree_delete_dquot: dquot_release remove_tree free_dqentry put_free_dqblk(6) info->dqi_free_blk = blk // info->dqi_free_blk = 6 Step 3. drop cache (buffer head for block 6 is released) Step 4. chown bin f_b -> dquot_acquire -> commit_dqblk -> v2_write_dquot: qtree_write_dquot do_insert_tree find_free_dqentry get_free_dqblk dh = (struct qt_disk_dqdbheader *)buf blk = info->dqi_free_blk // 6 ret = read_blk(info, blk, buf) // The content of buf is random info->dqi_free_blk = le32_to_cpu(dh->dqdh_next_free) // random blk Step 5. chown bin f_c -> notify_change -> ext4_setattr -> dquot_transfer: dquot = dqget -> acquire_dquot -> ext4_acquire_dquot -> dquot_acquire -> commit_dqblk -> v2_write_dquot -> dq_insert_tree: do_insert_tree find_free_dqentry get_free_dqblk blk = info->dqi_free_blk // If blk < 0 and blk is not an error code, it will be returned as dquot transfer_to[USRQUOTA] = dquot // A random negative value __dquot_transfer(transfer_to) dquot_add_inodes(transfer_to[cnt]) spin_lock(&dquot->dq_dqb_lock) // page fault , which will lead to kernel page fault: Quota error (device sda): qtree_write_dquot: Error -8000 occurred while creating quota BUG: unable to handle page fault for address: ffffffffffffe120 #PF: supervisor write access in kernel mode #PF: error_code(0x0002) - not-present page Oops: 0002 [#1] PREEMPT SMP CPU: 0 PID: 5974 Comm: chown Not tainted 6.0.0-rc1-00004 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996) RIP: 0010:_raw_spin_lock+0x3a/0x90 Call Trace: dquot_add_inodes+0x28/0x270 __dquot_transfer+0x377/0x840 dquot_transfer+0xde/0x540 ext4_setattr+0x405/0x14d0 notify_change+0x68e/0x9f0 chown_common+0x300/0x430 __x64_sys_fchownat+0x29/0x40 In order to avoid accessing invalid quota memory address, this patch adds block number checking of next/prev free block read from quota file. Fetch a reproducer in [Link]. Link: https://bugzilla.kernel.org/show_bug.cgi?id=216372 Fixes: 1da177e4c3f4152 ("Linux-2.6.12-rc2") CC: stable@vger.kernel.org Link: https://lore.kernel.org/r/20220923134555.2623931-2-chengzhihao1@huawei.com Signed-off-by: Zhihao Cheng Signed-off-by: Jan Kara --- fs/quota/quota_tree.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) (limited to 'fs') diff --git a/fs/quota/quota_tree.c b/fs/quota/quota_tree.c index 5f2405994280..7e65d67de9f3 100644 --- a/fs/quota/quota_tree.c +++ b/fs/quota/quota_tree.c @@ -71,6 +71,35 @@ static ssize_t write_blk(struct qtree_mem_dqinfo *info, uint blk, char *buf) return ret; } +static inline int do_check_range(struct super_block *sb, const char *val_name, + uint val, uint min_val, uint max_val) +{ + if (val < min_val || val > max_val) { + quota_error(sb, "Getting %s %u out of range %u-%u", + val_name, val, min_val, max_val); + return -EUCLEAN; + } + + return 0; +} + +static int check_dquot_block_header(struct qtree_mem_dqinfo *info, + struct qt_disk_dqdbheader *dh) +{ + int err = 0; + + err = do_check_range(info->dqi_sb, "dqdh_next_free", + le32_to_cpu(dh->dqdh_next_free), 0, + info->dqi_blocks - 1); + if (err) + return err; + err = do_check_range(info->dqi_sb, "dqdh_prev_free", + le32_to_cpu(dh->dqdh_prev_free), 0, + info->dqi_blocks - 1); + + return err; +} + /* Remove empty block from list and return it */ static int get_free_dqblk(struct qtree_mem_dqinfo *info) { @@ -85,6 +114,9 @@ static int get_free_dqblk(struct qtree_mem_dqinfo *info) ret = read_blk(info, blk, buf); if (ret < 0) goto out_buf; + ret = check_dquot_block_header(info, dh); + if (ret) + goto out_buf; info->dqi_free_blk = le32_to_cpu(dh->dqdh_next_free); } else { @@ -232,6 +264,9 @@ static uint find_free_dqentry(struct qtree_mem_dqinfo *info, *err = read_blk(info, blk, buf); if (*err < 0) goto out_buf; + *err = check_dquot_block_header(info, dh); + if (*err) + goto out_buf; } else { blk = get_free_dqblk(info); if ((int)blk < 0) { @@ -424,6 +459,9 @@ static int free_dqentry(struct qtree_mem_dqinfo *info, struct dquot *dquot, goto out_buf; } dh = (struct qt_disk_dqdbheader *)buf; + ret = check_dquot_block_header(info, dh); + if (ret) + goto out_buf; le16_add_cpu(&dh->dqdh_entries, -1); if (!le16_to_cpu(dh->dqdh_entries)) { /* Block got free? */ ret = remove_free_dqentry(info, buf, blk); -- cgit v1.2.3 From 3fc61e0e96a3261aacfd3150fb3a9228f7ce5dd6 Mon Sep 17 00:00:00 2001 From: Zhihao Cheng Date: Fri, 23 Sep 2022 21:45:53 +0800 Subject: quota: Replace all block number checking with helper function Cleanup all block checking places, replace them with helper function do_check_range(). Link: https://lore.kernel.org/r/20220923134555.2623931-3-chengzhihao1@huawei.com Signed-off-by: Zhihao Cheng Signed-off-by: Jan Kara --- fs/quota/quota_tree.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) (limited to 'fs') diff --git a/fs/quota/quota_tree.c b/fs/quota/quota_tree.c index 7e65d67de9f3..0fa73ca28045 100644 --- a/fs/quota/quota_tree.c +++ b/fs/quota/quota_tree.c @@ -518,12 +518,10 @@ static int remove_tree(struct qtree_mem_dqinfo *info, struct dquot *dquot, goto out_buf; } newblk = le32_to_cpu(ref[get_index(info, dquot->dq_id, depth)]); - if (newblk < QT_TREEOFF || newblk >= info->dqi_blocks) { - quota_error(dquot->dq_sb, "Getting block too big (%u >= %u)", - newblk, info->dqi_blocks); - ret = -EUCLEAN; + ret = do_check_range(dquot->dq_sb, "block", newblk, QT_TREEOFF, + info->dqi_blocks - 1); + if (ret) goto out_buf; - } if (depth == info->dqi_qtree_depth - 1) { ret = free_dqentry(info, dquot, newblk); @@ -624,12 +622,10 @@ static loff_t find_tree_dqentry(struct qtree_mem_dqinfo *info, blk = le32_to_cpu(ref[get_index(info, dquot->dq_id, depth)]); if (!blk) /* No reference? */ goto out_buf; - if (blk < QT_TREEOFF || blk >= info->dqi_blocks) { - quota_error(dquot->dq_sb, "Getting block too big (%u >= %u)", - blk, info->dqi_blocks); - ret = -EUCLEAN; + ret = do_check_range(dquot->dq_sb, "block", blk, QT_TREEOFF, + info->dqi_blocks - 1); + if (ret) goto out_buf; - } if (depth < info->dqi_qtree_depth - 1) ret = find_tree_dqentry(info, dquot, blk, depth+1); -- cgit v1.2.3 From 191249f708897fc34c78f4494f7156896aaaeca9 Mon Sep 17 00:00:00 2001 From: Zhihao Cheng Date: Fri, 23 Sep 2022 21:45:54 +0800 Subject: quota: Add more checking after reading from quota file It would be better to do more sanity checking (eg. dqdh_entries, block no.) for the content read from quota file, which can prevent corrupting the quota file. Link: https://lore.kernel.org/r/20220923134555.2623931-4-chengzhihao1@huawei.com Signed-off-by: Zhihao Cheng Signed-off-by: Jan Kara --- fs/quota/quota_tree.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) (limited to 'fs') diff --git a/fs/quota/quota_tree.c b/fs/quota/quota_tree.c index 0fa73ca28045..0f1493e0f6d0 100644 --- a/fs/quota/quota_tree.c +++ b/fs/quota/quota_tree.c @@ -96,6 +96,11 @@ static int check_dquot_block_header(struct qtree_mem_dqinfo *info, err = do_check_range(info->dqi_sb, "dqdh_prev_free", le32_to_cpu(dh->dqdh_prev_free), 0, info->dqi_blocks - 1); + if (err) + return err; + err = do_check_range(info->dqi_sb, "dqdh_entries", + le16_to_cpu(dh->dqdh_entries), 0, + qtree_dqstr_in_blk(info)); return err; } @@ -348,6 +353,10 @@ static int do_insert_tree(struct qtree_mem_dqinfo *info, struct dquot *dquot, } ref = (__le32 *)buf; newblk = le32_to_cpu(ref[get_index(info, dquot->dq_id, depth)]); + ret = do_check_range(dquot->dq_sb, "block", newblk, 0, + info->dqi_blocks - 1); + if (ret) + goto out_buf; if (!newblk) newson = 1; if (depth == info->dqi_qtree_depth - 1) { @@ -739,15 +748,21 @@ static int find_next_id(struct qtree_mem_dqinfo *info, qid_t *id, goto out_buf; } for (i = __get_index(info, *id, depth); i < epb; i++) { - if (ref[i] == cpu_to_le32(0)) { + uint blk_no = le32_to_cpu(ref[i]); + + if (blk_no == 0) { *id += level_inc; continue; } + ret = do_check_range(info->dqi_sb, "block", blk_no, 0, + info->dqi_blocks - 1); + if (ret) + goto out_buf; if (depth == info->dqi_qtree_depth - 1) { ret = 0; goto out_buf; } - ret = find_next_id(info, id, le32_to_cpu(ref[i]), depth + 1); + ret = find_next_id(info, id, blk_no, depth + 1); if (ret != -ENOENT) break; } -- cgit v1.2.3