Skip to contentSkip to conversation

ksmbd patches included vfs changes

PATCH In review Namjae Jeon 5 participants 14 messages Mar 15, 2023 – Jun 13, 2023

Subsystems (3)
VFS SMB client (CIFS) Kernel SMB3 server (ksmbd)
lore
Share & tools
lore

Namjae Jeon

author maintainer
#0 lore raw

v8 cover ksmbd patches included vfs changes

v8:
- Don't call vfs_path_lookup() to avoid repeat lookup, Instead, lookup last component after locking the parent that got from vfs_path_parent_lookup helper.
v7:
- constify struct path.
- recreate patch-set base on recent Al's patches.
v6:
- rename __lookup_hash() to lookup_one_qstr_excl and export.
- change dget() to dget_parent() in unlink.
- lock parent of open file in smb2_open() to make file_present worthable.
v5:
- add lock_rename_child() helper.
- remove d_is_symlink() check for new_path.dentry.
- use lock_rename_child() helper instead of lock_rename().
- use dget() instead of dget_parent().
- check that old_child is still hashed.
- directly check child->parent instead of using take_dentry_name_snapshot().
v4:
- switch the order of 3/4 and 4/4 patch.
- fix vfs_path_parent_lookup() parameter description mismatch.
v3:
- use dget_parent + take_dentry_name_snapshot() to check stability of source rename in smb2_vfs_rename().
v2:
- add filename_lock to avoid racy issue from fp->filename. (Sergey Senozhatsky)
- fix warning: variable 'old_dentry' is used uninitialized (kernel test robot)

Al Viro (1):
fs: introduce lock_rename_child() helper

Namjae Jeon (2):
ksmbd: remove internal.h include
ksmbd: fix racy issue from using ->d_parent and ->d_name

 fs/internal.h         |   2 -
 fs/ksmbd/smb2pdu.c    | 147 ++++----------
 fs/ksmbd/vfs.c        | 435 ++++++++++++++++++------------------------
 fs/ksmbd/vfs.h        |  19 +-
 fs/ksmbd/vfs_cache.c  |   5 +-
 fs/namei.c            | 125 +++++++++---
 include/linux/namei.h |   9 +
 7 files changed, 342 insertions(+), 400 deletions(-)
Signature · 1 line
2.25.1
author maintainer
#1 lore raw

PATCH v8 1/3 ksmbd: remove internal.h include

Since vfs_path_lookup is exported, It should not be internal. Move vfs_path_lookup prototype in internal.h to linux/namei.h.

Suggested-by Al Viro <[email protected]>
Signed-off-by Namjae Jeon <[email protected]>
---
 fs/internal.h         | 2 --
 fs/ksmbd/vfs.c        | 2 --
 include/linux/namei.h | 2 ++
 3 files changed, 2 insertions(+), 4 deletions(-)
fs/internal.h +0 -2
index dc4eb91a577a..071a7517f1a7 100644
@@ -59,8 +59,6 @@ extern int finish_clean_context(struct fs_context *fc);
  */
 extern int filename_lookup(int dfd, struct filename *name, unsigned flags,
 			   struct path *path, struct path *root);
-extern int vfs_path_lookup(struct dentry *, struct vfsmount *,
-			   const char *, unsigned int, struct path *);
 int do_rmdir(int dfd, struct filename *name);
 int do_unlinkat(int dfd, struct filename *name);
 int may_linkat(struct mnt_idmap *idmap, const struct path *link);
fs/ksmbd/vfs.c +0 -2
index 5ea9229dad2c..cef07d7fb7dc 100644
@@ -19,8 +19,6 @@
 #include <linux/sched/xacct.h>
 #include <linux/crc32c.h>
 
-#include "../internal.h"	/* for vfs_path_lookup */
-
 #include "glob.h"
 #include "oplock.h"
 #include "connection.h"
