[PATCH AUTOSEL 6.17] ovl: make sure that ovl_create_real() returns a hashed dentry

[PATCH AUTOSEL 6.17] ovl: make sure that ovl_create_real() returns a hashed dentry

[Sasha Levin]

From: Amir Goldstein <amir73il@gmail.com>

[ Upstream commit ad1423922781e6552f18d055a5742b1cff018cdc ]

e8bd877fb76bb9f3 ("ovl: fix possible double unlink") added a sanity
check of !d_unhashed(child) to try to verify that child dentry was not
unlinked while parent dir was unlocked.

This "was not unlink" check has a false positive result in the case of
casefolded parent dir, because in that case, ovl_create_temp() returns
an unhashed dentry after ovl_create_real() gets an unhashed dentry from
ovl_lookup_upper() and makes it positive.

To avoid returning unhashed dentry from ovl_create_temp(), let
ovl_create_real() lookup again after making the newdentry positive,
so it always returns a hashed positive dentry (or an error).

This fixes the error in ovl_parent_lock() in ovl_check_rename_whiteout()
after ovl_create_temp() and allows mount of overlayfs with casefolding
enabled layers.

Reported-by: André Almeida <andrealmeid@igalia.com>
Closes: https://lore.kernel.org/r/18704e8c-c734-43f3-bc7c-b8be345e1bf5@igalia.com/
Suggested-by: Neil Brown <neil@brown.name>
Reviewed-by: Neil Brown <neil@brown.name>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

YES
- `ovl_parent_lock()` now fails whenever the child dentry is unhashed
  (fs/overlayfs/util.c:1552-1560), which is exactly what happens for
  casefolded workdirs: `ovl_create_temp()` returns the unhashed dentry
  to `ovl_check_rename_whiteout()`, the sanity check at
  `ovl_parent_lock(workdir, temp)` (fs/overlayfs/super.c:575-584) hits
  `-EINVAL`, and overlayfs refuses to mount. That is a major user-
  visible regression caused by the earlier sanity check addition.
- The patch guarantees that `ovl_create_real()` only hands back hashed
  dentries: after the existing error gate (fs/overlayfs/dir.c:215), the
  new block detects `d_unhashed(newdentry)` and re-issues
  `ovl_lookup_upper()` while the parent lock is still held, replacing
  the unhashed instance with a freshly looked-up, hashed, positive
  dentry (fs/overlayfs/dir.c:218-237). This removes the false positive
  from `ovl_parent_lock()` and lets casefolded overlays mount again.
- The extra lookup only runs in the rare unhashed case, uses existing
  helpers, and preserves the previous cleanup path via `dput(newdentry)`
  and error propagation (fs/overlayfs/dir.c:234-239). All direct users
  of `ovl_create_real()`—temp/workdir setup (fs/overlayfs/dir.c:251,
  fs/overlayfs/copy_up.c:550, fs/overlayfs/dir.c:414) and generic upper
  creation (fs/overlayfs/dir.c:362)—benefit without behavioural changes
  elsewhere.
- Scope is limited to overlayfs; no ABI or architectural changes; the
  fix addresses a regression introduced by e8bd877fb76b and restores a
  broken workflow. That is exactly the sort of targeted bug fix we want
  in stable.

Given the severity (overlayfs + casefold mount broken) and the
contained, low-risk fix, this should be backported.

 fs/overlayfs/dir.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index dbd63a74df4b1..039e829aa7dee 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -205,12 +205,32 @@ struct dentry *ovl_create_real(struct ovl_fs *ofs, struct dentry *parent,
 			err = -EPERM;
 		}
 	}
-	if (!err && WARN_ON(!newdentry->d_inode)) {
+	if (err)
+		goto out;
+
+	if (WARN_ON(!newdentry->d_inode)) {
 		/*
 		 * Not quite sure if non-instantiated dentry is legal or not.
 		 * VFS doesn't seem to care so check and warn here.
 		 */
 		err = -EIO;
+	} else if (d_unhashed(newdentry)) {
+		struct dentry *d;
+		/*
+		 * Some filesystems (i.e. casefolded) may return an unhashed
+		 * negative dentry from the ovl_lookup_upper() call before
+		 * ovl_create_real().
+		 * In that case, lookup again after making the newdentry
+		 * positive, so ovl_create_upper() always returns a hashed
+		 * positive dentry.
+		 */
+		d = ovl_lookup_upper(ofs, newdentry->d_name.name, parent,
+				     newdentry->d_name.len);
+		dput(newdentry);
+		if (IS_ERR_OR_NULL(d))
+			err = d ? PTR_ERR(d) : -ENOENT;
+		else
+			return d;
 	}
 out:
 	if (err) {
-- 
2.51.0