public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, wenli xie <wlxie7296@gmail.com>,
	"Darrick J. Wong" <darrick.wong@oracle.com>,
	Brian Foster <bfoster@redhat.com>,
	Amir Goldstein <amir73il@gmail.com>
Subject: [PATCH 5.10 19/53] xfs: fix an ABBA deadlock in xfs_rename
Date: Fri,  3 Jun 2022 19:43:04 +0200	[thread overview]
Message-ID: <20220603173819.282169316@linuxfoundation.org> (raw)
In-Reply-To: <20220603173818.716010877@linuxfoundation.org>

From: "Darrick J. Wong" <darrick.wong@oracle.com>

commit 6da1b4b1ab36d80a3994fd4811c8381de10af604 upstream.

When overlayfs is running on top of xfs and the user unlinks a file in
the overlay, overlayfs will create a whiteout inode and ask xfs to
"rename" the whiteout file atop the one being unlinked.  If the file
being unlinked loses its one nlink, we then have to put the inode on the
unlinked list.

This requires us to grab the AGI buffer of the whiteout inode to take it
off the unlinked list (which is where whiteouts are created) and to grab
the AGI buffer of the file being deleted.  If the whiteout was created
in a higher numbered AG than the file being deleted, we'll lock the AGIs
in the wrong order and deadlock.

Therefore, grab all the AGI locks we think we'll need ahead of time, and
in order of increasing AG number per the locking rules.

Reported-by: wenli xie <wlxie7296@gmail.com>
Fixes: 93597ae8dac0 ("xfs: Fix deadlock between AGI and AGF when target_ip exists in xfs_rename()")
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 fs/xfs/libxfs/xfs_dir2.h    |    2 --
 fs/xfs/libxfs/xfs_dir2_sf.c |    2 +-
 fs/xfs/xfs_inode.c          |   42 +++++++++++++++++++++++++-----------------
 3 files changed, 26 insertions(+), 20 deletions(-)

--- a/fs/xfs/libxfs/xfs_dir2.h
+++ b/fs/xfs/libxfs/xfs_dir2.h
@@ -47,8 +47,6 @@ extern int xfs_dir_lookup(struct xfs_tra
 extern int xfs_dir_removename(struct xfs_trans *tp, struct xfs_inode *dp,
 				struct xfs_name *name, xfs_ino_t ino,
 				xfs_extlen_t tot);
-extern bool xfs_dir2_sf_replace_needblock(struct xfs_inode *dp,
-				xfs_ino_t inum);
 extern int xfs_dir_replace(struct xfs_trans *tp, struct xfs_inode *dp,
 				struct xfs_name *name, xfs_ino_t inum,
 				xfs_extlen_t tot);
--- a/fs/xfs/libxfs/xfs_dir2_sf.c
+++ b/fs/xfs/libxfs/xfs_dir2_sf.c
@@ -1018,7 +1018,7 @@ xfs_dir2_sf_removename(
 /*
  * Check whether the sf dir replace operation need more blocks.
  */
-bool
+static bool
 xfs_dir2_sf_replace_needblock(
 	struct xfs_inode	*dp,
 	xfs_ino_t		inum)
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -3152,7 +3152,7 @@ xfs_rename(
 	struct xfs_trans	*tp;
 	struct xfs_inode	*wip = NULL;		/* whiteout inode */
 	struct xfs_inode	*inodes[__XFS_SORT_INODES];
-	struct xfs_buf		*agibp;
+	int			i;
 	int			num_inodes = __XFS_SORT_INODES;
 	bool			new_parent = (src_dp != target_dp);
 	bool			src_is_directory = S_ISDIR(VFS_I(src_ip)->i_mode);
@@ -3266,6 +3266,30 @@ xfs_rename(
 	}
 
 	/*
+	 * Lock the AGI buffers we need to handle bumping the nlink of the
+	 * whiteout inode off the unlinked list and to handle dropping the
+	 * nlink of the target inode.  Per locking order rules, do this in
+	 * increasing AG order and before directory block allocation tries to
+	 * grab AGFs because we grab AGIs before AGFs.
+	 *
+	 * The (vfs) caller must ensure that if src is a directory then
+	 * target_ip is either null or an empty directory.
+	 */
+	for (i = 0; i < num_inodes && inodes[i] != NULL; i++) {
+		if (inodes[i] == wip ||
+		    (inodes[i] == target_ip &&
+		     (VFS_I(target_ip)->i_nlink == 1 || src_is_directory))) {
+			struct xfs_buf	*bp;
+			xfs_agnumber_t	agno;
+
+			agno = XFS_INO_TO_AGNO(mp, inodes[i]->i_ino);
+			error = xfs_read_agi(mp, tp, agno, &bp);
+			if (error)
+				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
@@ -3317,22 +3341,6 @@ xfs_rename(
 		 * In case there is already an entry with the same
 		 * name at the destination directory, remove it first.
 		 */
-
-		/*
-		 * Check whether the replace operation will need to allocate
-		 * blocks.  This happens when the shortform directory lacks
-		 * space and we have to convert it to a block format directory.
-		 * When more blocks are necessary, we must lock the AGI first
-		 * to preserve locking order (AGI -> AGF).
-		 */
-		if (xfs_dir2_sf_replace_needblock(target_dp, src_ip->i_ino)) {
-			error = xfs_read_agi(mp, tp,
-					XFS_INO_TO_AGNO(mp, target_ip->i_ino),
-					&agibp);
-			if (error)
-				goto out_trans_cancel;
-		}
-
 		error = xfs_dir_replace(tp, target_dp, target_name,
 					src_ip->i_ino, spaceres);
 		if (error)



  parent reply	other threads:[~2022-06-03 17:53 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-03 17:42 [PATCH 5.10 00/53] 5.10.120-rc1 review Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.10 01/53] pinctrl: sunxi: fix f1c100s uart2 function Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.10 02/53] percpu_ref_init(): clean ->percpu_count_ref on failure Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.10 03/53] net: af_key: check encryption module availability consistency Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.10 04/53] nfc: pn533: Fix buggy cleanup order Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.10 05/53] net: ftgmac100: Disable hardware checksum on AST2600 Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.10 06/53] i2c: ismt: Provide a DMA buffer for Interrupt Cause Logging Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.10 07/53] drivers: i2c: thunderx: Allow driver to work with ACPI defined TWSI controllers Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.10 08/53] netfilter: nf_tables: disallow non-stateful expression in sets earlier Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.10 09/53] pipe: make poll_usage boolean and annotate its access Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.10 10/53] pipe: Fix missing lock in pipe_resize_ring() Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.10 11/53] cfg80211: set custom regdomain after wiphy registration Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.10 12/53] assoc_array: Fix BUG_ON during garbage collect Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.10 13/53] io_uring: dont re-import iovecs from callbacks Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.10 14/53] io_uring: fix using under-expanded iters Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 15/53] net: ipa: compute proper aggregation limit Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 16/53] xfs: detect overflows in bmbt records Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 17/53] xfs: show the proper user quota options Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 18/53] xfs: fix the forward progress assertion in xfs_iwalk_run_callbacks Greg Kroah-Hartman
