linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: NeilBrown <neil@brown.name>
To: Namjae Jeon <linkinjeon@kernel.org>,
	Steve French <smfrench@gmail.com>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	Tom Talpey <tom@talpey.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>,
	Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>,
	linux-fsdevel@vger.kernel.org, linux-cifs@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH 2/4] smb/server: simplify ksmbd_vfs_kern_path_locked()
Date: Mon,  9 Jun 2025 09:35:08 +1000	[thread overview]
Message-ID: <20250608234108.30250-3-neil@brown.name> (raw)
In-Reply-To: <20250608234108.30250-1-neil@brown.name>

ksmbd_vfs_kern_path_locked() first tries to look up the path with the
given case.  When this fails, if caseless is set, it loops over the
components in the path, opening the relevant directory and searching
for a name which matches.  This name is copied over the original and the
the process repeats.  Each time a lookup with the newly updated path is
repeated from the top (vfs_path_lookup()).

When the last component has been case-corrected the simplest next step
is to repeat the original lookup with ksmbd_vfs_path_lookup_locked().
If this works it gives exactly what we want, if it fails it gives the
correct failure.

This observation allows the code to be simplified, in particular
removing the ksmbd_vfs_lock_parent() call.

This patch also removes the duplication of "name" and "filepath" (two
names for the one thing) and calls path_put(parent_path) sooner so
parent_path can be passed directly to vfs_path_lookup avoiding the need
to store it temporarily in "path" and then copying into parent_path.

This patch removes one user of ksmbd_vfs_lock_parent() which will
simplify a future patch.

Signed-off-by: NeilBrown <neil@brown.name>
---
 fs/smb/server/vfs.c | 101 ++++++++++++++++++--------------------------
 1 file changed, 40 insertions(+), 61 deletions(-)

diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
index ba45e809555a..654c2f14a9e6 100644
--- a/fs/smb/server/vfs.c
+++ b/fs/smb/server/vfs.c
@@ -1208,83 +1208,62 @@ static int ksmbd_vfs_lookup_in_dir(const struct path *dir, char *name,
  *
  * Return:	0 on success, otherwise error
  */
-int ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char *name,
+int ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char *filepath,
 			       unsigned int flags, struct path *parent_path,
 			       struct path *path, bool caseless)
 {
 	struct ksmbd_share_config *share_conf = work->tcon->share_conf;
+	size_t path_len, remain_len;
 	int err;
 
-	err = ksmbd_vfs_path_lookup_locked(share_conf, name, flags, parent_path,
+retry:
+	err = ksmbd_vfs_path_lookup_locked(share_conf, filepath, flags, parent_path,
 					   path);
-	if (!err)
-		return 0;
-
-	if (caseless) {
-		char *filepath;
-		size_t path_len, remain_len;
-
-		filepath = name;
-		path_len = strlen(filepath);
-		remain_len = path_len;
-
-		*parent_path = share_conf->vfs_path;
-		path_get(parent_path);
-
-		while (d_can_lookup(parent_path->dentry)) {
-			char *filename = filepath + path_len - remain_len;
-			char *next = strchrnul(filename, '/');
-			size_t filename_len = next - filename;
-			bool is_last = !next[0];
-
-			if (filename_len == 0)
-				break;
+	if (!err || !caseless)
+		return err;
 
-			err = ksmbd_vfs_lookup_in_dir(parent_path, filename,
-						      filename_len,
-						      work->conn->um);
-			if (err)
-				goto out2;
+	path_len = strlen(filepath);
+	remain_len = path_len;
 
-			next[0] = '\0';
+	*parent_path = share_conf->vfs_path;
+	path_get(parent_path);
 
-			err = vfs_path_lookup(share_conf->vfs_path.dentry,
-					      share_conf->vfs_path.mnt,
-					      filepath,
-					      flags,
-					      path);
-			if (!is_last)
-				next[0] = '/';
-			if (err)
-				goto out2;
-			else if (is_last)
-				goto out1;
-			path_put(parent_path);
-			*parent_path = *path;
+	while (d_can_lookup(parent_path->dentry)) {
+		char *filename = filepath + path_len - remain_len;
+		char *next = strchrnul(filename, '/');
+		size_t filename_len = next - filename;
+		bool is_last = !next[0];
 
-			remain_len -= filename_len + 1;
-		}
+		if (filename_len == 0)
+			break;
 
-		err = -EINVAL;
-out2:
+		err = ksmbd_vfs_lookup_in_dir(parent_path, filename,
+					      filename_len,
+					      work->conn->um);
 		path_put(parent_path);
-	}
-
-out1:
-	if (!err) {
-		err = mnt_want_write(parent_path->mnt);
-		if (err) {
-			path_put(path);
-			path_put(parent_path);
-			return err;
+		if (err)
+			goto out;
+		if (is_last) {
+			caseless = false;
+			goto retry;
 		}
+		next[0] = '\0';
+
+		err = vfs_path_lookup(share_conf->vfs_path.dentry,
+				      share_conf->vfs_path.mnt,
+				      filepath,
+				      flags,
+				      parent_path);
+		next[0] = '/';
+		if (err)
+			goto out;
 
-		err = ksmbd_vfs_lock_parent(parent_path->dentry, path->dentry);
-		if (err) {
-			path_put(path);
-			path_put(parent_path);
-		}
+		remain_len -= filename_len + 1;
 	}
+
+	err = -EINVAL;
+	path_put(parent_path);
+out:
 	return err;
 }
 
-- 
2.49.0


  parent reply	other threads:[~2025-06-08 23:48 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-08 23:35 [PATCH 0/4] smb/server: various clean-ups NeilBrown
2025-06-08 23:35 ` [PATCH 1/4] smb/server: use lookup_one_unlocked() NeilBrown
2025-06-08 23:35 ` NeilBrown [this message]
2025-06-08 23:35 ` [PATCH 3/4] smb/server: avoid deadlock when linking with ReplaceIfExists NeilBrown
2025-06-08 23:35 ` [PATCH 4/4] smb/server: add ksmbd_vfs_kern_path() NeilBrown
2025-07-23 15:36   ` Stefan Metzmacher
2025-07-23 23:03     ` NeilBrown
2025-07-23 23:19       ` Namjae Jeon
2025-07-16  5:22 ` [PATCH 0/4] smb/server: various clean-ups NeilBrown
2025-07-16  6:59   ` Namjae Jeon

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=20250608234108.30250-3-neil@brown.name \
    --to=neil@brown.name \
    --cc=brauner@kernel.org \
    --cc=jack@suse.cz \
    --cc=linkinjeon@kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=senozhatsky@chromium.org \
    --cc=smfrench@gmail.com \
    --cc=tom@talpey.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;
as well as URLs for NNTP newsgroup(s).