include/linux/namei.h +2 -0
index 0d4531fd46e7..c2747cfe97ac 100644
@@ -63,6 +63,8 @@ extern struct dentry *kern_path_create(int, const char *, struct path *, unsigne
 extern struct dentry *user_path_create(int, const char __user *, struct path *, unsigned int);
 extern void done_path_create(struct path *, struct dentry *);
 extern struct dentry *kern_path_locked(const char *, struct path *);
+int vfs_path_lookup(struct dentry *, struct vfsmount *, const char *,
+		    unsigned int, struct path *);
 
 extern struct dentry *try_lookup_one_len(const char *, struct dentry *, int);
 extern struct dentry *lookup_one_len(const char *, struct dentry *, int);
Signature · 1 line
2.25.1
author maintainer
#3 lore raw

PATCH v8 2/3 fs: introduce lock_rename_child() helper

In reply to Namjae Jeon

From: Al Viro <[email protected]>

Pass the dentry of a source file and the dentry of a destination directory to lock parent inodes for rename. As soon as this function returns, ->d_parent of the source file dentry is stable and inodes are properly locked for calling vfs-rename. This helper is needed for ksmbd server. rename request of SMB protocol has to rename an opened file, no matter which directory it's in.

Signed-off-by Al Viro <[email protected]>
Signed-off-by Namjae Jeon <[email protected]>
---
 fs/namei.c            | 68 ++++++++++++++++++++++++++++++++++++-------
 include/linux/namei.h |  1 +
 2 files changed, 58 insertions(+), 11 deletions(-)
fs/namei.c +57 -11
index 5855dc6edbd5..984d4f7b47dc 100644
@@ -2980,20 +2980,10 @@ static inline int may_create(struct mnt_idmap *idmap,
 	return inode_permission(idmap, dir, MAY_WRITE | MAY_EXEC);
 }
 
-/*
- * p1 and p2 should be directories on the same fs.
- */
-struct dentry *lock_rename(struct dentry *p1, struct dentry *p2)
+static struct dentry *lock_two_directories(struct dentry *p1, struct dentry *p2)
 {
 	struct dentry *p;
 
-	if (p1 == p2) {
-		inode_lock_nested(p1->d_inode, I_MUTEX_PARENT);
-		return NULL;
-	}
-
-	mutex_lock(&p1->d_sb->s_vfs_rename_mutex);
-
 	p = d_ancestor(p2, p1);
 	if (p) {
 		inode_lock_nested(p2->d_inode, I_MUTEX_PARENT);

@@ -3012,8 +3002,64 @@ struct dentry *lock_rename(struct dentry *p1, struct dentry *p2)
 	inode_lock_nested(p2->d_inode, I_MUTEX_PARENT2);
 	return NULL;
 }
+
+/*
+ * p1 and p2 should be directories on the same fs.
+ */
+struct dentry *lock_rename(struct dentry *p1, struct dentry *p2)
+{
+	if (p1 == p2) {
+		inode_lock_nested(p1->d_inode, I_MUTEX_PARENT);
+		return NULL;
+	}
+
+	mutex_lock(&p1->d_sb->s_vfs_rename_mutex);
+	return lock_two_directories(p1, p2);
+}
 EXPORT_SYMBOL(lock_rename);
 
+/*
+ * c1 and p2 should be on the same fs.
+ */
+struct dentry *lock_rename_child(struct dentry *c1, struct dentry *p2)
+{
+	if (READ_ONCE(c1->d_parent) == p2) {
+		/*
+		 * hopefully won't need to touch ->s_vfs_rename_mutex at all.
+		 */
+		inode_lock_nested(p2->d_inode, I_MUTEX_PARENT);
+		/*
+		 * now that p2 is locked, nobody can move in or out of it,
+		 * so the test below is safe.
+		 */
+		if (likely(c1->d_parent == p2))
+			return NULL;
+
+		/*
+		 * c1 got moved out of p2 while we'd been taking locks;
+		 * unlock and fall back to slow case.
+		 */
+		inode_unlock(p2->d_inode);
+	}
+
+	mutex_lock(&c1->d_sb->s_vfs_rename_mutex);
+	/*
+	 * nobody can move out of any directories on this fs.
+	 */
+	if (likely(c1->d_parent != p2))
+		return lock_two_directories(c1->d_parent, p2);
+
+	/*
+	 * c1 got moved into p2 while we were taking locks;
+	 * we need p2 locked and ->s_vfs_rename_mutex unlocked,
+	 * for consistency with lock_rename().
+	 */
+	inode_lock_nested(p2->d_inode, I_MUTEX_PARENT);
+	mutex_unlock(&c1->d_sb->s_vfs_rename_mutex);
+	return NULL;
+}
+EXPORT_SYMBOL(lock_rename_child);
+
 void unlock_rename(struct dentry *p1, struct dentry *p2)
 {
 	inode_unlock(p1->d_inode);
include/linux/namei.h +1 -0
index c2747cfe97ac..4c070fb29a91 100644
@@ -83,6 +83,7 @@ extern int follow_down(struct path *);
 extern int follow_up(struct path *);
 
 extern struct dentry *lock_rename(struct dentry *, struct dentry *);
+extern struct dentry *lock_rename_child(struct dentry *, struct dentry *);
 extern void unlock_rename(struct dentry *, struct dentry *);
 
 extern int __must_check nd_jump_link(const struct path *path);
Signature · 1 line
2.25.1
build bot
#4 lore raw

[lkp] [+118 bytes kernel size regression] [i386-tinyconfig] [6e6937ac88] fs: introduce lock_rename_child() helper

FYI, we noticed a +118 bytes kernel size regression due to commit:

FYI, we noticed a +118 bytes kernel size regression due to commit:

commit: 6e6937ac882828b632bbcecf35ce35d280a2b4fb (fs: introduce lock_rename_child() helper)
url: https://github.com/intel-lab-lkp/linux/commits/Namjae-Jeon/ksmbd-remove-internal-h-include/20230316-063731
base: https://git.kernel.org/cgit/linux/kernel/git/vfs/idmapping.git for-next
patch link: https://lore.kernel.org/all/[email protected]/
patch subject: [PATCH v8 2/3] fs: introduce lock_rename_child() helper

Details as below (size data is obtained by nm --size-sort vmlinux):

2d2e9d60: ksmbd: remove internal.h include 6e6937ac: fs: introduce lock_rename_child() helper

+---------------------------+----------+----------+-------+
|          symbol           | 2d2e9d60 | 6e6937ac | delta |
+---------------------------+----------+----------+-------+
| nm.T.lock_rename_child    | 0        | 98       | 98    |
| nm.t.lock_two_directories | 0        | 77       | 77    |
| bzImage                   | 501536   | 501600   | 64    |
| nm.T.lock_rename          | 109      | 52       | -57   |
+---------------------------+----------+----------+-------+

Thanks

author maintainer
#5 lore raw

PATCH v8 3/3 ksmbd: fix racy issue from using ->d_parent and ->d_name

In reply to Namjae Jeon

Al pointed out that ksmbd has racy issue from using ->d_parent and ->d_name in ksmbd_vfs_unlink and smb2_vfs_rename(). and use new lock_rename_child() to lock stable parent while underlying rename racy.
Introduce vfs_path_parent_lookup helper to avoid out of share access and export vfs functions like the following ones to use vfs_path_parent_lookup().
- rename __lookup_hash() to lookup_one_qstr_excl().
- export lookup_one_qstr_excl().
- export getname_kernel() and putname().

vfs_path_parent_lookup() is used for parent lookup of destination file using absolute pathname given from FILE_RENAME_INFORMATION request.

Signed-off-by Namjae Jeon <[email protected]>
---
 fs/ksmbd/smb2pdu.c    | 147 ++++----------
 fs/ksmbd/vfs.c        | 433 ++++++++++++++++++------------------------
 fs/ksmbd/vfs.h        |  19 +-
 fs/ksmbd/vfs_cache.c  |   5 +-
 fs/namei.c            |  57 ++++--
 include/linux/namei.h |   6 +
 6 files changed, 282 insertions(+), 385 deletions(-)
fs/ksmbd/smb2pdu.c +35 -112
index 547a10977454..e27f86b03ad4 100644
@@ -2424,7 +2424,7 @@ static int smb2_creat(struct ksmbd_work *work, struct path *path, char *name,
 			return rc;
 	}
 
-	rc = ksmbd_vfs_kern_path(work, name, 0, path, 0);
+	rc = ksmbd_vfs_kern_path_locked(work, name, 0, path, 0);
 	if (rc) {
 		pr_err("cannot get linux path (%s), err = %d\n",
 		       name, rc);

@@ -2715,8 +2715,10 @@ int smb2_open(struct ksmbd_work *work)
 		goto err_out1;
 	}
 
-	rc = ksmbd_vfs_kern_path(work, name, LOOKUP_NO_SYMLINKS, &path, 1);
+	rc = ksmbd_vfs_kern_path_locked(work, name, LOOKUP_NO_SYMLINKS, &path, 1);
 	if (!rc) {
+		file_present = true;
+
 		if (req->CreateOptions & FILE_DELETE_ON_CLOSE_LE) {
 			/*
 			 * If file exists with under flags, return access

@@ -2725,7 +2727,6 @@ int smb2_open(struct ksmbd_work *work)
 			if (req->CreateDisposition == FILE_OVERWRITE_IF_LE ||
 			    req->CreateDisposition == FILE_OPEN_IF_LE) {
 				rc = -EACCES;
-				path_put(&path);
 				goto err_out;
 			}
 

@@ -2733,26 +2734,23 @@ int smb2_open(struct ksmbd_work *work)
 				ksmbd_debug(SMB,
 					    "User does not have write permission\n");
 				rc = -EACCES;
-				path_put(&path);
 				goto err_out;
 			}
 		} else if (d_is_symlink(path.dentry)) {
 			rc = -EACCES;
-			path_put(&path);
 			goto err_out;
 		}
-	}
 
-	if (rc) {
+		file_present = true;
+		idmap = mnt_idmap(path.mnt);
+	} else {
 		if (rc != -ENOENT)
 			goto err_out;
 		ksmbd_debug(SMB, "can not get linux path for %s, rc = %d\n",
 			    name, rc);
 		rc = 0;
-	} else {
-		file_present = true;
-		idmap = mnt_idmap(path.mnt);
 	}
+
 	if (stream_name) {
 		if (req->CreateOptions & FILE_DIRECTORY_FILE_LE) {
 			if (s_type == DATA_STREAM) {

@@ -2880,8 +2878,9 @@ int smb2_open(struct ksmbd_work *work)
 
 			if ((daccess & FILE_DELETE_LE) ||
 			    (req->CreateOptions & FILE_DELETE_ON_CLOSE_LE)) {
-				rc = ksmbd_vfs_may_delete(idmap,
-							  path.dentry);
+				rc = inode_permission(idmap,
+						      d_inode(path.dentry->d_parent),
+						      MAY_EXEC | MAY_WRITE);
 				if (rc)
 					goto err_out;
 			}

@@ -3252,10 +3251,13 @@ int smb2_open(struct ksmbd_work *work)
 	}
 
 err_out:
-	if (file_present || created)
-		path_put(&path);
+	if (file_present || created) {
+		inode_unlock(d_inode(path.dentry->d_parent));
+		dput(path.dentry);
+	}
 	ksmbd_revert_fsids(work);
 err_out1:
+
 	if (rc) {
 		if (rc == -EINVAL)
 			rsp->hdr.Status = STATUS_INVALID_PARAMETER;

@@ -5406,44 +5408,19 @@ int smb2_echo(struct ksmbd_work *work)
 
 static int smb2_rename(struct ksmbd_work *work,
 		       struct ksmbd_file *fp,
-		       struct mnt_idmap *idmap,
 		       struct smb2_file_rename_info *file_info,
 		       struct nls_table *local_nls)
 {
 	struct ksmbd_share_config *share = fp->tcon->share_conf;
-	char *new_name = NULL, *abs_oldname = NULL, *old_name = NULL;
-	char *pathname = NULL;
-	struct path path;
-	bool file_present = true;
-	int rc;
+	char *new_name = NULL;
+	int rc, flags = 0;
 
 	ksmbd_debug(SMB, "setting FILE_RENAME_INFO\n");
-	pathname = kmalloc(PATH_MAX, GFP_KERNEL);
-	if (!pathname)
-		return -ENOMEM;
-
-	abs_oldname = file_path(fp->filp, pathname, PATH_MAX);
-	if (IS_ERR(abs_oldname)) {
-		rc = -EINVAL;
-		goto out;
-	}
-	old_name = strrchr(abs_oldname, '/');
-	if (old_name && old_name[1] != '\0') {
-		old_name++;
-	} else {
-		ksmbd_debug(SMB, "can't get last component in path %s\n",
-			    abs_oldname);
-		rc = -ENOENT;
-		goto out;
-	}
-
 	new_name = smb2_get_name(file_info->FileName,
 				 le32_to_cpu(file_info->FileNameLength),
 				 local_nls);
-	if (IS_ERR(new_name)) {
-		rc = PTR_ERR(new_name);
-		goto out;
-	}
+	if (IS_ERR(new_name))
+		return PTR_ERR(new_name);
 
 	if (strchr(new_name, ':')) {
 		int s_type;

@@ -5469,7 +5446,7 @@ static int smb2_rename(struct ksmbd_work *work,
 		if (rc)
 			goto out;
 
-		rc = ksmbd_vfs_setxattr(idmap,
+		rc = ksmbd_vfs_setxattr(file_mnt_idmap(fp->filp),
 					fp->filp->f_path.dentry,
 					xattr_stream_name,
 					NULL, 0, 0);

@@ -5484,47 +5461,18 @@ static int smb2_rename(struct ksmbd_work *work,
 	}
 
 	ksmbd_debug(SMB, "new name %s\n", new_name);
-	rc = ksmbd_vfs_kern_path(work, new_name, LOOKUP_NO_SYMLINKS, &path, 1);
-	if (rc) {
-		if (rc != -ENOENT)
-			goto out;
-		file_present = false;
-	} else {
-		path_put(&path);
-	}
-
 	if (ksmbd_share_veto_filename(share, new_name)) {
 		rc = -ENOENT;
 		ksmbd_debug(SMB, "Can't rename vetoed file: %s\n", new_name);
 		goto out;
 	}
 
-	if (file_info->ReplaceIfExists) {
-		if (file_present) {
-			rc = ksmbd_vfs_remove_file(work, new_name);
-			if (rc) {
-				if (rc != -ENOTEMPTY)
-					rc = -EINVAL;
-				ksmbd_debug(SMB, "cannot delete %s, rc %d\n",
-					    new_name, rc);
-				goto out;
-			}
-		}
-	} else {
-		if (file_present &&
-		    strncmp(old_name, path.dentry->d_name.name, strlen(old_name))) {
-			rc = -EEXIST;
-			ksmbd_debug(SMB,
-				    "cannot rename already existing file\n");
-			goto out;
-		}
-	}
+	if (!file_info->ReplaceIfExists)
+		flags = RENAME_NOREPLACE;
 
-	rc = ksmbd_vfs_fp_rename(work, fp, new_name);
+	rc = ksmbd_vfs_rename(work, &fp->filp->f_path, new_name, flags);
 out:
-	kfree(pathname);
-	if (!IS_ERR(new_name))
-		kfree(new_name);
+	kfree(new_name);
 	return rc;
 }
 

@@ -5564,18 +5512,17 @@ static int smb2_create_link(struct ksmbd_work *work,
 	}
 
 	ksmbd_debug(SMB, "target name is %s\n", target_name);
-	rc = ksmbd_vfs_kern_path(work, link_name, LOOKUP_NO_SYMLINKS, &path, 0);
+	rc = ksmbd_vfs_kern_path_locked(work, link_name, LOOKUP_NO_SYMLINKS,
+					&path, 0);
 	if (rc) {
 		if (rc != -ENOENT)
 			goto out;
 		file_present = false;
-	} else {
-		path_put(&path);
 	}
 
 	if (file_info->ReplaceIfExists) {
 		if (file_present) {
-			rc = ksmbd_vfs_remove_file(work, link_name);
+			rc = ksmbd_vfs_remove_file(work, &path);
 			if (rc) {
 				rc = -EINVAL;
 				ksmbd_debug(SMB, "cannot delete %s\n",

@@ -5595,6 +5542,10 @@ static int smb2_create_link(struct ksmbd_work *work,
 	if (rc)
 		rc = -EINVAL;
 out:
+	if (file_present) {
+		inode_unlock(d_inode(path.dentry->d_parent));
+		path_put(&path);
+	}
 	if (!IS_ERR(link_name))
 		kfree(link_name);
 	kfree(pathname);

@@ -5772,12 +5723,6 @@ static int set_rename_info(struct ksmbd_work *work, struct ksmbd_file *fp,
 			   struct smb2_file_rename_info *rename_info,
 			   unsigned int buf_len)
 {
-	struct mnt_idmap *idmap;
-	struct ksmbd_file *parent_fp;
-	struct dentry *parent;
-	struct dentry *dentry = fp->filp->f_path.dentry;
-	int ret;
-
 	if (!(fp->daccess & FILE_DELETE_LE)) {
 		pr_err("no right to delete : 0x%x\n", fp->daccess);
 		return -EACCES;

@@ -5787,32 +5732,10 @@ static int set_rename_info(struct ksmbd_work *work, struct ksmbd_file *fp,
 			le32_to_cpu(rename_info->FileNameLength))
 		return -EINVAL;
 
-	idmap = file_mnt_idmap(fp->filp);
-	if (ksmbd_stream_fd(fp))
-		goto next;
-
-	parent = dget_parent(dentry);
-	ret = ksmbd_vfs_lock_parent(idmap, parent, dentry);
-	if (ret) {
-		dput(parent);
-		return ret;
-	}
-
-	parent_fp = ksmbd_lookup_fd_inode(d_inode(parent));
-	inode_unlock(d_inode(parent));
-	dput(parent);
+	if (!le32_to_cpu(rename_info->FileNameLength))
+		return -EINVAL;
 
-	if (parent_fp) {
-		if (parent_fp->daccess & FILE_DELETE_LE) {
-			pr_err("parent dir is opened with delete access\n");
-			ksmbd_fd_put(work, parent_fp);
-			return -ESHARE;
-		}
-		ksmbd_fd_put(work, parent_fp);
-	}
-next:
-	return smb2_rename(work, fp, idmap, rename_info,
-			   work->conn->local_nls);
+	return smb2_rename(work, fp, rename_info, work->conn->local_nls);
 }
 
 static int set_file_disposition_info(struct ksmbd_file *fp,
fs/ksmbd/vfs.c +187 -246

587 lines not on this page · raw message

fs/ksmbd/vfs.h +8 -11
index 9d676ab0cd25..a4ae89f3230d 100644
@@ -71,9 +71,7 @@ struct ksmbd_kstat {
 	__le32			file_attributes;
 };
 
-int ksmbd_vfs_lock_parent(struct mnt_idmap *idmap, struct dentry *parent,
-			  struct dentry *child);
-int ksmbd_vfs_may_delete(struct mnt_idmap *idmap, struct dentry *dentry);
+int ksmbd_vfs_lock_parent(struct dentry *parent, struct dentry *child);
 int ksmbd_vfs_query_maximal_access(struct mnt_idmap *idmap,
 				   struct dentry *dentry, __le32 *daccess);
 int ksmbd_vfs_create(struct ksmbd_work *work, const char *name, umode_t mode);

@@ -84,12 +82,12 @@ int ksmbd_vfs_write(struct ksmbd_work *work, struct ksmbd_file *fp,
 		    char *buf, size_t count, loff_t *pos, bool sync,
 		    ssize_t *written);
 int ksmbd_vfs_fsync(struct ksmbd_work *work, u64 fid, u64 p_id);
-int ksmbd_vfs_remove_file(struct ksmbd_work *work, char *name);
+int ksmbd_vfs_remove_file(struct ksmbd_work *work, const struct path *path);
 int ksmbd_vfs_link(struct ksmbd_work *work,
 		   const char *oldname, const char *newname);
 int ksmbd_vfs_getattr(const struct path *path, struct kstat *stat);
-int ksmbd_vfs_fp_rename(struct ksmbd_work *work, struct ksmbd_file *fp,
-			char *newname);
+int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path,
+		     char *newname, int flags);
 int ksmbd_vfs_truncate(struct ksmbd_work *work,
 		       struct ksmbd_file *fp, loff_t size);
 struct srv_copychunk;

@@ -116,9 +114,9 @@ int ksmbd_vfs_xattr_stream_name(char *stream_name, char **xattr_stream_name,
 				size_t *xattr_stream_name_size, int s_type);
 int ksmbd_vfs_remove_xattr(struct mnt_idmap *idmap,
 			   struct dentry *dentry, char *attr_name);
-int ksmbd_vfs_kern_path(struct ksmbd_work *work,
-			char *name, unsigned int flags, struct path *path,
-			bool caseless);
+int ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char *name,
+			       unsigned int flags, struct path *path,
+			       bool caseless);
 struct dentry *ksmbd_vfs_kern_path_create(struct ksmbd_work *work,
 					  const char *name,
 					  unsigned int flags,

@@ -131,8 +129,7 @@ struct file_allocated_range_buffer;
 int ksmbd_vfs_fqar_lseek(struct ksmbd_file *fp, loff_t start, loff_t length,
 			 struct file_allocated_range_buffer *ranges,
 			 unsigned int in_count, unsigned int *out_count);
-int ksmbd_vfs_unlink(struct mnt_idmap *idmap, struct dentry *dir,
-		     struct dentry *dentry);
+int ksmbd_vfs_unlink(struct file *filp);
 void *ksmbd_vfs_init_kstat(char **p, struct ksmbd_kstat *ksmbd_kstat);
 int ksmbd_vfs_fill_dentry_attrs(struct ksmbd_work *work,
 				struct mnt_idmap *idmap,
fs/ksmbd/vfs_cache.c +1 -4
index 054a7d2e0f48..2d0138e72d78 100644
@@ -244,7 +244,6 @@ void ksmbd_release_inode_hash(void)
 
 static void __ksmbd_inode_close(struct ksmbd_file *fp)
 {
-	struct dentry *dir, *dentry;
 	struct ksmbd_inode *ci = fp->f_ci;
 	int err;
 	struct file *filp;

@@ -263,11 +262,9 @@ static void __ksmbd_inode_close(struct ksmbd_file *fp)
 	if (atomic_dec_and_test(&ci->m_count)) {
 		write_lock(&ci->m_lock);
 		if (ci->m_flags & (S_DEL_ON_CLS | S_DEL_PENDING)) {
-			dentry = filp->f_path.dentry;
-			dir = dentry->d_parent;
 			ci->m_flags &= ~(S_DEL_ON_CLS | S_DEL_PENDING);
 			write_unlock(&ci->m_lock);
-			ksmbd_vfs_unlink(file_mnt_idmap(filp), dir, dentry);
+			ksmbd_vfs_unlink(filp);
 			write_lock(&ci->m_lock);
 		}
 		write_unlock(&ci->m_lock);
fs/namei.c +45 -12

141 lines not on this page · raw message

include/linux/namei.h +6 -0
index 4c070fb29a91..85b12a17a23b 100644
@@ -57,12 +57,18 @@ static inline int user_path_at(int dfd, const char __user *name, unsigned flags,
 	return user_path_at_empty(dfd, name, flags, path, NULL);
 }
 
+struct dentry *lookup_one_qstr_excl(const struct qstr *name,
+				    struct dentry *base,
+				    unsigned int flags);
 extern int kern_path(const char *, unsigned, struct path *);
 
 extern struct dentry *kern_path_create(int, const char *, struct path *, unsigned int);
 extern struct dentry *user_path_create(int, const char __user *, struct path *, unsigned int);
 extern void done_path_create(struct path *, struct dentry *);
 extern struct dentry *kern_path_locked(const char *, struct path *);
+int vfs_path_parent_lookup(struct filename *filename, unsigned int flags,
+			   struct path *parent, struct qstr *last, int *type,
+			   const struct path *root);
 int vfs_path_lookup(struct dentry *, struct vfsmount *, const char *,
 		    unsigned int, struct path *);
Signature · 1 line
2.25.1
build bot
#6 lore raw

Re: [PATCH v8 3/3] ksmbd: fix racy issue from using ->d_parent and ->d_name

I love your patch! Perhaps something to improve:

Hi Namjae,

I love your patch! Perhaps something to improve:

[auto build test WARNING on vfs-idmapping/for-next]
[also build test WARNING on linus/master v6.3-rc2 next-20230315] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Namjae-Jeon/ksmbd-remove-internal-h-include/20230316-063731
base:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping.git for-next
patch link:    https://lore.kernel.org/r/20230315223435.5139-4-linkinjeon%40kernel.org
patch subject: [PATCH v8 3/3] ksmbd: fix racy issue from using ->d_parent and ->d_name
config: riscv-allmodconfig (https://download.01.org/0day-ci/archive/20230316/[email protected]/config)
compiler: riscv64-linux-gcc (GCC) 12.1.0

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/96a66748207926f3642ac4d24c7a591de22e3bad
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Namjae-Jeon/ksmbd-remove-internal-h-include/20230316-063731 git checkout 96a66748207926f3642ac4d24c7a591de22e3bad
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=riscv olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash fs/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

fs/ksmbd/vfs.c:54: warning: Function parameter or member 'parent' not described in 'ksmbd_vfs_lock_parent' fs/ksmbd/vfs.c:54: warning: Function parameter or member 'child' not described in 'ksmbd_vfs_lock_parent' fs/ksmbd/vfs.c:356: warning: Function parameter or member 'fp' not described in 'ksmbd_vfs_read' fs/ksmbd/vfs.c:356: warning: Excess function parameter 'fid' description in 'ksmbd_vfs_read' fs/ksmbd/vfs.c:472: warning: Function parameter or member 'fp' not described in 'ksmbd_vfs_write' fs/ksmbd/vfs.c:472: warning: Excess function parameter 'fid' description in 'ksmbd_vfs_write' fs/ksmbd/vfs.c:535: warning: Function parameter or member 'path' not described in 'ksmbd_vfs_getattr' fs/ksmbd/vfs.c:535: warning: Function parameter or member 'stat' not described in 'ksmbd_vfs_getattr' fs/ksmbd/vfs.c:535: warning: Excess function parameter 'work' description in 'ksmbd_vfs_getattr' fs/ksmbd/vfs.c:535: warning: Excess function parameter 'fid' description in 'ksmbd_vfs_getattr' fs/ksmbd/vfs.c:535: warning: Excess function parameter 'attrs' description in 'ksmbd_vfs_getattr' fs/ksmbd/vfs.c:552: warning: Function parameter or member 'p_id' not described in 'ksmbd_vfs_fsync' fs/ksmbd/vfs.c:575: warning: Function parameter or member 'work' not described in 'ksmbd_vfs_remove_file' fs/ksmbd/vfs.c:575: warning: Function parameter or member 'path' not described in 'ksmbd_vfs_remove_file' fs/ksmbd/vfs.c:575: warning: Excess function parameter 'name' description in 'ksmbd_vfs_remove_file' fs/ksmbd/vfs.c:613: warning: Function parameter or member 'work' not described in 'ksmbd_vfs_link' fs/ksmbd/vfs.c:779: warning: Function parameter or member 'fp' not described in 'ksmbd_vfs_truncate' fs/ksmbd/vfs.c:779: warning: Excess function parameter 'fid' description in 'ksmbd_vfs_truncate' fs/ksmbd/vfs.c:820: warning: Excess function parameter 'size' description in 'ksmbd_vfs_listxattr' fs/ksmbd/vfs.c:917: warning: Function parameter or member 'option' not described in 'ksmbd_vfs_set_fadvise' fs/ksmbd/vfs.c:917: warning: Excess function parameter 'options' description in 'ksmbd_vfs_set_fadvise' fs/ksmbd/vfs.c:1117: warning: Function parameter or member 'um' not described in 'ksmbd_vfs_lookup_in_dir' fs/ksmbd/vfs.c:1152: warning: Function parameter or member 'work' not described in 'ksmbd_vfs_kern_path_locked'

>> fs/ksmbd/vfs.c:1152: warning: expecting prototype for ksmbd_vfs_kern_path(). Prototype was for ksmbd_vfs_kern_path_locked() instead

vim +1152 fs/ksmbd/vfs.c

f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1139  
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1140  /**
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1141   * ksmbd_vfs_kern_path() - lookup a file and get path info
265fd1991c1db85 fs/ksmbd/vfs.c Hyunchul Lee  2021-09-25  1142   * @name:	file path that is relative to share
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1143   * @flags:	lookup flags
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1144   * @path:	if lookup succeed, return path info
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1145   * @caseless:	caseless filename lookup
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1146   *
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1147   * Return:	0 on success, otherwise error
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1148   */
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1149  int ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char *name,
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1150  			       unsigned int flags, struct path *path,
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1151  			       bool caseless)
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16 @1152  {
265fd1991c1db85 fs/ksmbd/vfs.c Hyunchul Lee  2021-09-25  1153  	struct ksmbd_share_config *share_conf = work->tcon->share_conf;
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1154  	int err;
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1155  	struct path parent_path;
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1156  
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1157  	err = ksmbd_vfs_path_lookup_locked(share_conf, name, flags, path);
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1158  	if (!err)
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1159  		return err;
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1160  
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1161  	if (caseless) {
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1162  		char *filepath;
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1163  		size_t path_len, remain_len;
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1164  
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1165  		filepath = kstrdup(name, GFP_KERNEL);
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1166  		if (!filepath)
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1167  			return -ENOMEM;
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1168  
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1169  		path_len = strlen(filepath);
265fd1991c1db85 fs/ksmbd/vfs.c Hyunchul Lee  2021-09-25  1170  		remain_len = path_len;
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1171  
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1172  		parent_path = share_conf->vfs_path;
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1173  		path_get(&parent_path);
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1174  
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1175  		while (d_can_lookup(parent_path.dentry)) {
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1176  			char *filename = filepath + path_len - remain_len;
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1177  			char *next = strchrnul(filename, '/');
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1178  			size_t filename_len = next - filename;
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1179  			bool is_last = !next[0];
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1180  
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1181  			if (filename_len == 0)
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1182  				break;
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1183  
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1184  			err = ksmbd_vfs_lookup_in_dir(&parent_path, filename,
dbab80e2071ad8c fs/ksmbd/vfs.c Atte Heikkilä 2022-09-28  1185  						      filename_len,
dbab80e2071ad8c fs/ksmbd/vfs.c Atte Heikkilä 2022-09-28  1186  						      work->conn->um);
265fd1991c1db85 fs/ksmbd/vfs.c Hyunchul Lee  2021-09-25  1187  			if (err)
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1188  				goto out2;
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1189  
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1190  			next[0] = '\0';
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1191  
265fd1991c1db85 fs/ksmbd/vfs.c Hyunchul Lee  2021-09-25  1192  			err = vfs_path_lookup(share_conf->vfs_path.dentry,
265fd1991c1db85 fs/ksmbd/vfs.c Hyunchul Lee  2021-09-25  1193  					      share_conf->vfs_path.mnt,
265fd1991c1db85 fs/ksmbd/vfs.c Hyunchul Lee  2021-09-25  1194  					      filepath,
265fd1991c1db85 fs/ksmbd/vfs.c Hyunchul Lee  2021-09-25  1195  					      flags,
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1196  					      path);
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1197  			if (err)
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1198  				goto out2;
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1199  			else if (is_last)
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1200  				goto out1;
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1201  			path_put(&parent_path);
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1202  			parent_path = *path;
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1203  
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1204  			next[0] = '/';
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1205  			remain_len -= filename_len + 1;
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1206  		}
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1207  
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1208  		err = -EINVAL;
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1209  out2:
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1210  		path_put(&parent_path);
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1211  out1:
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1212  		kfree(filepath);
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1213  	}
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1214  
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1215  	if (!err) {
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1216  		err = ksmbd_vfs_lock_parent(parent_path.dentry, path->dentry);
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1217  		if (err)
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1218  			dput(path->dentry);
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1219  		path_put(&parent_path);
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1220  	}
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1221  	return err;
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1222  }
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1223
Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
author maintainer
#7 lore raw

Re: [PATCH v8 3/3] ksmbd: fix racy issue from using ->d_parent and ->d_name

2023-03-16 9:30 GMT+09:00, kernel test robot <[email protected]>:

Quoted 1 line

Hi Namjae,

Hi,

First, Thanks for your report!

I will fix it on next version.

Thanks!

Quoted 240 lines

I love your patch! Perhaps something to improve:

[auto build test WARNING on vfs-idmapping/for-next]
[also build test WARNING on linus/master v6.3-rc2 next-20230315] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Namjae-Jeon/ksmbd-remove-internal-h-include/20230316-063731
base: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping.git
for-next
patch link:
https://lore.kernel.org/r/20230315223435.5139-4-linkinjeon%40kernel.org
patch subject: [PATCH v8 3/3] ksmbd: fix racy issue from using ->d_parent and ->d_name
config: riscv-allmodconfig
(https://download.01.org/0day-ci/archive/20230316/[email protected]/config)
compiler: riscv64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/intel-lab-lkp/linux/commit/96a66748207926f3642ac4d24c7a591de22e3bad
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review
Namjae-Jeon/ksmbd-remove-internal-h-include/20230316-063731

        git checkout 96a66748207926f3642ac4d24c7a591de22e3bad
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1
O=build_dir ARCH=riscv olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1

O=build_dir ARCH=riscv SHELL=/bin/bash fs/

If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <[email protected]>
| Link:
https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

fs/ksmbd/vfs.c:54: warning: Function parameter or member 'parent' not
described in 'ksmbd_vfs_lock_parent'
fs/ksmbd/vfs.c:54: warning: Function parameter or member 'child' not
described in 'ksmbd_vfs_lock_parent'
fs/ksmbd/vfs.c:356: warning: Function parameter or member 'fp' not
described in 'ksmbd_vfs_read'
fs/ksmbd/vfs.c:356: warning: Excess function parameter 'fid' description
in 'ksmbd_vfs_read'
fs/ksmbd/vfs.c:472: warning: Function parameter or member 'fp' not
described in 'ksmbd_vfs_write'
fs/ksmbd/vfs.c:472: warning: Excess function parameter 'fid' description
in 'ksmbd_vfs_write'
fs/ksmbd/vfs.c:535: warning: Function parameter or member 'path' not
described in 'ksmbd_vfs_getattr'
fs/ksmbd/vfs.c:535: warning: Function parameter or member 'stat' not
described in 'ksmbd_vfs_getattr'
fs/ksmbd/vfs.c:535: warning: Excess function parameter 'work' description
in 'ksmbd_vfs_getattr'
fs/ksmbd/vfs.c:535: warning: Excess function parameter 'fid' description
in 'ksmbd_vfs_getattr'
fs/ksmbd/vfs.c:535: warning: Excess function parameter 'attrs'
description in 'ksmbd_vfs_getattr'
fs/ksmbd/vfs.c:552: warning: Function parameter or member 'p_id' not
described in 'ksmbd_vfs_fsync'
fs/ksmbd/vfs.c:575: warning: Function parameter or member 'work' not
described in 'ksmbd_vfs_remove_file'
fs/ksmbd/vfs.c:575: warning: Function parameter or member 'path' not
described in 'ksmbd_vfs_remove_file'
fs/ksmbd/vfs.c:575: warning: Excess function parameter 'name' description
in 'ksmbd_vfs_remove_file'
fs/ksmbd/vfs.c:613: warning: Function parameter or member 'work' not
described in 'ksmbd_vfs_link'
fs/ksmbd/vfs.c:779: warning: Function parameter or member 'fp' not
described in 'ksmbd_vfs_truncate'
fs/ksmbd/vfs.c:779: warning: Excess function parameter 'fid' description
in 'ksmbd_vfs_truncate'
fs/ksmbd/vfs.c:820: warning: Excess function parameter 'size' description
in 'ksmbd_vfs_listxattr'
fs/ksmbd/vfs.c:917: warning: Function parameter or member 'option' not
described in 'ksmbd_vfs_set_fadvise'
fs/ksmbd/vfs.c:917: warning: Excess function parameter 'options'
description in 'ksmbd_vfs_set_fadvise'
fs/ksmbd/vfs.c:1117: warning: Function parameter or member 'um' not
described in 'ksmbd_vfs_lookup_in_dir'
fs/ksmbd/vfs.c:1152: warning: Function parameter or member 'work' not
described in 'ksmbd_vfs_kern_path_locked'

Quoted 3 lines
Quoted 3 lines

fs/ksmbd/vfs.c:1152: warning: expecting prototype for ksmbd_vfs_kern_path(). Prototype was for ksmbd_vfs_kern_path_locked() instead

vim +1152 fs/ksmbd/vfs.c

f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1139
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1140  /**
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1141   *

ksmbd_vfs_kern_path() - lookup a file and get path info
265fd1991c1db85 fs/ksmbd/vfs.c Hyunchul Lee 2021-09-25 1142 *
@name: file path that is relative to share
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon 2021-03-16 1143 *
@flags: lookup flags
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon 2021-03-16 1144 * @path: if
lookup succeed, return path info
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon 2021-03-16 1145 *
@caseless: caseless filename lookup

f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1146   *
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1147   * Return:	0
on success, otherwise error
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1148   */
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1149  int

ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char *name,
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon 2023-03-16 1150
unsigned int flags, struct path *path,
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon 2023-03-16 1151
bool caseless)

f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16 @1152  {
265fd1991c1db85 fs/ksmbd/vfs.c Hyunchul Lee  2021-09-25  1153  	struct
ksmbd_share_config *share_conf = work->tcon->share_conf;
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1154  	int err;
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1155  	struct path
parent_path;
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1156
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1157  	err =
ksmbd_vfs_path_lookup_locked(share_conf, name, flags, path);
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1158  	if (!err)
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1159  		return
err;
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1160
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1161  	if

(caseless) {
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee 2021-04-13 1162 char
*filepath;
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee 2021-04-13 1163 size_t
path_len, remain_len;

3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1164
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1165  		filepath =

kstrdup(name, GFP_KERNEL);
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee 2021-04-13 1166 if
(!filepath)
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee 2021-04-13 1167 return
-ENOMEM;

3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1168
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1169  		path_len =
strlen(filepath);
265fd1991c1db85 fs/ksmbd/vfs.c Hyunchul Lee  2021-09-25  1170  		remain_len
= path_len;
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1171
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1172  		parent_path
= share_conf->vfs_path;
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1173
		path_get(&parent_path);
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1174
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1175  		while

(d_can_lookup(parent_path.dentry)) {
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee 2021-04-13 1176 char
*filename = filepath + path_len - remain_len;
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee 2021-04-13 1177 char *next
= strchrnul(filename, '/');
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee 2021-04-13 1178 size_t
filename_len = next - filename;
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee 2021-04-13 1179 bool
is_last = !next[0];

3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1180
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1181  			if
(filename_len == 0)
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1182  				break;
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1183
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1184  			err =

ksmbd_vfs_lookup_in_dir(&parent_path, filename, dbab80e2071ad8c fs/ksmbd/vfs.c Atte Heikkilä 2022-09-28 1185 filename_len,
dbab80e2071ad8c fs/ksmbd/vfs.c Atte Heikkilä 2022-09-28 1186 work->conn->um);

265fd1991c1db85 fs/ksmbd/vfs.c Hyunchul Lee  2021-09-25  1187  			if (err)
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1188  				goto
out2;
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1189
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1190  			next[0] =
'\0';
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1191
265fd1991c1db85 fs/ksmbd/vfs.c Hyunchul Lee  2021-09-25  1192  			err =

vfs_path_lookup(share_conf->vfs_path.dentry, 265fd1991c1db85 fs/ksmbd/vfs.c Hyunchul Lee 2021-09-25 1193 share_conf->vfs_path.mnt,
265fd1991c1db85 fs/ksmbd/vfs.c Hyunchul Lee 2021-09-25 1194 filepath,
265fd1991c1db85 fs/ksmbd/vfs.c Hyunchul Lee 2021-09-25 1195 flags,
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon 2023-03-16 1196
path);

f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1197  			if (err)
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1198  				goto

out2;
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon 2023-03-16 1199 else if
(is_last)
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon 2023-03-16 1200 goto
out1;
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon 2023-03-16 1201

			path_put(&parent_path);
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1202
			parent_path = *path;
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1203
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1204  			next[0] =
'/';
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1205  			remain_len
-= filename_len + 1;
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1206  		}
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1207
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1208  		err =
-EINVAL;
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1209  out2:
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1210
		path_put(&parent_path);
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1211  out1:
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1212
		kfree(filepath);
3c20378325c710e fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1213  	}
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1214
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1215  	if (!err) {
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1216  		err =
ksmbd_vfs_lock_parent(parent_path.dentry, path->dentry);
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1217  		if (err)
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1218
			dput(path->dentry);
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1219
		path_put(&parent_path);
96a66748207926f fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1220  	}
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1221  	return err;
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1222  }
f44158485826c07 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1223

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

build bot
#8 lore raw

Re: [PATCH v8 3/3] ksmbd: fix racy issue from using ->d_parent and ->d_name

In reply to Namjae Jeon
I love your patch! Perhaps something to improve:

Hi Namjae,

I love your patch! Perhaps something to improve:

[auto build test WARNING on vfs-idmapping/for-next]
[also build test WARNING on linus/master v6.3-rc2 next-20230316] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Namjae-Jeon/ksmbd-remove-internal-h-include/20230316-063731
base:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping.git for-next
patch link:    https://lore.kernel.org/r/20230315223435.5139-4-linkinjeon%40kernel.org
patch subject: [PATCH v8 3/3] ksmbd: fix racy issue from using ->d_parent and ->d_name
config: i386-randconfig-a013-20230313 (https://download.01.org/0day-ci/archive/20230316/[email protected]/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/96a66748207926f3642ac4d24c7a591de22e3bad
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Namjae-Jeon/ksmbd-remove-internal-h-include/20230316-063731 git checkout 96a66748207926f3642ac4d24c7a591de22e3bad
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash fs/ksmbd/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

fs/ksmbd/vfs.c:54: warning: Function parameter or member 'parent' not described in 'ksmbd_vfs_lock_parent' fs/ksmbd/vfs.c:54: warning: Function parameter or member 'child' not described in 'ksmbd_vfs_lock_parent' fs/ksmbd/vfs.c:356: warning: Function parameter or member 'fp' not described in 'ksmbd_vfs_read' fs/ksmbd/vfs.c:356: warning: Excess function parameter 'fid' description in 'ksmbd_vfs_read' fs/ksmbd/vfs.c:472: warning: Function parameter or member 'fp' not described in 'ksmbd_vfs_write' fs/ksmbd/vfs.c:472: warning: Excess function parameter 'fid' description in 'ksmbd_vfs_write' fs/ksmbd/vfs.c:535: warning: Function parameter or member 'path' not described in 'ksmbd_vfs_getattr' fs/ksmbd/vfs.c:535: warning: Function parameter or member 'stat' not described in 'ksmbd_vfs_getattr' fs/ksmbd/vfs.c:535: warning: Excess function parameter 'work' description in 'ksmbd_vfs_getattr' fs/ksmbd/vfs.c:535: warning: Excess function parameter 'fid' description in 'ksmbd_vfs_getattr' fs/ksmbd/vfs.c:535: warning: Excess function parameter 'attrs' description in 'ksmbd_vfs_getattr' fs/ksmbd/vfs.c:552: warning: Function parameter or member 'p_id' not described in 'ksmbd_vfs_fsync' fs/ksmbd/vfs.c:575: warning: Function parameter or member 'work' not described in 'ksmbd_vfs_remove_file' fs/ksmbd/vfs.c:575: warning: Function parameter or member 'path' not described in 'ksmbd_vfs_remove_file' fs/ksmbd/vfs.c:575: warning: Excess function parameter 'name' description in 'ksmbd_vfs_remove_file' fs/ksmbd/vfs.c:613: warning: Function parameter or member 'work' not described in 'ksmbd_vfs_link' fs/ksmbd/vfs.c:779: warning: Function parameter or member 'fp' not described in 'ksmbd_vfs_truncate' fs/ksmbd/vfs.c:779: warning: Excess function parameter 'fid' description in 'ksmbd_vfs_truncate' fs/ksmbd/vfs.c:820: warning: Excess function parameter 'size' description in 'ksmbd_vfs_listxattr' fs/ksmbd/vfs.c:917: warning: Function parameter or member 'option' not described in 'ksmbd_vfs_set_fadvise' fs/ksmbd/vfs.c:917: warning: Excess function parameter 'options' description in 'ksmbd_vfs_set_fadvise' fs/ksmbd/vfs.c:1117: warning: Function parameter or member 'um' not described in 'ksmbd_vfs_lookup_in_dir' fs/ksmbd/vfs.c:1152: warning: Function parameter or member 'work' not described in 'ksmbd_vfs_kern_path_locked'

>> fs/ksmbd/vfs.c:1152: warning: expecting prototype for ksmbd_vfs_kern_path(). Prototype was for ksmbd_vfs_kern_path_locked() instead

vim +1152 fs/ksmbd/vfs.c

f44158485826c0 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1139  
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1140  /**
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1141   * ksmbd_vfs_kern_path() - lookup a file and get path info
265fd1991c1db8 fs/ksmbd/vfs.c Hyunchul Lee  2021-09-25  1142   * @name:	file path that is relative to share
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1143   * @flags:	lookup flags
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1144   * @path:	if lookup succeed, return path info
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1145   * @caseless:	caseless filename lookup
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1146   *
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1147   * Return:	0 on success, otherwise error
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1148   */
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1149  int ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char *name,
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1150  			       unsigned int flags, struct path *path,
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1151  			       bool caseless)
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon   2021-03-16 @1152  {
265fd1991c1db8 fs/ksmbd/vfs.c Hyunchul Lee  2021-09-25  1153  	struct ksmbd_share_config *share_conf = work->tcon->share_conf;
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1154  	int err;
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1155  	struct path parent_path;
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1156  
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1157  	err = ksmbd_vfs_path_lookup_locked(share_conf, name, flags, path);
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1158  	if (!err)
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1159  		return err;
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1160  
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1161  	if (caseless) {
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1162  		char *filepath;
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1163  		size_t path_len, remain_len;
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1164  
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1165  		filepath = kstrdup(name, GFP_KERNEL);
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1166  		if (!filepath)
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1167  			return -ENOMEM;
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1168  
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1169  		path_len = strlen(filepath);
265fd1991c1db8 fs/ksmbd/vfs.c Hyunchul Lee  2021-09-25  1170  		remain_len = path_len;
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1171  
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1172  		parent_path = share_conf->vfs_path;
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1173  		path_get(&parent_path);
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1174  
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1175  		while (d_can_lookup(parent_path.dentry)) {
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1176  			char *filename = filepath + path_len - remain_len;
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1177  			char *next = strchrnul(filename, '/');
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1178  			size_t filename_len = next - filename;
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1179  			bool is_last = !next[0];
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1180  
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1181  			if (filename_len == 0)
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1182  				break;
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1183  
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1184  			err = ksmbd_vfs_lookup_in_dir(&parent_path, filename,
dbab80e2071ad8 fs/ksmbd/vfs.c Atte Heikkilä 2022-09-28  1185  						      filename_len,
dbab80e2071ad8 fs/ksmbd/vfs.c Atte Heikkilä 2022-09-28  1186  						      work->conn->um);
265fd1991c1db8 fs/ksmbd/vfs.c Hyunchul Lee  2021-09-25  1187  			if (err)
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1188  				goto out2;
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1189  
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1190  			next[0] = '\0';
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1191  
265fd1991c1db8 fs/ksmbd/vfs.c Hyunchul Lee  2021-09-25  1192  			err = vfs_path_lookup(share_conf->vfs_path.dentry,
265fd1991c1db8 fs/ksmbd/vfs.c Hyunchul Lee  2021-09-25  1193  					      share_conf->vfs_path.mnt,
265fd1991c1db8 fs/ksmbd/vfs.c Hyunchul Lee  2021-09-25  1194  					      filepath,
265fd1991c1db8 fs/ksmbd/vfs.c Hyunchul Lee  2021-09-25  1195  					      flags,
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1196  					      path);
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1197  			if (err)
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1198  				goto out2;
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1199  			else if (is_last)
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1200  				goto out1;
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1201  			path_put(&parent_path);
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1202  			parent_path = *path;
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1203  
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1204  			next[0] = '/';
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1205  			remain_len -= filename_len + 1;
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1206  		}
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1207  
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1208  		err = -EINVAL;
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1209  out2:
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1210  		path_put(&parent_path);
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1211  out1:
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1212  		kfree(filepath);
3c20378325c710 fs/cifsd/vfs.c Hyunchul Lee  2021-04-13  1213  	}
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1214  
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1215  	if (!err) {
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1216  		err = ksmbd_vfs_lock_parent(parent_path.dentry, path->dentry);
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1217  		if (err)
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1218  			dput(path->dentry);
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1219  		path_put(&parent_path);
96a66748207926 fs/ksmbd/vfs.c Namjae Jeon   2023-03-16  1220  	}
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1221  	return err;
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1222  }
f44158485826c0 fs/cifsd/vfs.c Namjae Jeon   2021-03-16  1223
Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
maintainer
#9 lore raw
In reply to Namjae Jeon

On Thu, Mar 16, 2023 at 07:34:32AM +0900, Namjae Jeon wrote:

OK... Let's do it that way: I put the first two commits into never-rebased branch (work.lock_rename_child), then you pull it into your tree (and slap the third commit on top of that) while I merge it into #for-next.

maintainer
#10 lore raw
Quoted 6 lines · Al Viro

On Thu, Mar 16, 2023 at 07:34:32AM +0900, Namjae Jeon wrote:

OK... Let's do it that way: I put the first two commits into never-rebased branch (work.lock_rename_child), then you pull it into your tree (and slap the third commit on top of that) while I merge it into #for-next.

Done and pushed.

author maintainer
#11 lore raw
In reply to Al Viro

2023-04-21 11:35 GMT+09:00, Al Viro <[email protected]>:

Quoted 6 lines · Namjae Jeon

OK... Let's do it that way: I put the first two commits into never-rebased branch (work.lock_rename_child), then you pull it into your tree (and slap the third commit on top of that) while I merge it into #for-next.

Okay. Can I add your acked-by in third patch ?

Thank you!

Quoted 1 line
#12 lore raw
In reply to Namjae Jeon

Hi Namjae,

Maybe I am missing something, but I do not see any mnt_want_write() in ksmbd at all. Is it well hidden somewhere?

Thanks,
Amir.

Quoted 50 lines · Namjae Jeon

v8:
- Don't call vfs_path_lookup() to avoid repeat lookup, Instead, lookup last component after locking the parent that got from vfs_path_parent_lookup helper.
v7:
- constify struct path.
- recreate patch-set base on recent Al's patches.
v6:
- rename __lookup_hash() to lookup_one_qstr_excl and export.
- change dget() to dget_parent() in unlink.
- lock parent of open file in smb2_open() to make file_present worthable.
v5:
- add lock_rename_child() helper.
- remove d_is_symlink() check for new_path.dentry.
- use lock_rename_child() helper instead of lock_rename().
- use dget() instead of dget_parent().
- check that old_child is still hashed.
- directly check child->parent instead of using take_dentry_name_snapshot().
v4:
- switch the order of 3/4 and 4/4 patch.
- fix vfs_path_parent_lookup() parameter description mismatch.
v3:
- use dget_parent + take_dentry_name_snapshot() to check stability of source rename in smb2_vfs_rename().
v2:
- add filename_lock to avoid racy issue from fp->filename. (Sergey Senozhatsky)
- fix warning: variable 'old_dentry' is used uninitialized (kernel test robot)

Al Viro (1):
fs: introduce lock_rename_child() helper

Namjae Jeon (2):
ksmbd: remove internal.h include
ksmbd: fix racy issue from using ->d_parent and ->d_name

 fs/internal.h         |   2 -
 fs/ksmbd/smb2pdu.c    | 147 ++++----------
 fs/ksmbd/vfs.c        | 435 ++++++++++++++++++------------------------
 fs/ksmbd/vfs.h        |  19 +-
 fs/ksmbd/vfs_cache.c  |   5 +-
 fs/namei.c            | 125 +++++++++---
 include/linux/namei.h |   9 +
 7 files changed, 342 insertions(+), 400 deletions(-)

--
2.25.1

author maintainer
#13 lore raw

2023-06-13 19:57 GMT+09:00, Amir Goldstein <[email protected]>:

Quoted 1 line

Hi Namjae,

Hi Amir,

Quoted 3 lines

Maybe I am missing something, but I do not see any mnt_want_write() in ksmbd at all. Is it well hidden somewhere?

At a quick glance, We need to add it for ksmbd_vfs_unlink and ksmbd_vfs_rename. I'll look further into where else should I add it.

Thanks for letting me know!

Quoted 60 lines

Thanks,
Amir.

Quoted 54 lines · Namjae Jeon

v8:
- Don't call vfs_path_lookup() to avoid repeat lookup, Instead, lookup last component after locking the parent that got from
vfs_path_parent_lookup
helper.
v7:
- constify struct path.
- recreate patch-set base on recent Al's patches.
v6:
- rename __lookup_hash() to lookup_one_qstr_excl and export.
- change dget() to dget_parent() in unlink.
- lock parent of open file in smb2_open() to make file_present worthable.
v5:
- add lock_rename_child() helper.
- remove d_is_symlink() check for new_path.dentry.
- use lock_rename_child() helper instead of lock_rename().
- use dget() instead of dget_parent().
- check that old_child is still hashed.
- directly check child->parent instead of using
take_dentry_name_snapshot().
v4:
- switch the order of 3/4 and 4/4 patch.
- fix vfs_path_parent_lookup() parameter description mismatch.
v3:
- use dget_parent + take_dentry_name_snapshot() to check stability of
source
rename in smb2_vfs_rename().
v2:
- add filename_lock to avoid racy issue from fp->filename. (Sergey
Senozhatsky)
- fix warning: variable 'old_dentry' is used uninitialized (kernel test robot)

Al Viro (1):
fs: introduce lock_rename_child() helper

Namjae Jeon (2):
ksmbd: remove internal.h include
ksmbd: fix racy issue from using ->d_parent and ->d_name

 fs/internal.h         |   2 -
 fs/ksmbd/smb2pdu.c    | 147 ++++----------
 fs/ksmbd/vfs.c        | 435 ++++++++++++++++++------------------------
 fs/ksmbd/vfs.h        |  19 +-
 fs/ksmbd/vfs_cache.c  |   5 +-
 fs/namei.c            | 125 +++++++++---
 include/linux/namei.h |   9 +
 7 files changed, 342 insertions(+), 400 deletions(-)

--
2.25.1