2022-06-03 17:43 ` Greg Kroah-Hartman [this message]
2022-06-03 17:43 ` [PATCH 5.10 20/53] xfs: Fix CIL throttle hang when CIL space used going backwards Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 21/53] drm/i915: Fix -Wstringop-overflow warning in call to intel_read_wm_latency() Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 22/53] exfat: check if cluster num is valid Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 23/53] exfat: fix referencing wrong parent directory information after renaming Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 24/53] lib/crypto: add prompts back to crypto libraries Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 25/53] crypto: drbg - prepare for more fine-grained tracking of seeding state Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 26/53] crypto: drbg - track whether DRBG was seeded with !rng_is_initialized() Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 27/53] crypto: drbg - move dynamic ->reseed_threshold adjustments to __drbg_seed() Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 28/53] crypto: drbg - make reseeding from get_random_bytes() synchronous Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 29/53] netfilter: nf_tables: sanitize nft_set_desc_concat_parse() Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 30/53] netfilter: conntrack: re-fetch conntrack after insertion Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 31/53] KVM: PPC: Book3S HV: fix incorrect NULL check on list iterator Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 32/53] x86/kvm: Alloc dummy async #PF token outside of raw spinlock Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 33/53] x86, kvm: use correct GFP flags for preemption disabled Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 34/53] KVM: x86: avoid calling x86 emulator without a decoded instruction Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 35/53] crypto: caam - fix i.MX6SX entropy delay value Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 36/53] crypto: ecrdsa - Fix incorrect use of vli_cmp Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 37/53] zsmalloc: fix races between asynchronous zspage free and page migration Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 38/53] Bluetooth: hci_qca: Use del_timer_sync() before freeing Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 39/53] ARM: dts: s5pv210: Correct interrupt name for bluetooth in Aries Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 40/53] dm integrity: fix error code in dm_integrity_ctr() Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 41/53] dm crypt: make printing of the key constant-time Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 42/53] dm stats: add cond_resched when looping over entries Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 43/53] dm verity: set DM_TARGET_IMMUTABLE feature flag Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 44/53] raid5: introduce MD_BROKEN Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 45/53] HID: multitouch: Add support for Google Whiskers Touchpad Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 46/53] HID: multitouch: add quirks to enable Lenovo X12 trackpoint Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 47/53] tpm: Fix buffer access in tpm2_get_tpm_pt() Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 48/53] tpm: ibmvtpm: Correct the return value in tpm_ibmvtpm_probe() Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 49/53] docs: submitting-patches: Fix crossref to The canonical patch format Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 50/53] NFS: Memory allocation failures are not server fatal errors Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 51/53] NFSD: Fix possible sleep during nfsd4_release_lockowner() Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 52/53] bpf: Fix potential array overflow in bpf_trampoline_get_progs() Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.10 53/53] bpf: Enlarge offset check value to INT_MAX in bpf_skb_{load,store}_bytes Greg Kroah-Hartman
2022-06-04 12:24 ` [PATCH 5.10 00/53] 5.10.120-rc1 review Sudip Mukherjee
2022-06-04 17:23 ` Naresh Kamboju
2022-06-04 18:55 ` Guenter Roeck
2022-06-05  3:03 ` Fox Chen
2022-06-06  1:08 ` Samuel Zou
2022-06-06  8:43 ` Pavel Machek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220603173819.282169316@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=amir73il@gmail.com \
    --cc=bfoster@redhat.com \
    --cc=darrick.wong@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=wlxie7296@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox