From aa6d148e6d6270274e3d5a529b71c54cd329d17f Mon Sep 17 00:00:00 2001 From: Li Hua Date: Mon, 21 Nov 2022 19:18:47 +0800 Subject: ubifs: Fix build errors as symbol undefined With CONFIG_UBIFS_FS_AUTHENTICATION not set, the compiler can assume that ubifs_node_check_hash() is never true and drops the call to ubifs_bad_hash(). Is CONFIG_CC_OPTIMIZE_FOR_SIZE enabled this optimization does not happen anymore. So When CONFIG_UBIFS_FS and CONFIG_CC_OPTIMIZE_FOR_SIZE is enabled but CONFIG_UBIFS_FS_AUTHENTICATION is not set, the build errors is as followd: ERROR: modpost: "ubifs_bad_hash" [fs/ubifs/ubifs.ko] undefined! Fix it by add no-op ubifs_bad_hash() for the CONFIG_UBIFS_FS_AUTHENTICATION=n case. Fixes: 16a26b20d2af ("ubifs: authentication: Add hashes to index nodes") Signed-off-by: Li Hua Reviewed-by: Sascha Hauer Signed-off-by: Richard Weinberger --- fs/ubifs/ubifs.h | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'fs/ubifs') diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h index 478bbbb5382f..2f1f31581094 100644 --- a/fs/ubifs/ubifs.h +++ b/fs/ubifs/ubifs.h @@ -1623,8 +1623,13 @@ static inline int ubifs_check_hmac(const struct ubifs_info *c, return crypto_memneq(expected, got, c->hmac_desc_len); } +#ifdef CONFIG_UBIFS_FS_AUTHENTICATION void ubifs_bad_hash(const struct ubifs_info *c, const void *node, const u8 *hash, int lnum, int offs); +#else +static inline void ubifs_bad_hash(const struct ubifs_info *c, const void *node, + const u8 *hash, int lnum, int offs) {}; +#endif int __ubifs_node_check_hash(const struct ubifs_info *c, const void *buf, const u8 *expected); -- cgit From 203a55f04f66eea1a1ca7e5a302a7f5c99c62327 Mon Sep 17 00:00:00 2001 From: Liu Shixin Date: Thu, 20 Oct 2022 12:30:31 +0800 Subject: ubifs: Fix memory leak in ubifs_sysfs_init() When insmod ubifs.ko, a kmemleak reported as below: unreferenced object 0xffff88817fb1a780 (size 8): comm "insmod", pid 25265, jiffies 4295239702 (age 100.130s) hex dump (first 8 bytes): 75 62 69 66 73 00 ff ff ubifs... backtrace: [] slab_post_alloc_hook+0x9c/0x3c0 [] __kmalloc_track_caller+0x183/0x410 [] kstrdup+0x3a/0x80 [] kstrdup_const+0x66/0x80 [] kvasprintf_const+0x155/0x190 [] kobject_set_name_vargs+0x5b/0x150 [] kobject_set_name+0xbb/0xf0 [] do_one_initcall+0x14c/0x5a0 [] do_init_module+0x1f0/0x660 [] load_module+0x6d7e/0x7590 [] __do_sys_finit_module+0x19f/0x230 [] __x64_sys_finit_module+0x73/0xb0 [] do_syscall_64+0x35/0x80 [] entry_SYSCALL_64_after_hwframe+0x63/0xcd When kset_register() failed, we should call kset_put to cleanup it. Fixes: 2e3cbf425804 ("ubifs: Export filesystem error counters") Signed-off-by: Liu Shixin Signed-off-by: Richard Weinberger --- fs/ubifs/sysfs.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'fs/ubifs') diff --git a/fs/ubifs/sysfs.c b/fs/ubifs/sysfs.c index 06ad8fa1fcfb..54270ad36321 100644 --- a/fs/ubifs/sysfs.c +++ b/fs/ubifs/sysfs.c @@ -144,6 +144,8 @@ int __init ubifs_sysfs_init(void) kobject_set_name(&ubifs_kset.kobj, "ubifs"); ubifs_kset.kobj.parent = fs_kobj; ret = kset_register(&ubifs_kset); + if (ret) + kset_put(&ubifs_kset); return ret; } -- cgit From c2c36cc6ca23e614f9e4238d0ecf48549ee9002a Mon Sep 17 00:00:00 2001 From: Zhihao Cheng Date: Tue, 11 Oct 2022 11:47:27 +0800 Subject: ubifs: Rectify space budget for ubifs_symlink() if symlink is encrypted Fix bad space budget when symlink file is encrypted. Bad space budget may let make_reservation() return with -ENOSPC, which could turn ubifs to read-only mode in do_writepage() process. Fetch a reproducer in [Link]. Link: https://bugzilla.kernel.org/show_bug.cgi?id=216490 Fixes: ca7f85be8d6cf9 ("ubifs: Add support for encrypted symlinks") Signed-off-by: Zhihao Cheng Signed-off-by: Richard Weinberger --- fs/ubifs/dir.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/ubifs') diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c index 0f29cf201136..aaff3f3f0aa3 100644 --- a/fs/ubifs/dir.c +++ b/fs/ubifs/dir.c @@ -1151,7 +1151,6 @@ static int ubifs_symlink(struct user_namespace *mnt_userns, struct inode *dir, int err, sz_change, len = strlen(symname); struct fscrypt_str disk_link; struct ubifs_budget_req req = { .new_ino = 1, .new_dent = 1, - .new_ino_d = ALIGN(len, 8), .dirtied_ino = 1 }; struct fscrypt_name nm; @@ -1167,6 +1166,7 @@ static int ubifs_symlink(struct user_namespace *mnt_userns, struct inode *dir, * Budget request settings: new inode, new direntry and changing parent * directory inode. */ + req.new_ino_d = ALIGN(disk_link.len - 1, 8); err = ubifs_budget_space(c, &req); if (err) return err; -- cgit From 1b2ba09060e41adb356b9ae58ef94a7390928004 Mon Sep 17 00:00:00 2001 From: Zhihao Cheng Date: Tue, 11 Oct 2022 11:47:28 +0800 Subject: ubifs: Rectify space budget for ubifs_xrename() There is no space budget for ubifs_xrename(). It may let make_reservation() return with -ENOSPC, which could turn ubifs to read-only mode in do_writepage() process. Fix it by adding space budget for ubifs_xrename(). Fetch a reproducer in [Link]. Link: https://bugzilla.kernel.org/show_bug.cgi?id=216569 Fixes: 9ec64962afb170 ("ubifs: Implement RENAME_EXCHANGE") Signed-off-by: Zhihao Cheng Signed-off-by: Richard Weinberger --- fs/ubifs/dir.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'fs/ubifs') diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c index aaff3f3f0aa3..94634b872e38 100644 --- a/fs/ubifs/dir.c +++ b/fs/ubifs/dir.c @@ -1576,6 +1576,10 @@ static int ubifs_xrename(struct inode *old_dir, struct dentry *old_dentry, return err; } + err = ubifs_budget_space(c, &req); + if (err) + goto out; + lock_4_inodes(old_dir, new_dir, NULL, NULL); time = current_time(old_dir); @@ -1601,6 +1605,7 @@ static int ubifs_xrename(struct inode *old_dir, struct dentry *old_dentry, unlock_4_inodes(old_dir, new_dir, NULL, NULL); ubifs_release_budget(c, &req); +out: fscrypt_free_filename(&fst_nm); fscrypt_free_filename(&snd_nm); return err; -- cgit From c04cc68da856eac2b59e2a421c3f81a2afc560db Mon Sep 17 00:00:00 2001 From: Zhihao Cheng Date: Tue, 11 Oct 2022 11:47:29 +0800 Subject: ubifs: Add comments and debug info for ubifs_xrename() Just like other operations (eg. ubifs_create, do_rename), add comments and debug information for ubifs_xrename(). Signed-off-by: Zhihao Cheng Signed-off-by: Richard Weinberger --- fs/ubifs/dir.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'fs/ubifs') diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c index 94634b872e38..7a1dcd14e387 100644 --- a/fs/ubifs/dir.c +++ b/fs/ubifs/dir.c @@ -1566,6 +1566,15 @@ static int ubifs_xrename(struct inode *old_dir, struct dentry *old_dentry, ubifs_assert(c, fst_inode && snd_inode); + /* + * Budget request settings: changing two direntries, changing the two + * parent directory inodes. + */ + + dbg_gen("dent '%pd' ino %lu in dir ino %lu exchange dent '%pd' ino %lu in dir ino %lu", + old_dentry, fst_inode->i_ino, old_dir->i_ino, + new_dentry, snd_inode->i_ino, new_dir->i_ino); + err = fscrypt_setup_filename(old_dir, &old_dentry->d_name, 0, &fst_nm); if (err) return err; -- cgit From b248eaf049d9cdc5eb76b59399e4d3de233f02ac Mon Sep 17 00:00:00 2001 From: Zhihao Cheng Date: Tue, 11 Oct 2022 11:47:30 +0800 Subject: ubifs: Fix wrong dirty space budget for dirty inode Each dirty inode should reserve 'c->bi.inode_budget' bytes in space budget calculation. Currently, space budget for dirty inode reports more space than what UBIFS actually needs to write. Fixes: 1e51764a3c2ac0 ("UBIFS: add new flash file system") Signed-off-by: Zhihao Cheng Signed-off-by: Richard Weinberger --- fs/ubifs/budget.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/ubifs') diff --git a/fs/ubifs/budget.c b/fs/ubifs/budget.c index e8b9b756f0ac..986e6e4081c7 100644 --- a/fs/ubifs/budget.c +++ b/fs/ubifs/budget.c @@ -400,7 +400,7 @@ static int calc_dd_growth(const struct ubifs_info *c, dd_growth = req->dirtied_page ? c->bi.page_budget : 0; if (req->dirtied_ino) - dd_growth += c->bi.inode_budget << (req->dirtied_ino - 1); + dd_growth += c->bi.inode_budget * req->dirtied_ino; if (req->mod_dent) dd_growth += c->bi.dent_budget; dd_growth += req->dirtied_ino_d; -- cgit From 25fce616a61fc2f1821e4a9ce212d0e064707093 Mon Sep 17 00:00:00 2001 From: Zhihao Cheng Date: Tue, 11 Oct 2022 11:47:31 +0800 Subject: ubifs: do_rename: Fix wrong space budget when target inode's nlink > 1 If target inode is a special file (eg. block/char device) with nlink count greater than 1, the inode with ui->data will be re-written on disk. However, UBIFS losts target inode's data_len while doing space budget. Bad space budget may let make_reservation() return with -ENOSPC, which could turn ubifs to read-only mode in do_writepage() process. Fetch a reproducer in [Link]. Link: https://bugzilla.kernel.org/show_bug.cgi?id=216494 Fixes: 1e51764a3c2ac0 ("UBIFS: add new flash file system") Signed-off-by: Zhihao Cheng Signed-off-by: Richard Weinberger --- fs/ubifs/dir.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'fs/ubifs') diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c index 7a1dcd14e387..4509d9fa9e6e 100644 --- a/fs/ubifs/dir.c +++ b/fs/ubifs/dir.c @@ -1324,6 +1324,8 @@ static int do_rename(struct inode *old_dir, struct dentry *old_dentry, if (unlink) { ubifs_assert(c, inode_is_locked(new_inode)); + /* Budget for old inode's data when its nlink > 1. */ + req.dirtied_ino_d = ALIGN(ubifs_inode(new_inode)->data_len, 8); err = ubifs_purge_xattrs(new_inode); if (err) return err; -- cgit From e874dcde1cbf82c786c0e7f2899811c02630cc52 Mon Sep 17 00:00:00 2001 From: Zhihao Cheng Date: Tue, 11 Oct 2022 11:47:32 +0800 Subject: ubifs: Reserve one leb for each journal head while doing budget UBIFS calculates available space by c->main_bytes - c->lst.total_used (which means non-index lebs' free and dirty space is accounted into total available), then index lebs and four lebs (one for gc_lnum, one for deletions, two for journal heads) are deducted. In following situation, ubifs may get -ENOSPC from make_reservation(): LEB 84: DATAHD free 122880 used 1920 dirty 2176 dark 6144 LEB 110:DELETION free 126976 used 0 dirty 0 dark 6144 (empty) LEB 201:gc_lnum free 126976 used 0 dirty 0 dark 6144 LEB 272:GCHD free 77824 used 47672 dirty 1480 dark 6144 LEB 356:BASEHD free 0 used 39776 dirty 87200 dark 6144 OTHERS: index lebs, zero-available non-index lebs UBIFS calculates the available bytes is 6888 (How to calculate it: 126976 * 5[remain main bytes] - 1920[used] - 47672[used] - 39776[used] - 126976 * 1[deletions] - 126976 * 1[gc_lnum] - 126976 * 2[journal heads] - 6144 * 5[dark] = 6888) after doing budget, however UBIFS cannot use BASEHD's dirty space(87200), because UBIFS cannot find next BASEHD to reclaim current BASEHD. (c->bi.min_idx_lebs equals to c->lst.idx_lebs, the empty leb won't be found by ubifs_find_free_space(), and dirty index lebs won't be picked as gced lebs. All non-index lebs has dirty space less then c->dead_wm, non-index lebs won't be picked as gced lebs either. So new free lebs won't be produced.). See more details in Link. To fix it, reserve one leb for each journal head while doing budget. Link: https://bugzilla.kernel.org/show_bug.cgi?id=216562 Fixes: 1e51764a3c2ac0 ("UBIFS: add new flash file system") Signed-off-by: Zhihao Cheng Signed-off-by: Richard Weinberger --- fs/ubifs/budget.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'fs/ubifs') diff --git a/fs/ubifs/budget.c b/fs/ubifs/budget.c index 986e6e4081c7..d76eb7b39f56 100644 --- a/fs/ubifs/budget.c +++ b/fs/ubifs/budget.c @@ -209,11 +209,10 @@ long long ubifs_calc_available(const struct ubifs_info *c, int min_idx_lebs) subtract_lebs += 1; /* - * The GC journal head LEB is not really accessible. And since - * different write types go to different heads, we may count only on - * one head's space. + * Since different write types go to different heads, we should + * reserve one leb for each head. */ - subtract_lebs += c->jhead_cnt - 1; + subtract_lebs += c->jhead_cnt; /* We also reserve one LEB for deletions, which bypass budgeting */ subtract_lebs += 1; -- cgit From 4a1ff3c5d04b9079b4f768d9a71b51c4af578dd2 Mon Sep 17 00:00:00 2001 From: Li Zetao Date: Sat, 22 Oct 2022 19:52:11 +0800 Subject: ubifs: Fix memory leak in alloc_wbufs() kmemleak reported a sequence of memory leaks, and show them as following: unreferenced object 0xffff8881575f8400 (size 1024): comm "mount", pid 19625, jiffies 4297119604 (age 20.383s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ backtrace: [] __kmalloc+0x4d/0x150 [] ubifs_mount+0x307b/0x7170 [ubifs] [] legacy_get_tree+0xed/0x1d0 [] vfs_get_tree+0x7d/0x230 [] path_mount+0xdd4/0x17b0 [] __x64_sys_mount+0x1fa/0x270 [] do_syscall_64+0x35/0x80 [] entry_SYSCALL_64_after_hwframe+0x46/0xb0 unreferenced object 0xffff8881798a6e00 (size 512): comm "mount", pid 19677, jiffies 4297121912 (age 37.816s) hex dump (first 32 bytes): 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk backtrace: [] __kmalloc+0x4d/0x150 [] ubifs_wbuf_init+0x52/0x480 [ubifs] [] ubifs_mount+0x31f5/0x7170 [ubifs] [] legacy_get_tree+0xed/0x1d0 [] vfs_get_tree+0x7d/0x230 [] path_mount+0xdd4/0x17b0 [] __x64_sys_mount+0x1fa/0x270 [] do_syscall_64+0x35/0x80 [] entry_SYSCALL_64_after_hwframe+0x46/0xb0 The problem is that the ubifs_wbuf_init() returns an error in the loop which in the alloc_wbufs(), then the wbuf->buf and wbuf->inodes that were successfully alloced before are not freed. Fix it by adding error hanging path in alloc_wbufs() which frees the memory alloced before when ubifs_wbuf_init() returns an error. Fixes: 1e51764a3c2a ("UBIFS: add new flash file system") Signed-off-by: Li Zetao Reviewed-by: Zhihao Cheng Signed-off-by: Richard Weinberger --- fs/ubifs/super.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) (limited to 'fs/ubifs') diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index d0c9a09988bc..32cb14759796 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -833,7 +833,7 @@ static int alloc_wbufs(struct ubifs_info *c) INIT_LIST_HEAD(&c->jheads[i].buds_list); err = ubifs_wbuf_init(c, &c->jheads[i].wbuf); if (err) - return err; + goto out_wbuf; c->jheads[i].wbuf.sync_callback = &bud_wbuf_callback; c->jheads[i].wbuf.jhead = i; @@ -841,7 +841,7 @@ static int alloc_wbufs(struct ubifs_info *c) c->jheads[i].log_hash = ubifs_hash_get_desc(c); if (IS_ERR(c->jheads[i].log_hash)) { err = PTR_ERR(c->jheads[i].log_hash); - goto out; + goto out_log_hash; } } @@ -854,9 +854,18 @@ static int alloc_wbufs(struct ubifs_info *c) return 0; -out: - while (i--) +out_log_hash: + kfree(c->jheads[i].wbuf.buf); + kfree(c->jheads[i].wbuf.inodes); + +out_wbuf: + while (i--) { + kfree(c->jheads[i].wbuf.buf); + kfree(c->jheads[i].wbuf.inodes); kfree(c->jheads[i].log_hash); + } + kfree(c->jheads); + c->jheads = NULL; return err; } -- cgit From 944e096aa24071d3fe22822f6249d3ae309e39ea Mon Sep 17 00:00:00 2001 From: Zhihao Cheng Date: Fri, 18 Nov 2022 17:02:35 +0800 Subject: ubifs: Re-statistic cleaned znode count if commit failed Dirty znodes will be written on flash in committing process with following states: process A | znode state ------------------------------------------------------ do_commit | DIRTY_ZNODE ubifs_tnc_start_commit | DIRTY_ZNODE get_znodes_to_commit | DIRTY_ZNODE | COW_ZNODE layout_commit | DIRTY_ZNODE | COW_ZNODE fill_gap | 0 write master | 0 or OBSOLETE_ZNODE process B | znode state ------------------------------------------------------ do_commit | DIRTY_ZNODE[1] ubifs_tnc_start_commit | DIRTY_ZNODE get_znodes_to_commit | DIRTY_ZNODE | COW_ZNODE ubifs_tnc_end_commit | DIRTY_ZNODE | COW_ZNODE write_index | 0 write master | 0 or OBSOLETE_ZNODE[2] or | DIRTY_ZNODE[3] [1] znode is dirtied without concurrent committing process [2] znode is copied up (re-dirtied by other process) before cleaned up in committing process [3] znode is re-dirtied after cleaned up in committing process Currently, the clean znode count is updated in free_obsolete_znodes(), which is called only in normal path. If do_commit failed, clean znode count won't be updated, which triggers a failure ubifs assertion[4] in ubifs_tnc_close(): ubifs_assert_failed [ubifs]: UBIFS assert failed: freed == n [4] Commit 380347e9ca7682 ("UBIFS: Add an assertion for clean_zn_cnt"). Fix it by re-statisticing cleaned znode count in tnc_destroy_cnext(). Fetch a reproducer in [Link]. Link: https://bugzilla.kernel.org/show_bug.cgi?id=216704 Fixes: 1e51764a3c2a ("UBIFS: add new flash file system") Signed-off-by: Zhihao Cheng Signed-off-by: Richard Weinberger --- fs/ubifs/tnc.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'fs/ubifs') diff --git a/fs/ubifs/tnc.c b/fs/ubifs/tnc.c index 488f3da7a6c6..2df56bbc6865 100644 --- a/fs/ubifs/tnc.c +++ b/fs/ubifs/tnc.c @@ -3053,6 +3053,21 @@ static void tnc_destroy_cnext(struct ubifs_info *c) cnext = cnext->cnext; if (ubifs_zn_obsolete(znode)) kfree(znode); + else if (!ubifs_zn_cow(znode)) { + /* + * Don't forget to update clean znode count after + * committing failed, because ubifs will check this + * count while closing tnc. Non-obsolete znode could + * be re-dirtied during committing process, so dirty + * flag is untrustable. The flag 'COW_ZNODE' is set + * for each dirty znode before committing, and it is + * cleared as long as the znode become clean, so we + * can statistic clean znode count according to this + * flag. + */ + atomic_long_inc(&c->clean_zn_cnt); + atomic_long_inc(&ubifs_clean_zn_cnt); + } } while (cnext && cnext != c->cnext); } -- cgit From 122deabfe1428bffe95e2bf364ff8a5059bdf089 Mon Sep 17 00:00:00 2001 From: Zhihao Cheng Date: Fri, 18 Nov 2022 17:02:36 +0800 Subject: ubifs: dirty_cow_znode: Fix memleak in error handling path Following process will cause a memleak for copied up znode: dirty_cow_znode zn = copy_znode(c, znode); err = insert_old_idx(c, zbr->lnum, zbr->offs); if (unlikely(err)) return ERR_PTR(err); // No one refers to zn. Fix it by adding copied znode back to tnc, then it will be freed by ubifs_destroy_tnc_subtree() while closing tnc. Fetch a reproducer in [Link]. Link: https://bugzilla.kernel.org/show_bug.cgi?id=216705 Fixes: 1e51764a3c2a ("UBIFS: add new flash file system") Signed-off-by: Zhihao Cheng Signed-off-by: Richard Weinberger --- fs/ubifs/tnc.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'fs/ubifs') diff --git a/fs/ubifs/tnc.c b/fs/ubifs/tnc.c index 2df56bbc6865..2469f72eeaab 100644 --- a/fs/ubifs/tnc.c +++ b/fs/ubifs/tnc.c @@ -267,11 +267,18 @@ static struct ubifs_znode *dirty_cow_znode(struct ubifs_info *c, if (zbr->len) { err = insert_old_idx(c, zbr->lnum, zbr->offs); if (unlikely(err)) - return ERR_PTR(err); + /* + * Obsolete znodes will be freed by tnc_destroy_cnext() + * or free_obsolete_znodes(), copied up znodes should + * be added back to tnc and freed by + * ubifs_destroy_tnc_subtree(). + */ + goto out; err = add_idx_dirt(c, zbr->lnum, zbr->len); } else err = 0; +out: zbr->znode = zn; zbr->lnum = 0; zbr->offs = 0; -- cgit From fb8bc4c74ae4526d9489362ab2793a936d072b84 Mon Sep 17 00:00:00 2001 From: Zhihao Cheng Date: Wed, 1 Jun 2022 10:59:59 +0800 Subject: ubifs: ubifs_writepage: Mark page dirty after writing inode failed There are two states for ubifs writing pages: 1. Dirty, Private 2. Not Dirty, Not Private There is a third possibility which maybe related to [1] that page is private but not dirty caused by following process: PA lock(page) ubifs_write_end attach_page_private // set Private __set_page_dirty_nobuffers // set Dirty unlock(page) write_cache_pages lock(page) clear_page_dirty_for_io(page) // clear Dirty ubifs_writepage write_inode // fail, goto out, following codes are not executed // do_writepage // set_page_writeback // set Writeback // detach_page_private // clear Private // end_page_writeback // clear Writeback out: unlock(page) // Private, Not Dirty PB ksys_fadvise64_64 generic_fadvise invalidate_inode_page // page is neither Dirty nor Writeback invalidate_complete_page // page_has_private is true try_to_release_page ubifs_releasepage ubifs_assert(c, 0) !!! Then we may get following assertion failed: UBIFS error (ubi0:0 pid 1492): ubifs_assert_failed [ubifs]: UBIFS assert failed: 0, in fs/ubifs/file.c:1499 UBIFS warning (ubi0:0 pid 1492): ubifs_ro_mode [ubifs]: switched to read-only mode, error -22 CPU: 2 PID: 1492 Comm: aa Not tainted 5.16.0-rc2-00012-g7bb767dee0ba-dirty Call Trace: dump_stack+0x13/0x1b ubifs_ro_mode+0x54/0x60 [ubifs] ubifs_assert_failed+0x4b/0x80 [ubifs] ubifs_releasepage+0x7e/0x1e0 [ubifs] try_to_release_page+0x57/0xe0 invalidate_inode_page+0xfb/0x130 invalidate_mapping_pagevec+0x12/0x20 generic_fadvise+0x303/0x3c0 vfs_fadvise+0x35/0x40 ksys_fadvise64_64+0x4c/0xb0 Jump [2] to find a reproducer. [1] https://linux-mtd.infradead.narkive.com/NQoBeT1u/patch-rfc-ubifs-fix-assert-failed-in-ubifs-set-page-dirty [2] https://bugzilla.kernel.org/show_bug.cgi?id=215357 Fixes: 1e51764a3c2ac0 ("UBIFS: add new flash file system") Signed-off-by: Zhihao Cheng Signed-off-by: Richard Weinberger --- fs/ubifs/file.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'fs/ubifs') diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c index f2353dd676ef..1f429260a85f 100644 --- a/fs/ubifs/file.c +++ b/fs/ubifs/file.c @@ -1032,7 +1032,7 @@ static int ubifs_writepage(struct page *page, struct writeback_control *wbc) if (page->index >= synced_i_size >> PAGE_SHIFT) { err = inode->i_sb->s_op->write_inode(inode, NULL); if (err) - goto out_unlock; + goto out_redirty; /* * The inode has been written, but the write-buffer has * not been synchronized, so in case of an unclean @@ -1060,11 +1060,17 @@ static int ubifs_writepage(struct page *page, struct writeback_control *wbc) if (i_size > synced_i_size) { err = inode->i_sb->s_op->write_inode(inode, NULL); if (err) - goto out_unlock; + goto out_redirty; } return do_writepage(page, len); - +out_redirty: + /* + * redirty_page_for_writepage() won't call ubifs_dirty_inode() because + * it passes I_DIRTY_PAGES flag while calling __mark_inode_dirty(), so + * there is no need to do space budget for dirty inode. + */ + redirty_page_for_writepage(wbc, page); out_unlock: unlock_page(page); return err; -- cgit From 66f4742e93523ab2f062d9d9828b3e590bc61536 Mon Sep 17 00:00:00 2001 From: Zhihao Cheng Date: Wed, 1 Jun 2022 11:00:00 +0800 Subject: ubifs: ubifs_releasepage: Remove ubifs_assert(0) to valid this process There are two states for ubifs writing pages: 1. Dirty, Private 2. Not Dirty, Not Private The normal process cannot go to ubifs_releasepage() which means there exists pages being private but not dirty. Reproducer[1] shows that it could occur (which maybe related to [2]) with following process: PA PB PC lock(page)[PA] ubifs_write_end attach_page_private // set Private __set_page_dirty_nobuffers // set Dirty unlock(page) write_cache_pages[PA] lock(page) clear_page_dirty_for_io(page) // clear Dirty ubifs_writepage do_truncation[PB] truncate_setsize i_size_write(inode, newsize) // newsize = 0 i_size = i_size_read(inode) // i_size = 0 end_index = i_size >> PAGE_SHIFT if (page->index > end_index) goto out // jump out: unlock(page) // Private, Not Dirty generic_fadvise[PC] lock(page) invalidate_inode_page try_to_release_page ubifs_releasepage ubifs_assert(c, 0) // bad assertion! unlock(page) truncate_pagecache[PB] Then we may get following assertion failed: UBIFS error (ubi0:0 pid 1683): ubifs_assert_failed [ubifs]: UBIFS assert failed: 0, in fs/ubifs/file.c:1513 UBIFS warning (ubi0:0 pid 1683): ubifs_ro_mode [ubifs]: switched to read-only mode, error -22 CPU: 2 PID: 1683 Comm: aa Not tainted 5.16.0-rc5-00184-g0bca5994cacc-dirty #308 Call Trace: dump_stack+0x13/0x1b ubifs_ro_mode+0x54/0x60 [ubifs] ubifs_assert_failed+0x4b/0x80 [ubifs] ubifs_releasepage+0x67/0x1d0 [ubifs] try_to_release_page+0x57/0xe0 invalidate_inode_page+0xfb/0x130 __invalidate_mapping_pages+0xb9/0x280 invalidate_mapping_pagevec+0x12/0x20 generic_fadvise+0x303/0x3c0 ksys_fadvise64_64+0x4c/0xb0 [1] https://bugzilla.kernel.org/show_bug.cgi?id=215373 [2] https://linux-mtd.infradead.narkive.com/NQoBeT1u/patch-rfc-ubifs-fix-assert-failed-in-ubifs-set-page-dirty Fixes: 1e51764a3c2ac0 ("UBIFS: add new flash file system") Signed-off-by: Zhihao Cheng Signed-off-by: Richard Weinberger --- fs/ubifs/file.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) (limited to 'fs/ubifs') diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c index 1f429260a85f..10c1779af9c5 100644 --- a/fs/ubifs/file.c +++ b/fs/ubifs/file.c @@ -1472,14 +1472,23 @@ static bool ubifs_release_folio(struct folio *folio, gfp_t unused_gfp_flags) struct inode *inode = folio->mapping->host; struct ubifs_info *c = inode->i_sb->s_fs_info; - /* - * An attempt to release a dirty page without budgeting for it - should - * not happen. - */ if (folio_test_writeback(folio)) return false; + + /* + * Page is private but not dirty, weird? There is one condition + * making it happened. ubifs_writepage skipped the page because + * page index beyonds isize (for example. truncated by other + * process named A), then the page is invalidated by fadvise64 + * syscall before being truncated by process A. + */ ubifs_assert(c, folio_test_private(folio)); - ubifs_assert(c, 0); + if (folio_test_checked(folio)) + release_new_page_budget(c); + else + release_existing_page_budget(c); + + atomic_long_dec(&c->dirty_pg_cnt); folio_detach_private(folio); folio_clear_checked(folio); return true; -- cgit From 415c94532ebb2838dc9e2e5a5e609a6140caa5c5 Mon Sep 17 00:00:00 2001 From: Yang Li Date: Thu, 2 Jun 2022 14:55:56 +0800 Subject: ubifs: Fix some kernel-doc comments Remove warnings found by running scripts/kernel-doc, which is caused by using 'make W=1'. fs/ubifs/journal.c:1221: warning: Function parameter or member 'old_inode' not described in 'ubifs_jnl_rename' fs/ubifs/journal.c:1221: warning: Function parameter or member 'old_nm' not described in 'ubifs_jnl_rename' fs/ubifs/journal.c:1221: warning: Function parameter or member 'new_inode' not described in 'ubifs_jnl_rename' fs/ubifs/journal.c:1221: warning: Function parameter or member 'new_nm' not described in 'ubifs_jnl_rename' fs/ubifs/journal.c:1221: warning: Function parameter or member 'whiteout' not described in 'ubifs_jnl_rename' fs/ubifs/journal.c:1221: warning: Excess function parameter 'old_dentry' description in 'ubifs_jnl_rename' fs/ubifs/journal.c:1221: warning: Excess function parameter 'new_dentry' description in 'ubifs_jnl_rename' Reported-by: Abaci Robot Signed-off-by: Yang Li Reviewed-by: Zhihao Cheng Signed-off-by: Richard Weinberger --- fs/ubifs/journal.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'fs/ubifs') diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c index d02509920baf..dc52ac0f4a34 100644 --- a/fs/ubifs/journal.c +++ b/fs/ubifs/journal.c @@ -1201,9 +1201,13 @@ out_free: * ubifs_jnl_rename - rename a directory entry. * @c: UBIFS file-system description object * @old_dir: parent inode of directory entry to rename - * @old_dentry: directory entry to rename + * @old_inode: directory entry's inode to rename + * @old_nm: name of the old directory entry to rename * @new_dir: parent inode of directory entry to rename - * @new_dentry: new directory entry (or directory entry to replace) + * @new_inode: new directory entry's inode (or directory entry's inode to + * replace) + * @new_nm: new name of the new directory entry + * @whiteout: whiteout inode * @sync: non-zero if the write-buffer has to be synchronized * * This function implements the re-name operation which may involve writing up -- cgit From 422125232f6200f0490b7d7fb97722723eb8876d Mon Sep 17 00:00:00 2001 From: Yang Li Date: Wed, 17 Nov 2021 15:03:04 +0800 Subject: ubifs: Fix kernel-doc Fix function name in fs/ubifs/io.c kernel-doc comment to remove some warnings found by clang(make W=1 LLVM=1). fs/ubifs/io.c:497: warning: expecting prototype for wbuf_timer_callback(). Prototype was for wbuf_timer_callback_nolock() instead fs/ubifs/io.c:513: warning: expecting prototype for new_wbuf_timer(). Prototype was for new_wbuf_timer_nolock() instead fs/ubifs/io.c:538: warning: expecting prototype for cancel_wbuf_timer(). Prototype was for cancel_wbuf_timer_nolock() instead Reported-by: Abaci Robot Signed-off-by: Yang Li Signed-off-by: Richard Weinberger --- fs/ubifs/io.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'fs/ubifs') diff --git a/fs/ubifs/io.c b/fs/ubifs/io.c index 1607a3c76681..01d8eb170382 100644 --- a/fs/ubifs/io.c +++ b/fs/ubifs/io.c @@ -488,7 +488,7 @@ void ubifs_prep_grp_node(struct ubifs_info *c, void *node, int len, int last) } /** - * wbuf_timer_callback - write-buffer timer callback function. + * wbuf_timer_callback_nolock - write-buffer timer callback function. * @timer: timer data (write-buffer descriptor) * * This function is called when the write-buffer timer expires. @@ -505,7 +505,7 @@ static enum hrtimer_restart wbuf_timer_callback_nolock(struct hrtimer *timer) } /** - * new_wbuf_timer - start new write-buffer timer. + * new_wbuf_timer_nolock - start new write-buffer timer. * @c: UBIFS file-system description object * @wbuf: write-buffer descriptor */ @@ -531,7 +531,7 @@ static void new_wbuf_timer_nolock(struct ubifs_info *c, struct ubifs_wbuf *wbuf) } /** - * cancel_wbuf_timer - cancel write-buffer timer. + * cancel_wbuf_timer_nolock - cancel write-buffer timer. * @wbuf: write-buffer descriptor */ static void cancel_wbuf_timer_nolock(struct ubifs_wbuf *wbuf) -- cgit From 22d74bc26bbfde150f602b2a6e93fb11df30ce0b Mon Sep 17 00:00:00 2001 From: Thomas Weißschuh Date: Fri, 10 Feb 2023 02:16:13 +0000 Subject: ubifs: make kobj_type structures constant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since commit ee6d3dd4ed48 ("driver core: make kobj_type constant.") the driver core allows the usage of const struct kobj_type. Take advantage of this to constify the structure definitions to prevent modification at runtime. Signed-off-by: Thomas Weißschuh Reviewed-by: Zhihao Cheng Signed-off-by: Richard Weinberger --- fs/ubifs/sysfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fs/ubifs') diff --git a/fs/ubifs/sysfs.c b/fs/ubifs/sysfs.c index 54270ad36321..1c958148bb87 100644 --- a/fs/ubifs/sysfs.c +++ b/fs/ubifs/sysfs.c @@ -74,13 +74,13 @@ static const struct sysfs_ops ubifs_attr_ops = { .show = ubifs_attr_show, }; -static struct kobj_type ubifs_sb_ktype = { +static const struct kobj_type ubifs_sb_ktype = { .default_groups = ubifs_groups, .sysfs_ops = &ubifs_attr_ops, .release = ubifs_sb_release, }; -static struct kobj_type ubifs_ktype = { +static const struct kobj_type ubifs_ktype = { .sysfs_ops = &ubifs_attr_ops, }; -- cgit