linux-kernel - [PATCH 2/4] smb/server: simplify ksmbd_vfs_kern_path

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250608234108.30250-3-neil@brown.name>
Date: Mon,  9 Jun 2025 09:35:08 +1000
From: NeilBrown <neil@...wn.name>
To: Namjae Jeon <linkinjeon@...nel.org>,
	Steve French <smfrench@...il.com>,
	Sergey Senozhatsky <senozhatsky@...omium.org>,
	Tom Talpey <tom@...pey.com>
Cc: Alexander Viro <viro@...iv.linux.org.uk>,
	Christian Brauner <brauner@...nel.org>,
	Jan Kara <jack@...e.cz>,
	linux-fsdevel@...r.kernel.org,
	linux-cifs@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH 2/4] smb/server: simplify ksmbd_vfs_kern_path_locked()

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@...wn.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