From: Greg Kroah-Hartman Date: Tue, 28 Apr 2020 09:08:35 +0000 (+0200) Subject: 4.14-stable patches X-Git-Tag: v4.19.119~8 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=233e6a3d51f8815e6a2b10c4e4d7fa26784d9f9f;p=thirdparty%2Fkernel%2Fstable-queue.git 4.14-stable patches added patches: xfs-fix-deadlock-between-agi-and-agf-with-rename_whiteout.patch xfs-validate-sb_logsunit-is-a-multiple-of-the-fs-blocksize.patch --- diff --git a/queue-4.14/series b/queue-4.14/series index 2060c9ba09d..1ab31636ea4 100644 --- a/queue-4.14/series +++ b/queue-4.14/series @@ -80,3 +80,5 @@ uas-no-use-logging-any-details-in-case-of-enodev.patch uas-fix-deadlock-in-error-handling-and-pm-flushing-work.patch usb-f_fs-clear-os-extended-descriptor-counts-to-zero-in-ffs_data_reset.patch serial-sh-sci-make-sure-status-register-scxsr-is-read-in-correct-sequence.patch +xfs-validate-sb_logsunit-is-a-multiple-of-the-fs-blocksize.patch +xfs-fix-deadlock-between-agi-and-agf-with-rename_whiteout.patch diff --git a/queue-4.14/xfs-fix-deadlock-between-agi-and-agf-with-rename_whiteout.patch b/queue-4.14/xfs-fix-deadlock-between-agi-and-agf-with-rename_whiteout.patch new file mode 100644 index 00000000000..f1bc866f2e3 --- /dev/null +++ b/queue-4.14/xfs-fix-deadlock-between-agi-and-agf-with-rename_whiteout.patch @@ -0,0 +1,209 @@ +From bc56ad8c74b8588685c2875de0df8ab6974828ef Mon Sep 17 00:00:00 2001 +From: kaixuxia +Date: Tue, 3 Sep 2019 21:06:50 -0700 +Subject: xfs: Fix deadlock between AGI and AGF with RENAME_WHITEOUT + +From: kaixuxia + +commit bc56ad8c74b8588685c2875de0df8ab6974828ef upstream. + +When performing rename operation with RENAME_WHITEOUT flag, we will +hold AGF lock to allocate or free extents in manipulating the dirents +firstly, and then doing the xfs_iunlink_remove() call last to hold +AGI lock to modify the tmpfile info, so we the lock order AGI->AGF. + +The big problem here is that we have an ordering constraint on AGF +and AGI locking - inode allocation locks the AGI, then can allocate +a new extent for new inodes, locking the AGF after the AGI. Hence +the ordering that is imposed by other parts of the code is AGI before +AGF. So we get an ABBA deadlock between the AGI and AGF here. + +Process A: +Call trace: + ? __schedule+0x2bd/0x620 + schedule+0x33/0x90 + schedule_timeout+0x17d/0x290 + __down_common+0xef/0x125 + ? xfs_buf_find+0x215/0x6c0 [xfs] + down+0x3b/0x50 + xfs_buf_lock+0x34/0xf0 [xfs] + xfs_buf_find+0x215/0x6c0 [xfs] + xfs_buf_get_map+0x37/0x230 [xfs] + xfs_buf_read_map+0x29/0x190 [xfs] + xfs_trans_read_buf_map+0x13d/0x520 [xfs] + xfs_read_agf+0xa6/0x180 [xfs] + ? schedule_timeout+0x17d/0x290 + xfs_alloc_read_agf+0x52/0x1f0 [xfs] + xfs_alloc_fix_freelist+0x432/0x590 [xfs] + ? down+0x3b/0x50 + ? xfs_buf_lock+0x34/0xf0 [xfs] + ? xfs_buf_find+0x215/0x6c0 [xfs] + xfs_alloc_vextent+0x301/0x6c0 [xfs] + xfs_ialloc_ag_alloc+0x182/0x700 [xfs] + ? _xfs_trans_bjoin+0x72/0xf0 [xfs] + xfs_dialloc+0x116/0x290 [xfs] + xfs_ialloc+0x6d/0x5e0 [xfs] + ? xfs_log_reserve+0x165/0x280 [xfs] + xfs_dir_ialloc+0x8c/0x240 [xfs] + xfs_create+0x35a/0x610 [xfs] + xfs_generic_create+0x1f1/0x2f0 [xfs] + ... + +Process B: +Call trace: + ? __schedule+0x2bd/0x620 + ? xfs_bmapi_allocate+0x245/0x380 [xfs] + schedule+0x33/0x90 + schedule_timeout+0x17d/0x290 + ? xfs_buf_find+0x1fd/0x6c0 [xfs] + __down_common+0xef/0x125 + ? xfs_buf_get_map+0x37/0x230 [xfs] + ? xfs_buf_find+0x215/0x6c0 [xfs] + down+0x3b/0x50 + xfs_buf_lock+0x34/0xf0 [xfs] + xfs_buf_find+0x215/0x6c0 [xfs] + xfs_buf_get_map+0x37/0x230 [xfs] + xfs_buf_read_map+0x29/0x190 [xfs] + xfs_trans_read_buf_map+0x13d/0x520 [xfs] + xfs_read_agi+0xa8/0x160 [xfs] + xfs_iunlink_remove+0x6f/0x2a0 [xfs] + ? current_time+0x46/0x80 + ? xfs_trans_ichgtime+0x39/0xb0 [xfs] + xfs_rename+0x57a/0xae0 [xfs] + xfs_vn_rename+0xe4/0x150 [xfs] + ... + +In this patch we move the xfs_iunlink_remove() call to +before acquiring the AGF lock to preserve correct AGI/AGF locking +order. + +[Minor massage required to backport to apply due to removal of +out_bmap_cancel: error path label upstream as a result of code +rework. Only change was to the last code block removed by the +patch. Functionally equivalent to upstream.] + +Signed-off-by: kaixuxia +Reviewed-by: Brian Foster +Reviewed-by: Darrick J. Wong +Signed-off-by: Darrick J. Wong +Signed-off-by: Suraj Jitindar Singh +Signed-off-by: Greg Kroah-Hartman +--- + fs/xfs/xfs_inode.c | 85 ++++++++++++++++++++++++++--------------------------- + 1 file changed, 42 insertions(+), 43 deletions(-) + +--- a/fs/xfs/xfs_inode.c ++++ b/fs/xfs/xfs_inode.c +@@ -3035,7 +3035,8 @@ xfs_rename( + &dfops, &first_block, spaceres); + + /* +- * Set up the target. ++ * Check for expected errors before we dirty the transaction ++ * so we can return an error without a transaction abort. + */ + if (target_ip == NULL) { + /* +@@ -3047,6 +3048,46 @@ xfs_rename( + if (error) + goto out_trans_cancel; + } ++ } else { ++ /* ++ * If target exists and it's a directory, check that whether ++ * it can be destroyed. ++ */ ++ if (S_ISDIR(VFS_I(target_ip)->i_mode) && ++ (!xfs_dir_isempty(target_ip) || ++ (VFS_I(target_ip)->i_nlink > 2))) { ++ error = -EEXIST; ++ goto out_trans_cancel; ++ } ++ } ++ ++ /* ++ * Directory entry creation below may acquire the AGF. Remove ++ * the whiteout from the unlinked list first to preserve correct ++ * AGI/AGF locking order. This dirties the transaction so failures ++ * after this point will abort and log recovery will clean up the ++ * mess. ++ * ++ * For whiteouts, we need to bump the link count on the whiteout ++ * inode. After this point, we have a real link, clear the tmpfile ++ * state flag from the inode so it doesn't accidentally get misused ++ * in future. ++ */ ++ if (wip) { ++ ASSERT(VFS_I(wip)->i_nlink == 0); ++ error = xfs_iunlink_remove(tp, wip); ++ if (error) ++ goto out_trans_cancel; ++ ++ xfs_bumplink(tp, wip); ++ xfs_trans_log_inode(tp, wip, XFS_ILOG_CORE); ++ VFS_I(wip)->i_state &= ~I_LINKABLE; ++ } ++ ++ /* ++ * Set up the target. ++ */ ++ if (target_ip == NULL) { + /* + * If target does not exist and the rename crosses + * directories, adjust the target directory link count +@@ -3068,22 +3109,6 @@ xfs_rename( + } + } else { /* target_ip != NULL */ + /* +- * If target exists and it's a directory, check that both +- * target and source are directories and that target can be +- * destroyed, or that neither is a directory. +- */ +- if (S_ISDIR(VFS_I(target_ip)->i_mode)) { +- /* +- * Make sure target dir is empty. +- */ +- if (!(xfs_dir_isempty(target_ip)) || +- (VFS_I(target_ip)->i_nlink > 2)) { +- error = -EEXIST; +- goto out_trans_cancel; +- } +- } +- +- /* + * Link the source inode under the target name. + * If the source inode is a directory and we are moving + * it across directories, its ".." entry will be +@@ -3175,32 +3200,6 @@ xfs_rename( + if (error) + goto out_bmap_cancel; + +- /* +- * For whiteouts, we need to bump the link count on the whiteout inode. +- * This means that failures all the way up to this point leave the inode +- * on the unlinked list and so cleanup is a simple matter of dropping +- * the remaining reference to it. If we fail here after bumping the link +- * count, we're shutting down the filesystem so we'll never see the +- * intermediate state on disk. +- */ +- if (wip) { +- ASSERT(VFS_I(wip)->i_nlink == 0); +- error = xfs_bumplink(tp, wip); +- if (error) +- goto out_bmap_cancel; +- error = xfs_iunlink_remove(tp, wip); +- if (error) +- goto out_bmap_cancel; +- xfs_trans_log_inode(tp, wip, XFS_ILOG_CORE); +- +- /* +- * Now we have a real link, clear the "I'm a tmpfile" state +- * flag from the inode so it doesn't accidentally get misused in +- * future. +- */ +- VFS_I(wip)->i_state &= ~I_LINKABLE; +- } +- + xfs_trans_ichgtime(tp, src_dp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG); + xfs_trans_log_inode(tp, src_dp, XFS_ILOG_CORE); + if (new_parent) diff --git a/queue-4.14/xfs-validate-sb_logsunit-is-a-multiple-of-the-fs-blocksize.patch b/queue-4.14/xfs-validate-sb_logsunit-is-a-multiple-of-the-fs-blocksize.patch new file mode 100644 index 00000000000..4ba2a8698d5 --- /dev/null +++ b/queue-4.14/xfs-validate-sb_logsunit-is-a-multiple-of-the-fs-blocksize.patch @@ -0,0 +1,54 @@ +From 9c92ee208b1faa0ef2cc899b85fd0607b6fac7fe Mon Sep 17 00:00:00 2001 +From: "Darrick J. Wong" +Date: Wed, 25 Oct 2017 16:59:43 -0700 +Subject: xfs: validate sb_logsunit is a multiple of the fs blocksize + +From: Darrick J. Wong + +commit 9c92ee208b1faa0ef2cc899b85fd0607b6fac7fe upstream. + +Make sure the log stripe unit is sane before proceeding with mounting. +AFAICT this means that logsunit has to be 0, 1, or a multiple of the fs +block size. Found this by setting the LSB of logsunit in xfs/350 and +watching the system crash as soon as we try to write to the log. + +Signed-off-by: Darrick J. Wong +Reviewed-by: Brian Foster +Signed-off-by: Suraj Jitindar Singh +Signed-off-by: Greg Kroah-Hartman +--- + fs/xfs/xfs_log.c | 14 +++++++++++++- + 1 file changed, 13 insertions(+), 1 deletion(-) + +--- a/fs/xfs/xfs_log.c ++++ b/fs/xfs/xfs_log.c +@@ -608,6 +608,7 @@ xfs_log_mount( + xfs_daddr_t blk_offset, + int num_bblks) + { ++ bool fatal = xfs_sb_version_hascrc(&mp->m_sb); + int error = 0; + int min_logfsbs; + +@@ -659,9 +660,20 @@ xfs_log_mount( + XFS_FSB_TO_B(mp, mp->m_sb.sb_logblocks), + XFS_MAX_LOG_BYTES); + error = -EINVAL; ++ } else if (mp->m_sb.sb_logsunit > 1 && ++ mp->m_sb.sb_logsunit % mp->m_sb.sb_blocksize) { ++ xfs_warn(mp, ++ "log stripe unit %u bytes must be a multiple of block size", ++ mp->m_sb.sb_logsunit); ++ error = -EINVAL; ++ fatal = true; + } + if (error) { +- if (xfs_sb_version_hascrc(&mp->m_sb)) { ++ /* ++ * Log check errors are always fatal on v5; or whenever bad ++ * metadata leads to a crash. ++ */ ++ if (fatal) { + xfs_crit(mp, "AAIEEE! Log failed size checks. Abort!"); + ASSERT(0); + goto out_free_log;