From: Greg Kroah-Hartman Date: Sun, 3 Apr 2022 11:43:07 +0000 (+0200) Subject: 5.10-stable patches X-Git-Tag: v5.17.2~147 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d093b6b84ff2a6b922840de9ba09d771a9e53091;p=thirdparty%2Fkernel%2Fstable-queue.git 5.10-stable patches added patches: ubifs-add-missing-iput-if-do_tmpfile-failed-in-rename-whiteout.patch ubifs-fix-deadlock-in-concurrent-rename-whiteout-and-inode-writeback.patch ubifs-fix-read-out-of-bounds-in-ubifs_wbuf_write_nolock.patch ubifs-fix-to-add-refcount-once-page-is-set-private.patch ubifs-rename_whiteout-correct-old_dir-size-computing.patch ubifs-rename_whiteout-fix-double-free-for-whiteout_ui-data.patch ubifs-setflags-make-dirtied_ino_d-8-bytes-aligned.patch --- diff --git a/queue-5.10/series b/queue-5.10/series index 842e053ef43..6a91d9dd798 100644 --- a/queue-5.10/series +++ b/queue-5.10/series @@ -535,3 +535,10 @@ kvm-prevent-module-exit-until-all-vms-are-freed.patch kvm-x86-fix-sending-pv-ipi.patch kvm-svm-fix-panic-on-out-of-bounds-guest-irq.patch asoc-sof-intel-fix-null-ptr-dereference-when-enomem.patch +ubifs-rename_whiteout-fix-double-free-for-whiteout_ui-data.patch +ubifs-fix-deadlock-in-concurrent-rename-whiteout-and-inode-writeback.patch +ubifs-add-missing-iput-if-do_tmpfile-failed-in-rename-whiteout.patch +ubifs-setflags-make-dirtied_ino_d-8-bytes-aligned.patch +ubifs-fix-read-out-of-bounds-in-ubifs_wbuf_write_nolock.patch +ubifs-fix-to-add-refcount-once-page-is-set-private.patch +ubifs-rename_whiteout-correct-old_dir-size-computing.patch diff --git a/queue-5.10/ubifs-add-missing-iput-if-do_tmpfile-failed-in-rename-whiteout.patch b/queue-5.10/ubifs-add-missing-iput-if-do_tmpfile-failed-in-rename-whiteout.patch new file mode 100644 index 00000000000..524a827b0cd --- /dev/null +++ b/queue-5.10/ubifs-add-missing-iput-if-do_tmpfile-failed-in-rename-whiteout.patch @@ -0,0 +1,35 @@ +From 716b4573026bcbfa7b58ed19fe15554bac66b082 Mon Sep 17 00:00:00 2001 +From: Zhihao Cheng +Date: Mon, 27 Dec 2021 11:22:35 +0800 +Subject: ubifs: Add missing iput if do_tmpfile() failed in rename whiteout + +From: Zhihao Cheng + +commit 716b4573026bcbfa7b58ed19fe15554bac66b082 upstream. + +whiteout inode should be put when do_tmpfile() failed if inode has been +initialized. Otherwise we will get following warning during umount: + UBIFS error (ubi0:0 pid 1494): ubifs_assert_failed [ubifs]: UBIFS + assert failed: c->bi.dd_growth == 0, in fs/ubifs/super.c:1930 + VFS: Busy inodes after unmount of ubifs. Self-destruct in 5 seconds. + +Fixes: 9e0a1fff8db56ea ("ubifs: Implement RENAME_WHITEOUT") +Signed-off-by: Zhihao Cheng +Suggested-by: Sascha Hauer +Signed-off-by: Richard Weinberger +Signed-off-by: Greg Kroah-Hartman +--- + fs/ubifs/dir.c | 2 ++ + 1 file changed, 2 insertions(+) + +--- a/fs/ubifs/dir.c ++++ b/fs/ubifs/dir.c +@@ -431,6 +431,8 @@ out_inode: + make_bad_inode(inode); + if (!instantiated) + iput(inode); ++ else if (whiteout) ++ iput(*whiteout); + out_budg: + ubifs_release_budget(c, &req); + if (!instantiated) diff --git a/queue-5.10/ubifs-fix-deadlock-in-concurrent-rename-whiteout-and-inode-writeback.patch b/queue-5.10/ubifs-fix-deadlock-in-concurrent-rename-whiteout-and-inode-writeback.patch new file mode 100644 index 00000000000..d276e597e1e --- /dev/null +++ b/queue-5.10/ubifs-fix-deadlock-in-concurrent-rename-whiteout-and-inode-writeback.patch @@ -0,0 +1,118 @@ +From afd427048047e8efdedab30e8888044e2be5aa9c Mon Sep 17 00:00:00 2001 +From: Zhihao Cheng +Date: Mon, 27 Dec 2021 11:22:33 +0800 +Subject: ubifs: Fix deadlock in concurrent rename whiteout and inode writeback +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +From: Zhihao Cheng + +commit afd427048047e8efdedab30e8888044e2be5aa9c upstream. + +Following hung tasks: +[ 77.028764] task:kworker/u8:4 state:D stack: 0 pid: 132 +[ 77.028820] Call Trace: +[ 77.029027] schedule+0x8c/0x1b0 +[ 77.029067] mutex_lock+0x50/0x60 +[ 77.029074] ubifs_write_inode+0x68/0x1f0 [ubifs] +[ 77.029117] __writeback_single_inode+0x43c/0x570 +[ 77.029128] writeback_sb_inodes+0x259/0x740 +[ 77.029148] wb_writeback+0x107/0x4d0 +[ 77.029163] wb_workfn+0x162/0x7b0 + +[ 92.390442] task:aa state:D stack: 0 pid: 1506 +[ 92.390448] Call Trace: +[ 92.390458] schedule+0x8c/0x1b0 +[ 92.390461] wb_wait_for_completion+0x82/0xd0 +[ 92.390469] __writeback_inodes_sb_nr+0xb2/0x110 +[ 92.390472] writeback_inodes_sb_nr+0x14/0x20 +[ 92.390476] ubifs_budget_space+0x705/0xdd0 [ubifs] +[ 92.390503] do_rename.cold+0x7f/0x187 [ubifs] +[ 92.390549] ubifs_rename+0x8b/0x180 [ubifs] +[ 92.390571] vfs_rename+0xdb2/0x1170 +[ 92.390580] do_renameat2+0x554/0x770 + +, are caused by concurrent rename whiteout and inode writeback processes: + rename_whiteout(Thread 1) wb_workfn(Thread2) +ubifs_rename + do_rename + lock_4_inodes (Hold ui_mutex) + ubifs_budget_space + make_free_space + shrink_liability + __writeback_inodes_sb_nr + bdi_split_work_to_wbs (Queue new wb work) + wb_do_writeback(wb work) + __writeback_single_inode + ubifs_write_inode + LOCK(ui_mutex) + ↑ + wb_wait_for_completion (Wait wb work) <-- deadlock! + +Reproducer (Detail program in [Link]): + 1. SYS_renameat2("/mp/dir/file", "/mp/dir/whiteout", RENAME_WHITEOUT) + 2. Consume out of space before kernel(mdelay) doing budget for whiteout + +Fix it by doing whiteout space budget before locking ubifs inodes. +BTW, it also fixes wrong goto tag 'out_release' in whiteout budget +error handling path(It should at least recover dir i_size and unlock +4 ubifs inodes). + +Fixes: 9e0a1fff8db56ea ("ubifs: Implement RENAME_WHITEOUT") +Link: https://bugzilla.kernel.org/show_bug.cgi?id=214733 +Signed-off-by: Zhihao Cheng +Signed-off-by: Richard Weinberger +Signed-off-by: Greg Kroah-Hartman +--- + fs/ubifs/dir.c | 25 +++++++++++++++---------- + 1 file changed, 15 insertions(+), 10 deletions(-) + +--- a/fs/ubifs/dir.c ++++ b/fs/ubifs/dir.c +@@ -1322,6 +1322,7 @@ static int do_rename(struct inode *old_d + + if (flags & RENAME_WHITEOUT) { + union ubifs_dev_desc *dev = NULL; ++ struct ubifs_budget_req wht_req; + + dev = kmalloc(sizeof(union ubifs_dev_desc), GFP_NOFS); + if (!dev) { +@@ -1343,6 +1344,20 @@ static int do_rename(struct inode *old_d + whiteout_ui->data = dev; + whiteout_ui->data_len = ubifs_encode_dev(dev, MKDEV(0, 0)); + ubifs_assert(c, !whiteout_ui->dirty); ++ ++ memset(&wht_req, 0, sizeof(struct ubifs_budget_req)); ++ wht_req.dirtied_ino = 1; ++ wht_req.dirtied_ino_d = ALIGN(whiteout_ui->data_len, 8); ++ /* ++ * To avoid deadlock between space budget (holds ui_mutex and ++ * waits wb work) and writeback work(waits ui_mutex), do space ++ * budget before ubifs inodes locked. ++ */ ++ err = ubifs_budget_space(c, &wht_req); ++ if (err) { ++ iput(whiteout); ++ goto out_release; ++ } + } + + lock_4_inodes(old_dir, new_dir, new_inode, whiteout); +@@ -1417,16 +1432,6 @@ static int do_rename(struct inode *old_d + } + + if (whiteout) { +- struct ubifs_budget_req wht_req = { .dirtied_ino = 1, +- .dirtied_ino_d = \ +- ALIGN(ubifs_inode(whiteout)->data_len, 8) }; +- +- err = ubifs_budget_space(c, &wht_req); +- if (err) { +- iput(whiteout); +- goto out_release; +- } +- + inc_nlink(whiteout); + mark_inode_dirty(whiteout); + diff --git a/queue-5.10/ubifs-fix-read-out-of-bounds-in-ubifs_wbuf_write_nolock.patch b/queue-5.10/ubifs-fix-read-out-of-bounds-in-ubifs_wbuf_write_nolock.patch new file mode 100644 index 00000000000..20173de5f88 --- /dev/null +++ b/queue-5.10/ubifs-fix-read-out-of-bounds-in-ubifs_wbuf_write_nolock.patch @@ -0,0 +1,109 @@ +From 4f2262a334641e05f645364d5ade1f565c85f20b Mon Sep 17 00:00:00 2001 +From: Zhihao Cheng +Date: Mon, 27 Dec 2021 11:22:40 +0800 +Subject: ubifs: Fix read out-of-bounds in ubifs_wbuf_write_nolock() + +From: Zhihao Cheng + +commit 4f2262a334641e05f645364d5ade1f565c85f20b upstream. + +Function ubifs_wbuf_write_nolock() may access buf out of bounds in +following process: + +ubifs_wbuf_write_nolock(): + aligned_len = ALIGN(len, 8); // Assume len = 4089, aligned_len = 4096 + if (aligned_len <= wbuf->avail) ... // Not satisfy + if (wbuf->used) { + ubifs_leb_write() // Fill some data in avail wbuf + len -= wbuf->avail; // len is still not 8-bytes aligned + aligned_len -= wbuf->avail; + } + n = aligned_len >> c->max_write_shift; + if (n) { + n <<= c->max_write_shift; + err = ubifs_leb_write(c, wbuf->lnum, buf + written, + wbuf->offs, n); + // n > len, read out of bounds less than 8(n-len) bytes + } + +, which can be catched by KASAN: + ========================================================= + BUG: KASAN: slab-out-of-bounds in ecc_sw_hamming_calculate+0x1dc/0x7d0 + Read of size 4 at addr ffff888105594ff8 by task kworker/u8:4/128 + Workqueue: writeback wb_workfn (flush-ubifs_0_0) + Call Trace: + kasan_report.cold+0x81/0x165 + nand_write_page_swecc+0xa9/0x160 + ubifs_leb_write+0xf2/0x1b0 [ubifs] + ubifs_wbuf_write_nolock+0x421/0x12c0 [ubifs] + write_head+0xdc/0x1c0 [ubifs] + ubifs_jnl_write_inode+0x627/0x960 [ubifs] + wb_workfn+0x8af/0xb80 + +Function ubifs_wbuf_write_nolock() accepts that parameter 'len' is not 8 +bytes aligned, the 'len' represents the true length of buf (which is +allocated in 'ubifs_jnl_xxx', eg. ubifs_jnl_write_inode), so +ubifs_wbuf_write_nolock() must handle the length read from 'buf' carefully +to write leb safely. + +Fetch a reproducer in [Link]. + +Fixes: 1e51764a3c2ac0 ("UBIFS: add new flash file system") +Link: https://bugzilla.kernel.org/show_bug.cgi?id=214785 +Reported-by: Chengsong Ke +Signed-off-by: Zhihao Cheng +Signed-off-by: Richard Weinberger +Signed-off-by: Greg Kroah-Hartman +--- + fs/ubifs/io.c | 34 ++++++++++++++++++++++++++++++---- + 1 file changed, 30 insertions(+), 4 deletions(-) + +--- a/fs/ubifs/io.c ++++ b/fs/ubifs/io.c +@@ -846,16 +846,42 @@ int ubifs_wbuf_write_nolock(struct ubifs + */ + n = aligned_len >> c->max_write_shift; + if (n) { +- n <<= c->max_write_shift; ++ int m = n - 1; ++ + dbg_io("write %d bytes to LEB %d:%d", n, wbuf->lnum, + wbuf->offs); +- err = ubifs_leb_write(c, wbuf->lnum, buf + written, +- wbuf->offs, n); ++ ++ if (m) { ++ /* '(n-1)<max_write_shift < len' is always true. */ ++ m <<= c->max_write_shift; ++ err = ubifs_leb_write(c, wbuf->lnum, buf + written, ++ wbuf->offs, m); ++ if (err) ++ goto out; ++ wbuf->offs += m; ++ aligned_len -= m; ++ len -= m; ++ written += m; ++ } ++ ++ /* ++ * The non-written len of buf may be less than 'n' because ++ * parameter 'len' is not 8 bytes aligned, so here we read ++ * min(len, n) bytes from buf. ++ */ ++ n = 1 << c->max_write_shift; ++ memcpy(wbuf->buf, buf + written, min(len, n)); ++ if (n > len) { ++ ubifs_assert(c, n - len < 8); ++ ubifs_pad(c, wbuf->buf + len, n - len); ++ } ++ ++ err = ubifs_leb_write(c, wbuf->lnum, wbuf->buf, wbuf->offs, n); + if (err) + goto out; + wbuf->offs += n; + aligned_len -= n; +- len -= n; ++ len -= min(len, n); + written += n; + } + diff --git a/queue-5.10/ubifs-fix-to-add-refcount-once-page-is-set-private.patch b/queue-5.10/ubifs-fix-to-add-refcount-once-page-is-set-private.patch new file mode 100644 index 00000000000..9a23681765e --- /dev/null +++ b/queue-5.10/ubifs-fix-to-add-refcount-once-page-is-set-private.patch @@ -0,0 +1,184 @@ +From 3b67db8a6ca83e6ff90b756d3da0c966f61cd37b Mon Sep 17 00:00:00 2001 +From: Zhihao Cheng +Date: Mon, 27 Dec 2021 11:22:41 +0800 +Subject: ubifs: Fix to add refcount once page is set private + +From: Zhihao Cheng + +commit 3b67db8a6ca83e6ff90b756d3da0c966f61cd37b upstream. + +MM defined the rule [1] very clearly that once page was set with PG_private +flag, we should increment the refcount in that page, also main flows like +pageout(), migrate_page() will assume there is one additional page +reference count if page_has_private() returns true. Otherwise, we may +get a BUG in page migration: + + page:0000000080d05b9d refcount:-1 mapcount:0 mapping:000000005f4d82a8 + index:0xe2 pfn:0x14c12 + aops:ubifs_file_address_operations [ubifs] ino:8f1 dentry name:"f30e" + flags: 0x1fffff80002405(locked|uptodate|owner_priv_1|private|node=0| + zone=1|lastcpupid=0x1fffff) + page dumped because: VM_BUG_ON_PAGE(page_count(page) != 0) + ------------[ cut here ]------------ + kernel BUG at include/linux/page_ref.h:184! + invalid opcode: 0000 [#1] SMP + CPU: 3 PID: 38 Comm: kcompactd0 Not tainted 5.15.0-rc5 + RIP: 0010:migrate_page_move_mapping+0xac3/0xe70 + Call Trace: + ubifs_migrate_page+0x22/0xc0 [ubifs] + move_to_new_page+0xb4/0x600 + migrate_pages+0x1523/0x1cc0 + compact_zone+0x8c5/0x14b0 + kcompactd+0x2bc/0x560 + kthread+0x18c/0x1e0 + ret_from_fork+0x1f/0x30 + +Before the time, we should make clean a concept, what does refcount means +in page gotten from grab_cache_page_write_begin(). There are 2 situations: +Situation 1: refcount is 3, page is created by __page_cache_alloc. + TYPE_A - the write process is using this page + TYPE_B - page is assigned to one certain mapping by calling + __add_to_page_cache_locked() + TYPE_C - page is added into pagevec list corresponding current cpu by + calling lru_cache_add() +Situation 2: refcount is 2, page is gotten from the mapping's tree + TYPE_B - page has been assigned to one certain mapping + TYPE_A - the write process is using this page (by calling + page_cache_get_speculative()) +Filesystem releases one refcount by calling put_page() in xxx_write_end(), +the released refcount corresponds to TYPE_A (write task is using it). If +there are any processes using a page, page migration process will skip the +page by judging whether expected_page_refs() equals to page refcount. + +The BUG is caused by following process: + PA(cpu 0) kcompactd(cpu 1) + compact_zone +ubifs_write_begin + page_a = grab_cache_page_write_begin + add_to_page_cache_lru + lru_cache_add + pagevec_add // put page into cpu 0's pagevec + (refcnf = 3, for page creation process) +ubifs_write_end + SetPagePrivate(page_a) // doesn't increase page count ! + unlock_page(page_a) + put_page(page_a) // refcnt = 2 + [...] + + PB(cpu 0) +filemap_read + filemap_get_pages + add_to_page_cache_lru + lru_cache_add + __pagevec_lru_add // traverse all pages in cpu 0's pagevec + __pagevec_lru_add_fn + SetPageLRU(page_a) + isolate_migratepages + isolate_migratepages_block + get_page_unless_zero(page_a) + // refcnt = 3 + list_add(page_a, from_list) + migrate_pages(from_list) + __unmap_and_move + move_to_new_page + ubifs_migrate_page(page_a) + migrate_page_move_mapping + expected_page_refs get 3 + (migration[1] + mapping[1] + private[1]) + release_pages + put_page_testzero(page_a) // refcnt = 3 + page_ref_freeze // refcnt = 0 + page_ref_dec_and_test(0 - 1 = -1) + page_ref_unfreeze + VM_BUG_ON_PAGE(-1 != 0, page) + +UBIFS doesn't increase the page refcount after setting private flag, which +leads to page migration task believes the page is not used by any other +processes, so the page is migrated. This causes concurrent accessing on +page refcount between put_page() called by other process(eg. read process +calls lru_cache_add) and page_ref_unfreeze() called by migration task. + +Actually zhangjun has tried to fix this problem [2] by recalculating page +refcnt in ubifs_migrate_page(). It's better to follow MM rules [1], because +just like Kirill suggested in [2], we need to check all users of +page_has_private() helper. Like f2fs does in [3], fix it by adding/deleting +refcount when setting/clearing private for a page. BTW, according to [4], +we set 'page->private' as 1 because ubifs just simply SetPagePrivate(). +And, [5] provided a common helper to set/clear page private, ubifs can +use this helper following the example of iomap, afs, btrfs, etc. + +Jump [6] to find a reproducer. + +[1] https://lore.kernel.org/lkml/2b19b3c4-2bc4-15fa-15cc-27a13e5c7af1@aol.com +[2] https://www.spinics.net/lists/linux-mtd/msg04018.html +[3] http://lkml.iu.edu/hypermail/linux/kernel/1903.0/03313.html +[4] https://lore.kernel.org/linux-f2fs-devel/20210422154705.GO3596236@casper.infradead.org +[5] https://lore.kernel.org/all/20200517214718.468-1-guoqing.jiang@cloud.ionos.com +[6] https://bugzilla.kernel.org/show_bug.cgi?id=214961 + +Fixes: 1e51764a3c2ac0 ("UBIFS: add new flash file system") +Signed-off-by: Zhihao Cheng +Signed-off-by: Richard Weinberger +Signed-off-by: Greg Kroah-Hartman +--- + fs/ubifs/file.c | 14 +++++++------- + 1 file changed, 7 insertions(+), 7 deletions(-) + +--- a/fs/ubifs/file.c ++++ b/fs/ubifs/file.c +@@ -570,7 +570,7 @@ static int ubifs_write_end(struct file * + } + + if (!PagePrivate(page)) { +- SetPagePrivate(page); ++ attach_page_private(page, (void *)1); + atomic_long_inc(&c->dirty_pg_cnt); + __set_page_dirty_nobuffers(page); + } +@@ -947,7 +947,7 @@ static int do_writepage(struct page *pag + release_existing_page_budget(c); + + atomic_long_dec(&c->dirty_pg_cnt); +- ClearPagePrivate(page); ++ detach_page_private(page); + ClearPageChecked(page); + + kunmap(page); +@@ -1303,7 +1303,7 @@ static void ubifs_invalidatepage(struct + release_existing_page_budget(c); + + atomic_long_dec(&c->dirty_pg_cnt); +- ClearPagePrivate(page); ++ detach_page_private(page); + ClearPageChecked(page); + } + +@@ -1470,8 +1470,8 @@ static int ubifs_migrate_page(struct add + return rc; + + if (PagePrivate(page)) { +- ClearPagePrivate(page); +- SetPagePrivate(newpage); ++ detach_page_private(page); ++ attach_page_private(newpage, (void *)1); + } + + if (mode != MIGRATE_SYNC_NO_COPY) +@@ -1495,7 +1495,7 @@ static int ubifs_releasepage(struct page + return 0; + ubifs_assert(c, PagePrivate(page)); + ubifs_assert(c, 0); +- ClearPagePrivate(page); ++ detach_page_private(page); + ClearPageChecked(page); + return 1; + } +@@ -1566,7 +1566,7 @@ static vm_fault_t ubifs_vm_page_mkwrite( + else { + if (!PageChecked(page)) + ubifs_convert_page_budget(c); +- SetPagePrivate(page); ++ attach_page_private(page, (void *)1); + atomic_long_inc(&c->dirty_pg_cnt); + __set_page_dirty_nobuffers(page); + } diff --git a/queue-5.10/ubifs-rename_whiteout-correct-old_dir-size-computing.patch b/queue-5.10/ubifs-rename_whiteout-correct-old_dir-size-computing.patch new file mode 100644 index 00000000000..22c55b9d97e --- /dev/null +++ b/queue-5.10/ubifs-rename_whiteout-correct-old_dir-size-computing.patch @@ -0,0 +1,35 @@ +From 705757274599e2e064dd3054aabc74e8af31a095 Mon Sep 17 00:00:00 2001 +From: Baokun Li +Date: Tue, 15 Feb 2022 12:07:36 +0800 +Subject: ubifs: rename_whiteout: correct old_dir size computing + +From: Baokun Li + +commit 705757274599e2e064dd3054aabc74e8af31a095 upstream. + +When renaming the whiteout file, the old whiteout file is not deleted. +Therefore, we add the old dentry size to the old dir like XFS. +Otherwise, an error may be reported due to `fscki->calc_sz != fscki->size` +in check_indes. + +Fixes: 9e0a1fff8db56ea ("ubifs: Implement RENAME_WHITEOUT") +Reported-by: Zhihao Cheng +Signed-off-by: Baokun Li +Signed-off-by: Richard Weinberger +Signed-off-by: Greg Kroah-Hartman +--- + fs/ubifs/dir.c | 3 +++ + 1 file changed, 3 insertions(+) + +--- a/fs/ubifs/dir.c ++++ b/fs/ubifs/dir.c +@@ -1360,6 +1360,9 @@ static int do_rename(struct inode *old_d + iput(whiteout); + goto out_release; + } ++ ++ /* Add the old_dentry size to the old_dir size. */ ++ old_sz -= CALC_DENT_SIZE(fname_len(&old_nm)); + } + + lock_4_inodes(old_dir, new_dir, new_inode, whiteout); diff --git a/queue-5.10/ubifs-rename_whiteout-fix-double-free-for-whiteout_ui-data.patch b/queue-5.10/ubifs-rename_whiteout-fix-double-free-for-whiteout_ui-data.patch new file mode 100644 index 00000000000..73a90640ea9 --- /dev/null +++ b/queue-5.10/ubifs-rename_whiteout-fix-double-free-for-whiteout_ui-data.patch @@ -0,0 +1,71 @@ +From 40a8f0d5e7b3999f096570edab71c345da812e3e Mon Sep 17 00:00:00 2001 +From: Zhihao Cheng +Date: Mon, 27 Dec 2021 11:22:32 +0800 +Subject: ubifs: rename_whiteout: Fix double free for whiteout_ui->data + +From: Zhihao Cheng + +commit 40a8f0d5e7b3999f096570edab71c345da812e3e upstream. + +'whiteout_ui->data' will be freed twice if space budget fail for +rename whiteout operation as following process: + +rename_whiteout + dev = kmalloc + whiteout_ui->data = dev + kfree(whiteout_ui->data) // Free first time + iput(whiteout) + ubifs_free_inode + kfree(ui->data) // Double free! + +KASAN reports: +================================================================== +BUG: KASAN: double-free or invalid-free in ubifs_free_inode+0x4f/0x70 +Call Trace: + kfree+0x117/0x490 + ubifs_free_inode+0x4f/0x70 [ubifs] + i_callback+0x30/0x60 + rcu_do_batch+0x366/0xac0 + __do_softirq+0x133/0x57f + +Allocated by task 1506: + kmem_cache_alloc_trace+0x3c2/0x7a0 + do_rename+0x9b7/0x1150 [ubifs] + ubifs_rename+0x106/0x1f0 [ubifs] + do_syscall_64+0x35/0x80 + +Freed by task 1506: + kfree+0x117/0x490 + do_rename.cold+0x53/0x8a [ubifs] + ubifs_rename+0x106/0x1f0 [ubifs] + do_syscall_64+0x35/0x80 + +The buggy address belongs to the object at ffff88810238bed8 which +belongs to the cache kmalloc-8 of size 8 +================================================================== + +Let ubifs_free_inode() free 'whiteout_ui->data'. BTW, delete unused +assignment 'whiteout_ui->data_len = 0', process 'ubifs_evict_inode() +-> ubifs_jnl_delete_inode() -> ubifs_jnl_write_inode()' doesn't need it +(because 'inc_nlink(whiteout)' won't be excuted by 'goto out_release', + and the nlink of whiteout inode is 0). + +Fixes: 9e0a1fff8db56ea ("ubifs: Implement RENAME_WHITEOUT") +Signed-off-by: Zhihao Cheng +Signed-off-by: Richard Weinberger +Signed-off-by: Greg Kroah-Hartman +--- + fs/ubifs/dir.c | 2 -- + 1 file changed, 2 deletions(-) + +--- a/fs/ubifs/dir.c ++++ b/fs/ubifs/dir.c +@@ -1423,8 +1423,6 @@ static int do_rename(struct inode *old_d + + err = ubifs_budget_space(c, &wht_req); + if (err) { +- kfree(whiteout_ui->data); +- whiteout_ui->data_len = 0; + iput(whiteout); + goto out_release; + } diff --git a/queue-5.10/ubifs-setflags-make-dirtied_ino_d-8-bytes-aligned.patch b/queue-5.10/ubifs-setflags-make-dirtied_ino_d-8-bytes-aligned.patch new file mode 100644 index 00000000000..77c93d4723b --- /dev/null +++ b/queue-5.10/ubifs-setflags-make-dirtied_ino_d-8-bytes-aligned.patch @@ -0,0 +1,38 @@ +From 1b83ec057db16b4d0697dc21ef7a9743b6041f72 Mon Sep 17 00:00:00 2001 +From: Zhihao Cheng +Date: Mon, 27 Dec 2021 11:22:39 +0800 +Subject: ubifs: setflags: Make dirtied_ino_d 8 bytes aligned + +From: Zhihao Cheng + +commit 1b83ec057db16b4d0697dc21ef7a9743b6041f72 upstream. + +Make 'ui->data_len' aligned with 8 bytes before it is assigned to +dirtied_ino_d. Since 8871d84c8f8b0c6b("ubifs: convert to fileattr") +applied, 'setflags()' only affects regular files and directories, only +xattr inode, symlink inode and special inode(pipe/char_dev/block_dev) +have none- zero 'ui->data_len' field, so assertion +'!(req->dirtied_ino_d & 7)' cannot fail in ubifs_budget_space(). +To avoid assertion fails in future evolution(eg. setflags can operate +special inodes), it's better to make dirtied_ino_d 8 bytes aligned, +after all aligned size is still zero for regular files. + +Fixes: 1e51764a3c2ac05a ("UBIFS: add new flash file system") +Signed-off-by: Zhihao Cheng +Signed-off-by: Richard Weinberger +Signed-off-by: Greg Kroah-Hartman +--- + fs/ubifs/ioctl.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +--- a/fs/ubifs/ioctl.c ++++ b/fs/ubifs/ioctl.c +@@ -107,7 +107,7 @@ static int setflags(struct inode *inode, + struct ubifs_inode *ui = ubifs_inode(inode); + struct ubifs_info *c = inode->i_sb->s_fs_info; + struct ubifs_budget_req req = { .dirtied_ino = 1, +- .dirtied_ino_d = ui->data_len }; ++ .dirtied_ino_d = ALIGN(ui->data_len, 8) }; + + err = ubifs_budget_space(c, &req); + if (err)