From 0dafb7e671f0e769c854609548037754a68dcbc6 Mon Sep 17 00:00:00 2001 From: Li zeming Date: Wed, 12 Oct 2022 18:42:35 +0800 Subject: [PATCH 01/14] fs: udf: Optimize udf_free_in_core_inode and udf_find_fileset function These two functions perform the following optimizations. 1. Delete the type cast of foo pointer. Void * does not need to convert the type. 2. Delete the initialization assignment of bh variable, which is assigned first. Signed-off-by: Li zeming Signed-off-by: Jan Kara Link: https://lore.kernel.org/r/20221012104235.3331-1-zeming@nfschina.com --- fs/udf/super.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/udf/super.c b/fs/udf/super.c index 4042d9739fb7..06eda8177b5f 100644 --- a/fs/udf/super.c +++ b/fs/udf/super.c @@ -162,7 +162,7 @@ static void udf_free_in_core_inode(struct inode *inode) static void init_once(void *foo) { - struct udf_inode_info *ei = (struct udf_inode_info *)foo; + struct udf_inode_info *ei = foo; ei->i_data = NULL; inode_init_once(&ei->vfs_inode); @@ -820,7 +820,7 @@ static int udf_find_fileset(struct super_block *sb, struct kernel_lb_addr *fileset, struct kernel_lb_addr *root) { - struct buffer_head *bh = NULL; + struct buffer_head *bh; uint16_t ident; int ret; From c791730f2554a9ebb8f18df9368dc27d4ebc38c2 Mon Sep 17 00:00:00 2001 From: Shigeru Yoshida Date: Sun, 23 Oct 2022 18:57:41 +0900 Subject: [PATCH 02/14] udf: Avoid double brelse() in udf_rename() syzbot reported a warning like below [1]: VFS: brelse: Trying to free free buffer WARNING: CPU: 2 PID: 7301 at fs/buffer.c:1145 __brelse+0x67/0xa0 ... Call Trace: invalidate_bh_lru+0x99/0x150 smp_call_function_many_cond+0xe2a/0x10c0 ? generic_remap_file_range_prep+0x50/0x50 ? __brelse+0xa0/0xa0 ? __mutex_lock+0x21c/0x12d0 ? smp_call_on_cpu+0x250/0x250 ? rcu_read_lock_sched_held+0xb/0x60 ? lock_release+0x587/0x810 ? __brelse+0xa0/0xa0 ? generic_remap_file_range_prep+0x50/0x50 on_each_cpu_cond_mask+0x3c/0x80 blkdev_flush_mapping+0x13a/0x2f0 blkdev_put_whole+0xd3/0xf0 blkdev_put+0x222/0x760 deactivate_locked_super+0x96/0x160 deactivate_super+0xda/0x100 cleanup_mnt+0x222/0x3d0 task_work_run+0x149/0x240 ? task_work_cancel+0x30/0x30 do_exit+0xb29/0x2a40 ? reacquire_held_locks+0x4a0/0x4a0 ? do_raw_spin_lock+0x12a/0x2b0 ? mm_update_next_owner+0x7c0/0x7c0 ? rwlock_bug.part.0+0x90/0x90 ? zap_other_threads+0x234/0x2d0 do_group_exit+0xd0/0x2a0 __x64_sys_exit_group+0x3a/0x50 do_syscall_64+0x34/0xb0 entry_SYSCALL_64_after_hwframe+0x63/0xcd The cause of the issue is that brelse() is called on both ofibh.sbh and ofibh.ebh by udf_find_entry() when it returns NULL. However, brelse() is called by udf_rename(), too. So, b_count on buffer_head becomes unbalanced. This patch fixes the issue by not calling brelse() by udf_rename() when udf_find_entry() returns NULL. Link: https://syzkaller.appspot.com/bug?id=8297f45698159c6bca8a1f87dc983667c1a1c851 [1] Reported-by: syzbot+7902cd7684bc35306224@syzkaller.appspotmail.com Signed-off-by: Shigeru Yoshida Signed-off-by: Jan Kara Link: https://lore.kernel.org/r/20221023095741.271430-1-syoshida@redhat.com --- fs/udf/namei.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/fs/udf/namei.c b/fs/udf/namei.c index fb4c30e05245..d6081538bfc0 100644 --- a/fs/udf/namei.c +++ b/fs/udf/namei.c @@ -1091,8 +1091,9 @@ static int udf_rename(struct user_namespace *mnt_userns, struct inode *old_dir, return -EINVAL; ofi = udf_find_entry(old_dir, &old_dentry->d_name, &ofibh, &ocfi); - if (IS_ERR(ofi)) { - retval = PTR_ERR(ofi); + if (!ofi || IS_ERR(ofi)) { + if (IS_ERR(ofi)) + retval = PTR_ERR(ofi); goto end_rename; } @@ -1101,8 +1102,7 @@ static int udf_rename(struct user_namespace *mnt_userns, struct inode *old_dir, brelse(ofibh.sbh); tloc = lelb_to_cpu(ocfi.icb.extLocation); - if (!ofi || udf_get_lb_pblock(old_dir->i_sb, &tloc, 0) - != old_inode->i_ino) + if (udf_get_lb_pblock(old_dir->i_sb, &tloc, 0) != old_inode->i_ino) goto end_rename; nfi = udf_find_entry(new_dir, &new_dentry->d_name, &nfibh, &ncfi); From ab7720a2b1175b21cace83757a9fd70408156bae Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Tue, 25 Oct 2022 11:30:28 +0200 Subject: [PATCH 03/14] maintainers: Add ISOFS entry We miss ISOFS entry in MAINTAINERS file. Add it and write me as the maintainer there since ISOFS is pretty low effort these days. Less random patches for Andrew to merge ;-). Signed-off-by: Jan Kara --- MAINTAINERS | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index e04d944005ba..20667c64dad3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10893,6 +10893,13 @@ F: drivers/isdn/Makefile F: drivers/isdn/hardware/ F: drivers/isdn/mISDN/ +ISOFS FILESYSTEM +M: Jan Kara +L: linux-fsdevel@vger.kernel.org +S: Maintained +F: Documentation/filesystems/isofs.rst +F: fs/isofs/ + IT87 HARDWARE MONITORING DRIVER M: Jean Delvare L: linux-hwmon@vger.kernel.org From d030bd1a66580b7b198a085e7220c794bcdc2770 Mon Sep 17 00:00:00 2001 From: Bo Liu Date: Fri, 11 Nov 2022 01:43:52 -0500 Subject: [PATCH 04/14] ext2: Fix some kernel-doc warnings The current code provokes some kernel-doc warnings: fs/ext2/dir.c:417: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst Signed-off-by: Bo Liu Acked-by: Randy Dunlap Signed-off-by: Jan Kara --- fs/ext2/dir.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c index 8f597753ac12..27873733ed8c 100644 --- a/fs/ext2/dir.c +++ b/fs/ext2/dir.c @@ -413,7 +413,7 @@ found: return de; } -/** +/* * Return the '..' directory entry and the page in which the entry was found * (as a parameter - p). * From bc943f4872a722c5cc64d1cf41daaaf4ec63158e Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Wed, 16 Nov 2022 19:08:23 +0100 Subject: [PATCH 05/14] ext2: Don't flush page immediately for DIRSYNC directories We do not need to writeout modified directory blocks immediately when modifying them while the page is locked. It is enough to do the flush somewhat later which has the added benefit that inode times can be flushed as well. It also allows us to stop depending on write_one_page() function. Signed-off-by: Jan Kara --- fs/ext2/dir.c | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c index 27873733ed8c..6fa714dbee84 100644 --- a/fs/ext2/dir.c +++ b/fs/ext2/dir.c @@ -81,11 +81,10 @@ ext2_last_byte(struct inode *inode, unsigned long page_nr) return last_byte; } -static int ext2_commit_chunk(struct page *page, loff_t pos, unsigned len) +static void ext2_commit_chunk(struct page *page, loff_t pos, unsigned len) { struct address_space *mapping = page->mapping; struct inode *dir = mapping->host; - int err = 0; inode_inc_iversion(dir); block_write_end(NULL, mapping, pos, len, len, page, NULL); @@ -94,16 +93,7 @@ static int ext2_commit_chunk(struct page *page, loff_t pos, unsigned len) i_size_write(dir, pos+len); mark_inode_dirty(dir); } - - if (IS_DIRSYNC(dir)) { - err = write_one_page(page); - if (!err) - err = sync_inode_metadata(dir, 1); - } else { - unlock_page(page); - } - - return err; + unlock_page(page); } static bool ext2_check_page(struct page *page, int quiet, char *kaddr) @@ -460,6 +450,17 @@ static int ext2_prepare_chunk(struct page *page, loff_t pos, unsigned len) return __block_write_begin(page, pos, len, ext2_get_block); } + +static int ext2_handle_dirsync(struct inode *dir) +{ + int err; + + err = filemap_write_and_wait(dir->i_mapping); + if (!err) + err = sync_inode_metadata(dir, 1); + return err; +} + void ext2_set_link(struct inode *dir, struct ext2_dir_entry_2 *de, struct page *page, void *page_addr, struct inode *inode, int update_times) @@ -474,11 +475,12 @@ void ext2_set_link(struct inode *dir, struct ext2_dir_entry_2 *de, BUG_ON(err); de->inode = cpu_to_le32(inode->i_ino); ext2_set_de_type(de, inode); - err = ext2_commit_chunk(page, pos, len); + ext2_commit_chunk(page, pos, len); if (update_times) dir->i_mtime = dir->i_ctime = current_time(dir); EXT2_I(dir)->i_flags &= ~EXT2_BTREE_FL; mark_inode_dirty(dir); + ext2_handle_dirsync(dir); } /* @@ -566,10 +568,11 @@ got_it: memcpy(de->name, name, namelen); de->inode = cpu_to_le32(inode->i_ino); ext2_set_de_type (de, inode); - err = ext2_commit_chunk(page, pos, rec_len); + ext2_commit_chunk(page, pos, rec_len); dir->i_mtime = dir->i_ctime = current_time(dir); EXT2_I(dir)->i_flags &= ~EXT2_BTREE_FL; mark_inode_dirty(dir); + err = ext2_handle_dirsync(dir); /* OFFSET_CACHE */ out_put: ext2_put_page(page, page_addr); @@ -615,10 +618,11 @@ int ext2_delete_entry (struct ext2_dir_entry_2 *dir, struct page *page, if (pde) pde->rec_len = ext2_rec_len_to_disk(to - from); dir->inode = 0; - err = ext2_commit_chunk(page, pos, to - from); + ext2_commit_chunk(page, pos, to - from); inode->i_ctime = inode->i_mtime = current_time(inode); EXT2_I(inode)->i_flags &= ~EXT2_BTREE_FL; mark_inode_dirty(inode); + err = ext2_handle_dirsync(inode); out: return err; } @@ -658,7 +662,8 @@ int ext2_make_empty(struct inode *inode, struct inode *parent) memcpy (de->name, "..\0", 4); ext2_set_de_type (de, inode); kunmap_atomic(kaddr); - err = ext2_commit_chunk(page, 0, chunk_size); + ext2_commit_chunk(page, 0, chunk_size); + err = ext2_handle_dirsync(inode); fail: put_page(page); return err; From a27c442d61cea70f38d9340528225b234888885b Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Sun, 13 Nov 2022 17:28:55 +0100 Subject: [PATCH 06/14] ext2: remove ->writepage ->writepage is a very inefficient method to write back data, and only used through write_cache_pages or as a fallback when no ->migrate_folio method is present. Signed-off-by: Christoph Hellwig Signed-off-by: Jan Kara --- fs/ext2/inode.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c index 918ab2f9e4c0..3b2e3e1e0fa2 100644 --- a/fs/ext2/inode.c +++ b/fs/ext2/inode.c @@ -869,11 +869,6 @@ int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, return ret; } -static int ext2_writepage(struct page *page, struct writeback_control *wbc) -{ - return block_write_full_page(page, ext2_get_block, wbc); -} - static int ext2_read_folio(struct file *file, struct folio *folio) { return mpage_read_folio(folio, ext2_get_block); @@ -948,7 +943,6 @@ const struct address_space_operations ext2_aops = { .invalidate_folio = block_invalidate_folio, .read_folio = ext2_read_folio, .readahead = ext2_readahead, - .writepage = ext2_writepage, .write_begin = ext2_write_begin, .write_end = ext2_write_end, .bmap = ext2_bmap, From 36273e5b4e3a934c6d346c8f0b16b97e018094af Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Sun, 13 Nov 2022 17:29:02 +0100 Subject: [PATCH 07/14] udf: remove ->writepage ->writepage is a very inefficient method to write back data, and only used through write_cache_pages or as a fallback when no ->migrate_folio method is present. Set ->migrate_folio to the generic buffer_head based helper, and remove the ->writepage implementation in extfat. Signed-off-by: Christoph Hellwig Signed-off-by: Jan Kara --- fs/udf/inode.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/fs/udf/inode.c b/fs/udf/inode.c index dce6ae9ae306..0246b1b86fb9 100644 --- a/fs/udf/inode.c +++ b/fs/udf/inode.c @@ -182,11 +182,6 @@ static void udf_write_failed(struct address_space *mapping, loff_t to) } } -static int udf_writepage(struct page *page, struct writeback_control *wbc) -{ - return block_write_full_page(page, udf_get_block, wbc); -} - static int udf_writepages(struct address_space *mapping, struct writeback_control *wbc) { @@ -239,12 +234,12 @@ const struct address_space_operations udf_aops = { .invalidate_folio = block_invalidate_folio, .read_folio = udf_read_folio, .readahead = udf_readahead, - .writepage = udf_writepage, .writepages = udf_writepages, .write_begin = udf_write_begin, .write_end = generic_write_end, .direct_IO = udf_direct_IO, .bmap = udf_bmap, + .migrate_folio = buffer_migrate_folio, }; /* From 27e714c007e4ad01837bf0fac5c11913a38d7695 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 26 Nov 2022 03:17:17 +0000 Subject: [PATCH 08/14] ext2: unbugger ext2_empty_dir() In 27cfa258951a "ext2: fix fs corruption when trying to remove a non-empty directory with IO error" a funny thing has happened: - page = ext2_get_page(inode, i, dir_has_error, &page_addr); + page = ext2_get_page(inode, i, 0, &page_addr); - if (IS_ERR(page)) { - dir_has_error = 1; - continue; - } + if (IS_ERR(page)) + goto not_empty; And at not_empty: we hit ext2_put_page(page, page_addr), which does put_page(page). Which, unless I'm very mistaken, should oops immediately when given ERR_PTR(-E...) as page. OK, shit happens, insufficiently tested patches included. But when commit in question describes the fault-injection test that exercised that particular failure exit... Ow. CC: stable@vger.kernel.org Fixes: 27cfa258951a ("ext2: fix fs corruption when trying to remove a non-empty directory with IO error") Signed-off-by: Al Viro Signed-off-by: Jan Kara --- fs/ext2/dir.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c index 6fa714dbee84..e5cbc27ba459 100644 --- a/fs/ext2/dir.c +++ b/fs/ext2/dir.c @@ -684,7 +684,7 @@ int ext2_empty_dir (struct inode * inode) page = ext2_get_page(inode, i, 0, &page_addr); if (IS_ERR(page)) - goto not_empty; + return 0; kaddr = page_addr; de = (ext2_dirent *)kaddr; From b41b98e12a954c306e8eb9527ada0a70db339683 Mon Sep 17 00:00:00 2001 From: Rong Tao Date: Mon, 28 Nov 2022 20:05:49 +0800 Subject: [PATCH 09/14] fs/ext2: Fix code indentation ts=4 can cause misunderstanding in code reading. It is better to replace 8 spaces with one tab. Signed-off-by: Rong Tao Signed-off-by: Jan Kara --- fs/ext2/balloc.c | 12 ++++++------ fs/ext2/super.c | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c index 5dc0a31f4a08..eca60b747c6b 100644 --- a/fs/ext2/balloc.c +++ b/fs/ext2/balloc.c @@ -667,7 +667,7 @@ ext2_try_to_allocate(struct super_block *sb, int group, { ext2_fsblk_t group_first_block = ext2_group_first_block_no(sb, group); ext2_fsblk_t group_last_block = ext2_group_last_block_no(sb, group); - ext2_grpblk_t start, end; + ext2_grpblk_t start, end; unsigned long num = 0; start = 0; @@ -1481,11 +1481,11 @@ unsigned long ext2_count_free_blocks (struct super_block * sb) desc_count, bitmap_count); return bitmap_count; #else - for (i = 0; i < EXT2_SB(sb)->s_groups_count; i++) { - desc = ext2_get_group_desc (sb, i, NULL); - if (!desc) - continue; - desc_count += le16_to_cpu(desc->bg_free_blocks_count); + for (i = 0; i < EXT2_SB(sb)->s_groups_count; i++) { + desc = ext2_get_group_desc(sb, i, NULL); + if (!desc) + continue; + desc_count += le16_to_cpu(desc->bg_free_blocks_count); } return desc_count; #endif diff --git a/fs/ext2/super.c b/fs/ext2/super.c index 03f2af98b1b4..69c88facfe90 100644 --- a/fs/ext2/super.c +++ b/fs/ext2/super.c @@ -1648,7 +1648,7 @@ static int __init init_ext2_fs(void) err = init_inodecache(); if (err) return err; - err = register_filesystem(&ext2_fs_type); + err = register_filesystem(&ext2_fs_type); if (err) goto out; return 0; From 7868f93006ad27c00ecddcc2904118aa705459ca Mon Sep 17 00:00:00 2001 From: Bartosz Taudul Date: Sat, 3 Dec 2022 01:07:24 +0000 Subject: [PATCH 10/14] udf: Increase UDF_MAX_READ_VERSION to 0x0260 Some discs containing the UDF file system are unable to be mounted, failing with the following message: UDF-fs: error (device sr0): udf_fill_super: minUDFReadRev=260 (max is 250) The UDF 2.60 specification [0] states in the section Basic Restrictions & Requirements (page 10): The Minimum UDF Read Revision value shall be at most #0250 for all media with a UDF 2.60 file system. This indicates that a UDF 2.50 implementation can read all UDF 2.60 media. Media that do not have a Metadata Partition may use a value lower than #250. The conclusion is that the discs failing to mount were burned with a faulty software, which didn't follow the specification. This can be worked around by increasing UDF_MAX_READ_VERSION to 0x260, to match the Minimum Read Revision. No other changes are required, as reading UDF 2.60 is backward compatible with UDF 2.50. [0] http://www.osta.org/specs/pdf/udf260.pdf Signed-off-by: Bartosz Taudul Signed-off-by: Jan Kara --- fs/udf/udf_sb.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/fs/udf/udf_sb.h b/fs/udf/udf_sb.h index 4fa620543d30..291b56dd011e 100644 --- a/fs/udf/udf_sb.h +++ b/fs/udf/udf_sb.h @@ -6,7 +6,11 @@ #include #include -#define UDF_MAX_READ_VERSION 0x0250 +/* + * Even UDF 2.6 media should have version <= 0x250 but apparently there are + * some broken filesystems with version set to 0x260. Accommodate those. + */ +#define UDF_MAX_READ_VERSION 0x0260 #define UDF_MAX_WRITE_VERSION 0x0201 #define UDF_FLAG_USE_EXTENDED_FE 0 From cfe4c1b25dd6d2f056afc00b7c98bcb3dd0b1fc3 Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Wed, 7 Dec 2022 17:25:10 +0100 Subject: [PATCH 11/14] udf: Fix preallocation discarding at indirect extent boundary When preallocation extent is the first one in the extent block, the code would corrupt extent tree header instead. Fix the problem and use udf_delete_aext() for deleting extent to avoid some code duplication. CC: stable@vger.kernel.org Signed-off-by: Jan Kara --- fs/udf/truncate.c | 45 +++++++++++++-------------------------------- 1 file changed, 13 insertions(+), 32 deletions(-) diff --git a/fs/udf/truncate.c b/fs/udf/truncate.c index 532cda99644e..a9790fb32f5f 100644 --- a/fs/udf/truncate.c +++ b/fs/udf/truncate.c @@ -120,60 +120,41 @@ void udf_truncate_tail_extent(struct inode *inode) void udf_discard_prealloc(struct inode *inode) { - struct extent_position epos = { NULL, 0, {0, 0} }; + struct extent_position epos = {}; + struct extent_position prev_epos = {}; struct kernel_lb_addr eloc; uint32_t elen; uint64_t lbcount = 0; int8_t etype = -1, netype; - int adsize; struct udf_inode_info *iinfo = UDF_I(inode); if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB || inode->i_size == iinfo->i_lenExtents) return; - if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_SHORT) - adsize = sizeof(struct short_ad); - else if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_LONG) - adsize = sizeof(struct long_ad); - else - adsize = 0; - epos.block = iinfo->i_location; /* Find the last extent in the file */ - while ((netype = udf_next_aext(inode, &epos, &eloc, &elen, 1)) != -1) { - etype = netype; + while ((netype = udf_next_aext(inode, &epos, &eloc, &elen, 0)) != -1) { + brelse(prev_epos.bh); + prev_epos = epos; + if (prev_epos.bh) + get_bh(prev_epos.bh); + + etype = udf_next_aext(inode, &epos, &eloc, &elen, 1); lbcount += elen; } if (etype == (EXT_NOT_RECORDED_ALLOCATED >> 30)) { - epos.offset -= adsize; lbcount -= elen; - extent_trunc(inode, &epos, &eloc, etype, elen, 0); - if (!epos.bh) { - iinfo->i_lenAlloc = - epos.offset - - udf_file_entry_alloc_offset(inode); - mark_inode_dirty(inode); - } else { - struct allocExtDesc *aed = - (struct allocExtDesc *)(epos.bh->b_data); - aed->lengthAllocDescs = - cpu_to_le32(epos.offset - - sizeof(struct allocExtDesc)); - if (!UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_STRICT) || - UDF_SB(inode->i_sb)->s_udfrev >= 0x0201) - udf_update_tag(epos.bh->b_data, epos.offset); - else - udf_update_tag(epos.bh->b_data, - sizeof(struct allocExtDesc)); - mark_buffer_dirty_inode(epos.bh, inode); - } + udf_delete_aext(inode, prev_epos); + udf_free_blocks(inode->i_sb, inode, &eloc, 0, + DIV_ROUND_UP(elen, 1 << inode->i_blkbits)); } /* This inode entry is in-memory only and thus we don't have to mark * the inode dirty */ iinfo->i_lenExtents = lbcount; brelse(epos.bh); + brelse(prev_epos.bh); } static void udf_update_alloc_ext_desc(struct inode *inode, From 6ad53f0f71c52871202a7bf096feb2c59db33fc5 Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Wed, 7 Dec 2022 17:34:33 +0100 Subject: [PATCH 12/14] udf: Do not bother looking for prealloc extents if i_lenExtents matches i_size If rounded block-rounded i_lenExtents matches block rounded i_size, there are no preallocation extents. Do not bother walking extent linked list. CC: stable@vger.kernel.org Signed-off-by: Jan Kara --- fs/udf/truncate.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/udf/truncate.c b/fs/udf/truncate.c index a9790fb32f5f..036ebd892b85 100644 --- a/fs/udf/truncate.c +++ b/fs/udf/truncate.c @@ -127,9 +127,10 @@ void udf_discard_prealloc(struct inode *inode) uint64_t lbcount = 0; int8_t etype = -1, netype; struct udf_inode_info *iinfo = UDF_I(inode); + int bsize = 1 << inode->i_blkbits; if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB || - inode->i_size == iinfo->i_lenExtents) + ALIGN(inode->i_size, bsize) == ALIGN(iinfo->i_lenExtents, bsize)) return; epos.block = iinfo->i_location; From 16d0556568148bdcaa45d077cac9f8f7077cf70a Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Wed, 7 Dec 2022 18:17:34 +0100 Subject: [PATCH 13/14] udf: Discard preallocation before extending file with a hole When extending file with a hole, we tried to preserve existing preallocation for the file. However that is not very useful and complicates code because the previous extent may need to be rounded to block boundary as well (which we forgot to do thus causing data corruption for sequence like: xfs_io -f -c "pwrite 0x75e63 11008" -c "truncate 0x7b24b" \ -c "truncate 0xabaa3" -c "pwrite 0xac70b 22954" \ -c "pwrite 0x93a43 11358" -c "pwrite 0xb8e65 52211" file with 512-byte block size. Just discard preallocation before extending file to simplify things and also fix this data corruption. CC: stable@vger.kernel.org Signed-off-by: Jan Kara --- fs/udf/inode.c | 46 ++++++++++++++++++---------------------------- 1 file changed, 18 insertions(+), 28 deletions(-) diff --git a/fs/udf/inode.c b/fs/udf/inode.c index 0246b1b86fb9..44988e4c3fb2 100644 --- a/fs/udf/inode.c +++ b/fs/udf/inode.c @@ -434,6 +434,12 @@ static int udf_get_block(struct inode *inode, sector_t block, iinfo->i_next_alloc_goal++; } + /* + * Block beyond EOF and prealloc extents? Just discard preallocation + * as it is not useful and complicates things. + */ + if (((loff_t)block) << inode->i_blkbits > iinfo->i_lenExtents) + udf_discard_prealloc(inode); udf_clear_extent_cache(inode); phys = inode_getblk(inode, block, &err, &new); if (!phys) @@ -483,8 +489,6 @@ static int udf_do_extend_file(struct inode *inode, uint32_t add; int count = 0, fake = !(last_ext->extLength & UDF_EXTENT_LENGTH_MASK); struct super_block *sb = inode->i_sb; - struct kernel_lb_addr prealloc_loc = {}; - uint32_t prealloc_len = 0; struct udf_inode_info *iinfo; int err; @@ -505,19 +509,6 @@ static int udf_do_extend_file(struct inode *inode, ~(sb->s_blocksize - 1); } - /* Last extent are just preallocated blocks? */ - if ((last_ext->extLength & UDF_EXTENT_FLAG_MASK) == - EXT_NOT_RECORDED_ALLOCATED) { - /* Save the extent so that we can reattach it to the end */ - prealloc_loc = last_ext->extLocation; - prealloc_len = last_ext->extLength; - /* Mark the extent as a hole */ - last_ext->extLength = EXT_NOT_RECORDED_NOT_ALLOCATED | - (last_ext->extLength & UDF_EXTENT_LENGTH_MASK); - last_ext->extLocation.logicalBlockNum = 0; - last_ext->extLocation.partitionReferenceNum = 0; - } - /* Can we merge with the previous extent? */ if ((last_ext->extLength & UDF_EXTENT_FLAG_MASK) == EXT_NOT_RECORDED_NOT_ALLOCATED) { @@ -545,7 +536,7 @@ static int udf_do_extend_file(struct inode *inode, * more extents, we may need to enter possible following * empty indirect extent. */ - if (new_block_bytes || prealloc_len) + if (new_block_bytes) udf_next_aext(inode, last_pos, &tmploc, &tmplen, 0); } @@ -579,17 +570,6 @@ static int udf_do_extend_file(struct inode *inode, } out: - /* Do we have some preallocated blocks saved? */ - if (prealloc_len) { - err = udf_add_aext(inode, last_pos, &prealloc_loc, - prealloc_len, 1); - if (err) - return err; - last_ext->extLocation = prealloc_loc; - last_ext->extLength = prealloc_len; - count++; - } - /* last_pos should point to the last written extent... */ if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_SHORT) last_pos->offset -= sizeof(struct short_ad); @@ -642,8 +622,17 @@ static int udf_extend_file(struct inode *inode, loff_t newsize) else BUG(); + /* + * When creating hole in file, just don't bother with preserving + * preallocation. It likely won't be very useful anyway. + */ + udf_discard_prealloc(inode); + etype = inode_bmap(inode, first_block, &epos, &eloc, &elen, &offset); within_final_block = (etype != -1); + /* We don't expect extents past EOF... */ + WARN_ON_ONCE(etype != -1 && + elen > ((loff_t)offset + 1) << inode->i_blkbits); if ((!epos.bh && epos.offset == udf_file_entry_alloc_offset(inode)) || (epos.bh && epos.offset == sizeof(struct allocExtDesc))) { @@ -772,10 +761,11 @@ static sector_t inode_getblk(struct inode *inode, sector_t block, goto out_free; } - /* Are we beyond EOF? */ + /* Are we beyond EOF and preallocated extent? */ if (etype == -1) { int ret; loff_t hole_len; + isBeyondEOF = true; if (count) { if (c) From 1f3868f06855c97a4954c99b36f3fc9eb8f60326 Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Thu, 8 Dec 2022 13:03:30 +0100 Subject: [PATCH 14/14] udf: Fix extending file within last block When extending file within last block it can happen that the extent is already rounded to the blocksize and thus contains the offset we want to grow up to. In such case we would mistakenly expand the last extent and make it one block longer than it should be, exposing unallocated block in a file and causing data corruption. Fix the problem by properly detecting this case and bailing out. CC: stable@vger.kernel.org Signed-off-by: Jan Kara --- fs/udf/inode.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/fs/udf/inode.c b/fs/udf/inode.c index 44988e4c3fb2..1d7c2a812fc1 100644 --- a/fs/udf/inode.c +++ b/fs/udf/inode.c @@ -585,13 +585,17 @@ out: static void udf_do_extend_final_block(struct inode *inode, struct extent_position *last_pos, struct kernel_long_ad *last_ext, - uint32_t final_block_len) + uint32_t new_elen) { - struct super_block *sb = inode->i_sb; uint32_t added_bytes; - added_bytes = final_block_len - - (last_ext->extLength & (sb->s_blocksize - 1)); + /* + * Extent already large enough? It may be already rounded up to block + * size... + */ + if (new_elen <= (last_ext->extLength & UDF_EXTENT_LENGTH_MASK)) + return; + added_bytes = (last_ext->extLength & UDF_EXTENT_LENGTH_MASK) - new_elen; last_ext->extLength += added_bytes; UDF_I(inode)->i_lenExtents += added_bytes; @@ -608,12 +612,12 @@ static int udf_extend_file(struct inode *inode, loff_t newsize) int8_t etype; struct super_block *sb = inode->i_sb; sector_t first_block = newsize >> sb->s_blocksize_bits, offset; - unsigned long partial_final_block; + loff_t new_elen; int adsize; struct udf_inode_info *iinfo = UDF_I(inode); struct kernel_long_ad extent; int err = 0; - int within_final_block; + bool within_last_ext; if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_SHORT) adsize = sizeof(struct short_ad); @@ -629,9 +633,9 @@ static int udf_extend_file(struct inode *inode, loff_t newsize) udf_discard_prealloc(inode); etype = inode_bmap(inode, first_block, &epos, &eloc, &elen, &offset); - within_final_block = (etype != -1); + within_last_ext = (etype != -1); /* We don't expect extents past EOF... */ - WARN_ON_ONCE(etype != -1 && + WARN_ON_ONCE(within_last_ext && elen > ((loff_t)offset + 1) << inode->i_blkbits); if ((!epos.bh && epos.offset == udf_file_entry_alloc_offset(inode)) || @@ -648,19 +652,17 @@ static int udf_extend_file(struct inode *inode, loff_t newsize) extent.extLength |= etype << 30; } - partial_final_block = newsize & (sb->s_blocksize - 1); + new_elen = ((loff_t)offset << inode->i_blkbits) | + (newsize & (sb->s_blocksize - 1)); /* File has extent covering the new size (could happen when extending * inside a block)? */ - if (within_final_block) { + if (within_last_ext) { /* Extending file within the last file block */ - udf_do_extend_final_block(inode, &epos, &extent, - partial_final_block); + udf_do_extend_final_block(inode, &epos, &extent, new_elen); } else { - loff_t add = ((loff_t)offset << sb->s_blocksize_bits) | - partial_final_block; - err = udf_do_extend_file(inode, &epos, &extent, add); + err = udf_do_extend_file(inode, &epos, &extent, new_elen); } if (err < 0)