--- /dev/null
+From bc56ad8c74b8588685c2875de0df8ab6974828ef Mon Sep 17 00:00:00 2001
+From: kaixuxia <xiakaixu1987@gmail.com>
+Date: Tue, 3 Sep 2019 21:06:50 -0700
+Subject: xfs: Fix deadlock between AGI and AGF with RENAME_WHITEOUT
+
+From: kaixuxia <xiakaixu1987@gmail.com>
+
+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 <kaixuxia@tencent.com>
+Reviewed-by: Brian Foster <bfoster@redhat.com>
+Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
+Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
+Signed-off-by: Suraj Jitindar Singh <surajjs@amazon.com>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+---
+ 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)
--- /dev/null
+From 9c92ee208b1faa0ef2cc899b85fd0607b6fac7fe Mon Sep 17 00:00:00 2001
+From: "Darrick J. Wong" <darrick.wong@oracle.com>
+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 <darrick.wong@oracle.com>
+
+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 <darrick.wong@oracle.com>
+Reviewed-by: Brian Foster <bfoster@redhat.com>
+Signed-off-by: Suraj Jitindar Singh <surajjs@amazon.com>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+---
+ 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;
projects / thirdparty / kernel / stable-queue.git / commitdiff
