From: NeilBrown <neilb@ownmail.net>
To: Christian Brauner <brauner@kernel.org>,
Alexander Viro <viro@zeniv.linux.org.uk>,
David Howells <dhowells@redhat.com>, Jan Kara <jack@suse.cz>,
Chuck Lever <chuck.lever@oracle.com>,
Jeff Layton <jlayton@kernel.org>,
Miklos Szeredi <miklos@szeredi.hu>,
Amir Goldstein <amir73il@gmail.com>,
John Johansen <john.johansen@canonical.com>,
Paul Moore <paul@paul-moore.com>,
James Morris <jmorris@namei.org>,
"Serge E. Hallyn" <serge@hallyn.com>,
Stephen Smalley <stephen.smalley.work@gmail.com>
Cc: linux-kernel@vger.kernel.org, netfs@lists.linux.dev,
linux-fsdevel@vger.kernel.org, linux-nfs@vger.kernel.org,
linux-unionfs@vger.kernel.org, apparmor@lists.ubuntu.com,
linux-security-module@vger.kernel.org, selinux@vger.kernel.org
Subject: [PATCH 08/13] ovl: Simplify ovl_lookup_real_one()
Date: Wed, 4 Feb 2026 15:57:52 +1100 [thread overview]
Message-ID: <20260204050726.177283-9-neilb@ownmail.net> (raw)
In-Reply-To: <20260204050726.177283-1-neilb@ownmail.net>
From: NeilBrown <neil@brown.name>
The primary purpose of this patch is to remove the locking from
ovl_lookup_real_one() as part of centralising all locking of directories
for name operations.
The locking here isn't needed. By performing consistency tests after
the lookup we can be sure that the result of the lookup was valid at
least for a moment, which is all the original code promised.
lookup_noperm_unlocked() is used for the lookup and it will take the
lock if needed only where it is needed.
Also:
- don't take a reference to real->d_parent. The parent is
only use for a pointer comparison, and no reference is needed for
that.
- Several "if" statements have a "goto" followed by "else" - the
else isn't needed: the following statement can directly follow
the "if" as a new statement
- Use a consistent pattern of setting "err" before performing a test
and possibly going to "fail".
- remove the "out" label (now that we don't need to dput(parent) or
unlock) and simply return from fail:.
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/overlayfs/export.c | 61 ++++++++++++++++++-------------------------
1 file changed, 26 insertions(+), 35 deletions(-)
diff --git a/fs/overlayfs/export.c b/fs/overlayfs/export.c
index 83f80fdb1567..dcd28ffc4705 100644
--- a/fs/overlayfs/export.c
+++ b/fs/overlayfs/export.c
@@ -359,59 +359,50 @@ static struct dentry *ovl_lookup_real_one(struct dentry *connected,
struct dentry *real,
const struct ovl_layer *layer)
{
- struct inode *dir = d_inode(connected);
- struct dentry *this, *parent = NULL;
+ struct dentry *this;
struct name_snapshot name;
int err;
/*
- * Lookup child overlay dentry by real name. The dir mutex protects us
- * from racing with overlay rename. If the overlay dentry that is above
- * real has already been moved to a parent that is not under the
- * connected overlay dir, we return -ECHILD and restart the lookup of
- * connected real path from the top.
+ * @connected is a directory in the overlay and @real is an object
+ * on @layer which is expected to be a child of @connected.
+ * The goal is to return a dentry from the overlay which corresponds
+ * to @real. This is done by looking up the name from @real in
+ * @connected and checking that the result meets expectations.
+ *
+ * Return %-ECHILD if the parent of @real no-longer corresponds to
+ * @connected, and %-ESTALE if the dentry found by lookup doesn't
+ * correspond to @real.
*/
- inode_lock_nested(dir, I_MUTEX_PARENT);
- err = -ECHILD;
- parent = dget_parent(real);
- if (ovl_dentry_real_at(connected, layer->idx) != parent)
- goto fail;
- /*
- * We also need to take a snapshot of real dentry name to protect us
- * from racing with underlying layer rename. In this case, we don't
- * care about returning ESTALE, only from dereferencing a free name
- * pointer because we hold no lock on the real dentry.
- */
take_dentry_name_snapshot(&name, real);
- /*
- * No idmap handling here: it's an internal lookup.
- */
- this = lookup_noperm(&name.name, connected);
+ this = lookup_noperm_unlocked(&name.name, connected);
release_dentry_name_snapshot(&name);
+
+ err = -ECHILD;
+ if (ovl_dentry_real_at(connected, layer->idx) != real->d_parent)
+ goto fail;
+
err = PTR_ERR(this);
- if (IS_ERR(this)) {
+ if (IS_ERR(this))
goto fail;
- } else if (!this || !this->d_inode) {
- dput(this);
- err = -ENOENT;
+
+ err = -ENOENT;
+ if (!this || !this->d_inode)
goto fail;
- } else if (ovl_dentry_real_at(this, layer->idx) != real) {
- dput(this);
- err = -ESTALE;
+
+ err = -ESTALE;
+ if (ovl_dentry_real_at(this, layer->idx) != real)
goto fail;
- }
-out:
- dput(parent);
- inode_unlock(dir);
return this;
fail:
pr_warn_ratelimited("failed to lookup one by real (%pd2, layer=%d, connected=%pd2, err=%i)\n",
real, layer->idx, connected, err);
- this = ERR_PTR(err);
- goto out;
+ if (!IS_ERR(this))
+ dput(this);
+ return ERR_PTR(err);
}
static struct dentry *ovl_lookup_real(struct super_block *sb,
--
2.50.0.107.gf914562f5916.dirty
next prev parent reply other threads:[~2026-02-04 5:09 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-04 4:57 [PATCH 00/13] Further centralising of directory locking for name ops NeilBrown
2026-02-04 4:57 ` [PATCH 01/13] fs/proc: Don't lock root inode when creating "self" and "thread-self" NeilBrown
2026-02-05 12:33 ` Jeff Layton
2026-02-04 4:57 ` [PATCH 02/13] VFS: move the start_dirop() kerndoc comment to before start_dirop() NeilBrown
2026-02-05 12:33 ` Jeff Layton
2026-02-04 4:57 ` [PATCH 03/13] libfs: change simple_done_creating() to use end_creating() NeilBrown
2026-02-05 12:19 ` Jeff Layton
2026-02-06 2:13 ` NeilBrown
2026-02-04 4:57 ` [PATCH 04/13] Apparmor: Use simple_start_creating() / simple_done_creating() NeilBrown
2026-02-04 10:58 ` kernel test robot
2026-02-05 12:58 ` Jeff Layton
2026-02-06 0:21 ` NeilBrown
2026-02-04 4:57 ` [PATCH 05/13] selinux: " NeilBrown
2026-02-05 12:36 ` Jeff Layton
2026-02-20 22:47 ` Paul Moore
2026-02-21 22:28 ` NeilBrown
2026-02-22 14:19 ` Paul Moore
2026-02-23 0:58 ` NeilBrown
2026-02-04 4:57 ` [PATCH 06/13] nfsd: switch purge_old() to use start_removing_noperm() NeilBrown
2026-02-05 12:36 ` Jeff Layton
2026-02-04 4:57 ` [PATCH 07/13] VFS: make lookup_one_qstr_excl() static NeilBrown
2026-02-05 12:37 ` Jeff Layton
2026-02-04 4:57 ` NeilBrown [this message]
2026-02-05 12:38 ` [PATCH 08/13] ovl: Simplify ovl_lookup_real_one() Jeff Layton
2026-02-05 13:04 ` Amir Goldstein
2026-02-06 0:43 ` NeilBrown
2026-02-04 4:57 ` [PATCH 09/13] cachefiles: change cachefiles_bury_object to use start_renaming_dentry() NeilBrown
2026-02-05 12:38 ` Jeff Layton
2026-02-04 4:57 ` [PATCH 10/13] ovl: change ovl_create_real() to get a new lock when re-opening created file NeilBrown
2026-02-05 12:26 ` Amir Goldstein
2026-02-06 1:11 ` NeilBrown
2026-02-06 13:35 ` Amir Goldstein
2026-02-04 4:57 ` [PATCH 11/13] ovl: use is_subdir() for testing if one thing is a subdir of another NeilBrown
2026-02-05 9:37 ` Amir Goldstein
2026-02-05 12:38 ` Jeff Layton
2026-02-04 4:57 ` [PATCH 12/13] ovl: remove ovl_lock_rename_workdir() NeilBrown
2026-02-05 9:45 ` Amir Goldstein
2026-02-06 1:18 ` NeilBrown
2026-02-05 12:40 ` Jeff Layton
2026-02-05 12:58 ` Jeff Layton
2026-02-04 4:57 ` [PATCH 13/13] VFS: unexport lock_rename(), lock_rename_child(), unlock_rename() NeilBrown
2026-02-05 12:41 ` Jeff Layton
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=20260204050726.177283-9-neilb@ownmail.net \
--to=neilb@ownmail.net \
--cc=amir73il@gmail.com \
--cc=apparmor@lists.ubuntu.com \
--cc=brauner@kernel.org \
--cc=chuck.lever@oracle.com \
--cc=dhowells@redhat.com \
--cc=jack@suse.cz \
--cc=jlayton@kernel.org \
--cc=jmorris@namei.org \
--cc=john.johansen@canonical.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=linux-unionfs@vger.kernel.org \
--cc=miklos@szeredi.hu \
--cc=neil@brown.name \
--cc=netfs@lists.linux.dev \
--cc=paul@paul-moore.com \
--cc=selinux@vger.kernel.org \
--cc=serge@hallyn.com \
--cc=stephen.smalley.work@gmail.com \
--cc=viro@zeniv.linux.org.uk \
/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