* [PATCH v8 0/3] ksmbd patches included vfs changes
@ 2023-03-15 22:34 Namjae Jeon
2023-03-15 22:34 ` [PATCH v8 1/3] ksmbd: remove internal.h include Namjae Jeon
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: Namjae Jeon @ 2023-03-15 22:34 UTC (permalink / raw)
To: viro
Cc: linux-fsdevel, linux-cifs, smfrench, senozhatsky, tom, brauner,
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
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v8 1/3] ksmbd: remove internal.h include 2023-03-15 22:34 [PATCH v8 0/3] ksmbd patches included vfs changes Namjae Jeon @ 2023-03-15 22:34 ` Namjae Jeon 2023-03-17 8:55 ` Christian Brauner 2023-03-15 22:34 ` [PATCH v8 2/3] fs: introduce lock_rename_child() helper Namjae Jeon ` (3 subsequent siblings) 4 siblings, 1 reply; 14+ messages in thread From: Namjae Jeon @ 2023-03-15 22:34 UTC (permalink / raw) To: viro Cc: linux-fsdevel, linux-cifs, smfrench, senozhatsky, tom, brauner, Namjae Jeon 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 <viro@zeniv.linux.org.uk> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> --- fs/internal.h | 2 -- fs/ksmbd/vfs.c | 2 -- include/linux/namei.h | 2 ++ 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/fs/internal.h b/fs/internal.h index dc4eb91a577a..071a7517f1a7 100644 --- a/fs/internal.h +++ b/fs/internal.h @@ -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); diff --git a/fs/ksmbd/vfs.c b/fs/ksmbd/vfs.c index 5ea9229dad2c..cef07d7fb7dc 100644 --- a/fs/ksmbd/vfs.c +++ b/fs/ksmbd/vfs.c @@ -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" diff --git a/include/linux/namei.h b/include/linux/namei.h index 0d4531fd46e7..c2747cfe97ac 100644 --- a/include/linux/namei.h +++ b/include/linux/namei.h @@ -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); -- 2.25.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v8 1/3] ksmbd: remove internal.h include 2023-03-15 22:34 ` [PATCH v8 1/3] ksmbd: remove internal.h include Namjae Jeon @ 2023-03-17 8:55 ` Christian Brauner 0 siblings, 0 replies; 14+ messages in thread From: Christian Brauner @ 2023-03-17 8:55 UTC (permalink / raw) To: Namjae Jeon; +Cc: viro, linux-fsdevel, linux-cifs, smfrench, senozhatsky, tom On Thu, Mar 16, 2023 at 07:34:33AM +0900, Namjae Jeon wrote: > 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 <viro@zeniv.linux.org.uk> > Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> > --- Looks good to me, Reviewed-by: Christian Brauner <brauner@kernel.org> ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v8 2/3] fs: introduce lock_rename_child() helper 2023-03-15 22:34 [PATCH v8 0/3] ksmbd patches included vfs changes Namjae Jeon 2023-03-15 22:34 ` [PATCH v8 1/3] ksmbd: remove internal.h include Namjae Jeon @ 2023-03-15 22:34 ` Namjae Jeon 2023-03-16 0:46 ` [lkp] [+118 bytes kernel size regression] [i386-tinyconfig] [6e6937ac88] " kernel test robot 2023-03-15 22:34 ` [PATCH v8 3/3] ksmbd: fix racy issue from using ->d_parent and ->d_name Namjae Jeon ` (2 subsequent siblings) 4 siblings, 1 reply; 14+ messages in thread From: Namjae Jeon @ 2023-03-15 22:34 UTC (permalink / raw) To: viro Cc: linux-fsdevel, linux-cifs, smfrench, senozhatsky, tom, brauner, Namjae Jeon From: Al Viro <viro@zeniv.linux.org.uk> 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 <viro@zeniv.linux.org.uk> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> --- fs/namei.c | 68 ++++++++++++++++++++++++++++++++++++------- include/linux/namei.h | 1 + 2 files changed, 58 insertions(+), 11 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index 5855dc6edbd5..984d4f7b47dc 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -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); diff --git a/include/linux/namei.h b/include/linux/namei.h index c2747cfe97ac..4c070fb29a91 100644 --- a/include/linux/namei.h +++ b/include/linux/namei.h @@ -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); -- 2.25.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [lkp] [+118 bytes kernel size regression] [i386-tinyconfig] [6e6937ac88] fs: introduce lock_rename_child() helper 2023-03-15 22:34 ` [PATCH v8 2/3] fs: introduce lock_rename_child() helper Namjae Jeon @ 2023-03-16 0:46 ` kernel test robot 0 siblings, 0 replies; 14+ messages in thread From: kernel test robot @ 2023-03-16 0:46 UTC (permalink / raw) To: Al Viro; +Cc: oe-kbuild-all, lkp 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/20230315223435.5139-3-linkinjeon@kernel.org/ 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v8 3/3] ksmbd: fix racy issue from using ->d_parent and ->d_name 2023-03-15 22:34 [PATCH v8 0/3] ksmbd patches included vfs changes Namjae Jeon 2023-03-15 22:34 ` [PATCH v8 1/3] ksmbd: remove internal.h include Namjae Jeon 2023-03-15 22:34 ` [PATCH v8 2/3] fs: introduce lock_rename_child() helper Namjae Jeon @ 2023-03-15 22:34 ` Namjae Jeon 2023-03-16 0:30 ` kernel test robot 2023-03-16 7:30 ` kernel test robot 2023-04-21 2:35 ` [PATCH v8 0/3] ksmbd patches included vfs changes Al Viro 2023-06-13 10:57 ` Amir Goldstein 4 siblings, 2 replies; 14+ messages in thread From: Namjae Jeon @ 2023-03-15 22:34 UTC (permalink / raw) To: viro Cc: linux-fsdevel, linux-cifs, smfrench, senozhatsky, tom, brauner, 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 <linkinjeon@kernel.org> --- 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(-) diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 547a10977454..e27f86b03ad4 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -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, diff --git a/fs/ksmbd/vfs.c b/fs/ksmbd/vfs.c index cef07d7fb7dc..491f1b6acd56 100644 --- a/fs/ksmbd/vfs.c +++ b/fs/ksmbd/vfs.c @@ -18,6 +18,7 @@ #include <linux/vmalloc.h> #include <linux/sched/xacct.h> #include <linux/crc32c.h> +#include <linux/namei.h> #include "glob.h" #include "oplock.h" @@ -35,19 +36,6 @@ #include "mgmt/user_session.h" #include "mgmt/user_config.h" -static char *extract_last_component(char *path) -{ - char *p = strrchr(path, '/'); - - if (p && p[1] != '\0') { - *p = '\0'; - p++; - } else { - p = NULL; - } - return p; -} - static void ksmbd_vfs_inherit_owner(struct ksmbd_work *work, struct inode *parent_inode, struct inode *inode) @@ -61,65 +49,77 @@ static void ksmbd_vfs_inherit_owner(struct ksmbd_work *work, /** * ksmbd_vfs_lock_parent() - lock parent dentry if it is stable - * - * the parent dentry got by dget_parent or @parent could be - * unstable, we try to lock a parent inode and lookup the - * child dentry again. - * - * the reference count of @parent isn't incremented. */ -int ksmbd_vfs_lock_parent(struct mnt_idmap *idmap, struct dentry *parent, - struct dentry *child) +int ksmbd_vfs_lock_parent(struct dentry *parent, struct dentry *child) { - struct dentry *dentry; - int ret = 0; - inode_lock_nested(d_inode(parent), I_MUTEX_PARENT); - dentry = lookup_one(idmap, child->d_name.name, parent, - child->d_name.len); - if (IS_ERR(dentry)) { - ret = PTR_ERR(dentry); - goto out_err; - } - - if (dentry != child) { - ret = -ESTALE; - dput(dentry); - goto out_err; + if (child->d_parent != parent) { + inode_unlock(d_inode(parent)); + return -ENOENT; } - dput(dentry); return 0; -out_err: - inode_unlock(d_inode(parent)); - return ret; } -int ksmbd_vfs_may_delete(struct mnt_idmap *idmap, - struct dentry *dentry) +static int ksmbd_vfs_path_lookup_locked(struct ksmbd_share_config *share_conf, + char *pathname, unsigned int flags, + struct path *path) { - struct dentry *parent; - int ret; + struct qstr last; + struct filename *filename; + struct path *root_share_path = &share_conf->vfs_path; + int err, type; + struct path parent_path; + struct dentry *d; + + if (pathname[0] == '\0') { + pathname = share_conf->path; + root_share_path = NULL; + } else { + flags |= LOOKUP_BENEATH; + } - parent = dget_parent(dentry); - ret = ksmbd_vfs_lock_parent(idmap, parent, dentry); - if (ret) { - dput(parent); - return ret; + filename = getname_kernel(pathname); + if (IS_ERR(filename)) + return PTR_ERR(filename); + + err = vfs_path_parent_lookup(filename, flags, + &parent_path, &last, &type, + root_share_path); + putname(filename); + if (err) + return err; + + if (unlikely(type != LAST_NORM)) { + path_put(&parent_path); + return -ENOENT; } - ret = inode_permission(idmap, d_inode(parent), - MAY_EXEC | MAY_WRITE); + inode_lock_nested(parent_path.dentry->d_inode, I_MUTEX_PARENT); + d = lookup_one_qstr_excl(&last, parent_path.dentry, 0); + if (IS_ERR(d)) + goto err_out; - inode_unlock(d_inode(parent)); - dput(parent); - return ret; + if (d_is_negative(d)) { + dput(d); + goto err_out; + } + + path->dentry = d; + path->mnt = share_conf->vfs_path.mnt; + path_put(&parent_path); + + return 0; + +err_out: + inode_unlock(parent_path.dentry->d_inode); + path_put(&parent_path); + return -ENOENT; } int ksmbd_vfs_query_maximal_access(struct mnt_idmap *idmap, struct dentry *dentry, __le32 *daccess) { - struct dentry *parent; int ret = 0; *daccess = cpu_to_le32(FILE_READ_ATTRIBUTES | READ_CONTROL); @@ -136,18 +136,9 @@ int ksmbd_vfs_query_maximal_access(struct mnt_idmap *idmap, if (!inode_permission(idmap, d_inode(dentry), MAY_OPEN | MAY_EXEC)) *daccess |= FILE_EXECUTE_LE; - parent = dget_parent(dentry); - ret = ksmbd_vfs_lock_parent(idmap, parent, dentry); - if (ret) { - dput(parent); - return ret; - } - - if (!inode_permission(idmap, d_inode(parent), MAY_EXEC | MAY_WRITE)) + if (!inode_permission(idmap, d_inode(dentry->d_parent), MAY_EXEC | MAY_WRITE)) *daccess |= FILE_DELETE_LE; - inode_unlock(d_inode(parent)); - dput(parent); return ret; } @@ -580,54 +571,32 @@ int ksmbd_vfs_fsync(struct ksmbd_work *work, u64 fid, u64 p_id) * * Return: 0 on success, otherwise error */ -int ksmbd_vfs_remove_file(struct ksmbd_work *work, char *name) +int ksmbd_vfs_remove_file(struct ksmbd_work *work, const struct path *path) { struct mnt_idmap *idmap; - struct path path; - struct dentry *parent; + struct dentry *parent = path->dentry->d_parent; int err; if (ksmbd_override_fsids(work)) return -ENOMEM; - err = ksmbd_vfs_kern_path(work, name, LOOKUP_NO_SYMLINKS, &path, false); - if (err) { - ksmbd_debug(VFS, "can't get %s, err %d\n", name, err); - ksmbd_revert_fsids(work); - return err; - } - - idmap = mnt_idmap(path.mnt); - parent = dget_parent(path.dentry); - err = ksmbd_vfs_lock_parent(idmap, parent, path.dentry); - if (err) { - dput(parent); - path_put(&path); - ksmbd_revert_fsids(work); - return err; - } - - if (!d_inode(path.dentry)->i_nlink) { + if (!d_inode(path->dentry)->i_nlink) { err = -ENOENT; goto out_err; } - if (S_ISDIR(d_inode(path.dentry)->i_mode)) { - err = vfs_rmdir(idmap, d_inode(parent), path.dentry); + idmap = mnt_idmap(path->mnt); + if (S_ISDIR(d_inode(path->dentry)->i_mode)) { + err = vfs_rmdir(idmap, d_inode(parent), path->dentry); if (err && err != -ENOTEMPTY) - ksmbd_debug(VFS, "%s: rmdir failed, err %d\n", name, - err); + ksmbd_debug(VFS, "rmdir failed, err %d\n", err); } else { - err = vfs_unlink(idmap, d_inode(parent), path.dentry, NULL); + err = vfs_unlink(idmap, d_inode(parent), path->dentry, NULL); if (err) - ksmbd_debug(VFS, "%s: unlink failed, err %d\n", name, - err); + ksmbd_debug(VFS, "unlink failed, err %d\n", err); } out_err: - inode_unlock(d_inode(parent)); - dput(parent); - path_put(&path); ksmbd_revert_fsids(work); return err; } @@ -686,149 +655,114 @@ int ksmbd_vfs_link(struct ksmbd_work *work, const char *oldname, return err; } -static int ksmbd_validate_entry_in_use(struct dentry *src_dent) +int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path, + char *newname, int flags) { - struct dentry *dst_dent; - - spin_lock(&src_dent->d_lock); - list_for_each_entry(dst_dent, &src_dent->d_subdirs, d_child) { - struct ksmbd_file *child_fp; + struct dentry *old_parent, *new_dentry, *trap; + struct dentry *old_child = old_path->dentry; + struct path new_path; + struct qstr new_last; + struct renamedata rd; + struct filename *to; + struct ksmbd_share_config *share_conf = work->tcon->share_conf; + struct ksmbd_file *parent_fp; + int new_type; + int err, lookup_flags = LOOKUP_NO_SYMLINKS; - if (d_really_is_negative(dst_dent)) - continue; + if (ksmbd_override_fsids(work)) + return -ENOMEM; - child_fp = ksmbd_lookup_fd_inode(d_inode(dst_dent)); - if (child_fp) { - spin_unlock(&src_dent->d_lock); - ksmbd_debug(VFS, "Forbid rename, sub file/dir is in use\n"); - return -EACCES; - } + to = getname_kernel(newname); + if (IS_ERR(to)) { + err = PTR_ERR(to); + goto revert_fsids; } - spin_unlock(&src_dent->d_lock); - return 0; -} +retry: + err = vfs_path_parent_lookup(to, lookup_flags | LOOKUP_BENEATH, + &new_path, &new_last, &new_type, + &share_conf->vfs_path); + if (err) + goto out1; -static int __ksmbd_vfs_rename(struct ksmbd_work *work, - struct mnt_idmap *src_idmap, - struct dentry *src_dent_parent, - struct dentry *src_dent, - struct mnt_idmap *dst_idmap, - struct dentry *dst_dent_parent, - struct dentry *trap_dent, - char *dst_name) -{ - struct dentry *dst_dent; - int err; + if (old_path->mnt != new_path.mnt) { + err = -EXDEV; + goto out2; + } - if (!work->tcon->posix_extensions) { - err = ksmbd_validate_entry_in_use(src_dent); - if (err) - return err; + trap = lock_rename_child(old_child, new_path.dentry); + + old_parent = dget(old_child->d_parent); + if (d_unhashed(old_child)) { + err = -EINVAL; + goto out3; } - if (d_really_is_negative(src_dent_parent)) - return -ENOENT; - if (d_really_is_negative(dst_dent_parent)) - return -ENOENT; - if (d_really_is_negative(src_dent)) - return -ENOENT; - if (src_dent == trap_dent) - return -EINVAL; + parent_fp = ksmbd_lookup_fd_inode(d_inode(old_child->d_parent)); + if (parent_fp) { + if (parent_fp->daccess & FILE_DELETE_LE) { + pr_err("parent dir is opened with delete access\n"); + err = -ESHARE; + ksmbd_fd_put(work, parent_fp); + goto out3; + } + ksmbd_fd_put(work, parent_fp); + } - if (ksmbd_override_fsids(work)) - return -ENOMEM; + new_dentry = lookup_one_qstr_excl(&new_last, new_path.dentry, + lookup_flags | LOOKUP_RENAME_TARGET); + if (IS_ERR(new_dentry)) { + err = PTR_ERR(new_dentry); + goto out3; + } - dst_dent = lookup_one(dst_idmap, dst_name, - dst_dent_parent, strlen(dst_name)); - err = PTR_ERR(dst_dent); - if (IS_ERR(dst_dent)) { - pr_err("lookup failed %s [%d]\n", dst_name, err); - goto out; + if (d_is_symlink(new_dentry)) { + err = -EACCES; + goto out4; } - err = -ENOTEMPTY; - if (dst_dent != trap_dent && !d_really_is_positive(dst_dent)) { - struct renamedata rd = { - .old_mnt_idmap = src_idmap, - .old_dir = d_inode(src_dent_parent), - .old_dentry = src_dent, - .new_mnt_idmap = dst_idmap, - .new_dir = d_inode(dst_dent_parent), - .new_dentry = dst_dent, - }; - err = vfs_rename(&rd); + if ((flags & RENAME_NOREPLACE) && d_is_positive(new_dentry)) { + err = -EEXIST; + goto out4; } - if (err) - pr_err("vfs_rename failed err %d\n", err); - if (dst_dent) - dput(dst_dent); -out: - ksmbd_revert_fsids(work); - return err; -} -int ksmbd_vfs_fp_rename(struct ksmbd_work *work, struct ksmbd_file *fp, - char *newname) -{ - struct mnt_idmap *idmap; - struct path dst_path; - struct dentry *src_dent_parent, *dst_dent_parent; - struct dentry *src_dent, *trap_dent, *src_child; - char *dst_name; - int err; + if (old_child == trap) { + err = -EINVAL; + goto out4; + } - dst_name = extract_last_component(newname); - if (!dst_name) { - dst_name = newname; - newname = ""; + if (new_dentry == trap) { + err = -ENOTEMPTY; + goto out4; } - src_dent_parent = dget_parent(fp->filp->f_path.dentry); - src_dent = fp->filp->f_path.dentry; + rd.old_mnt_idmap = mnt_idmap(old_path->mnt), + rd.old_dir = d_inode(old_parent), + rd.old_dentry = old_child, + rd.new_mnt_idmap = mnt_idmap(new_path.mnt), + rd.new_dir = new_path.dentry->d_inode, + rd.new_dentry = new_dentry, + rd.flags = flags, + err = vfs_rename(&rd); + if (err) + ksmbd_debug(VFS, "vfs_rename failed err %d\n", err); - err = ksmbd_vfs_kern_path(work, newname, - LOOKUP_NO_SYMLINKS | LOOKUP_DIRECTORY, - &dst_path, false); - if (err) { - ksmbd_debug(VFS, "Cannot get path for %s [%d]\n", newname, err); - goto out; +out4: + dput(new_dentry); +out3: + dput(old_parent); + unlock_rename(old_parent, new_path.dentry); +out2: + path_put(&new_path); + + if (retry_estale(err, lookup_flags)) { + lookup_flags |= LOOKUP_REVAL; + goto retry; } - dst_dent_parent = dst_path.dentry; - - trap_dent = lock_rename(src_dent_parent, dst_dent_parent); - dget(src_dent); - dget(dst_dent_parent); - idmap = file_mnt_idmap(fp->filp); - src_child = lookup_one(idmap, src_dent->d_name.name, src_dent_parent, - src_dent->d_name.len); - if (IS_ERR(src_child)) { - err = PTR_ERR(src_child); - goto out_lock; - } - - if (src_child != src_dent) { - err = -ESTALE; - dput(src_child); - goto out_lock; - } - dput(src_child); - - err = __ksmbd_vfs_rename(work, - idmap, - src_dent_parent, - src_dent, - mnt_idmap(dst_path.mnt), - dst_dent_parent, - trap_dent, - dst_name); -out_lock: - dput(src_dent); - dput(dst_dent_parent); - unlock_rename(src_dent_parent, dst_dent_parent); - path_put(&dst_path); -out: - dput(src_dent_parent); +out1: + putname(to); +revert_fsids: + ksmbd_revert_fsids(work); return err; } @@ -1079,14 +1013,16 @@ int ksmbd_vfs_remove_xattr(struct mnt_idmap *idmap, return vfs_removexattr(idmap, dentry, attr_name); } -int ksmbd_vfs_unlink(struct mnt_idmap *idmap, - struct dentry *dir, struct dentry *dentry) +int ksmbd_vfs_unlink(struct file *filp) { int err = 0; + struct dentry *dir, *dentry = filp->f_path.dentry; + struct mnt_idmap *idmap = file_mnt_idmap(filp); - err = ksmbd_vfs_lock_parent(idmap, dir, dentry); + dir = dget_parent(dentry); + err = ksmbd_vfs_lock_parent(dir, dentry); if (err) - return err; + goto out; dget(dentry); if (S_ISDIR(d_inode(dentry)->i_mode)) @@ -1098,6 +1034,8 @@ int ksmbd_vfs_unlink(struct mnt_idmap *idmap, inode_unlock(d_inode(dir)); if (err) ksmbd_debug(VFS, "failed to delete, err %d\n", err); +out: + dput(dir); return err; } @@ -1208,24 +1146,20 @@ static int ksmbd_vfs_lookup_in_dir(const struct path *dir, char *name, * * Return: 0 on success, otherwise error */ -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 ksmbd_share_config *share_conf = work->tcon->share_conf; int err; + struct path parent_path; - flags |= LOOKUP_BENEATH; - err = vfs_path_lookup(share_conf->vfs_path.dentry, - share_conf->vfs_path.mnt, - name, - flags, - path); + err = ksmbd_vfs_path_lookup_locked(share_conf, name, flags, path); if (!err) - return 0; + return err; if (caseless) { char *filepath; - struct path parent; size_t path_len, remain_len; filepath = kstrdup(name, GFP_KERNEL); @@ -1235,10 +1169,10 @@ int ksmbd_vfs_kern_path(struct ksmbd_work *work, char *name, path_len = strlen(filepath); remain_len = path_len; - parent = share_conf->vfs_path; - path_get(&parent); + parent_path = share_conf->vfs_path; + path_get(&parent_path); - while (d_can_lookup(parent.dentry)) { + while (d_can_lookup(parent_path.dentry)) { char *filename = filepath + path_len - remain_len; char *next = strchrnul(filename, '/'); size_t filename_len = next - filename; @@ -1247,12 +1181,11 @@ int ksmbd_vfs_kern_path(struct ksmbd_work *work, char *name, if (filename_len == 0) break; - err = ksmbd_vfs_lookup_in_dir(&parent, filename, + err = ksmbd_vfs_lookup_in_dir(&parent_path, filename, filename_len, work->conn->um); - path_put(&parent); if (err) - goto out; + goto out2; next[0] = '\0'; @@ -1260,23 +1193,31 @@ int ksmbd_vfs_kern_path(struct ksmbd_work *work, char *name, share_conf->vfs_path.mnt, filepath, flags, - &parent); + path); if (err) - goto out; - else if (is_last) { - *path = parent; - goto out; - } + goto out2; + else if (is_last) + goto out1; + path_put(&parent_path); + parent_path = *path; next[0] = '/'; remain_len -= filename_len + 1; } - path_put(&parent); err = -EINVAL; -out: +out2: + path_put(&parent_path); +out1: kfree(filepath); } + + if (!err) { + err = ksmbd_vfs_lock_parent(parent_path.dentry, path->dentry); + if (err) + dput(path->dentry); + path_put(&parent_path); + } return err; } diff --git a/fs/ksmbd/vfs.h b/fs/ksmbd/vfs.h index 9d676ab0cd25..a4ae89f3230d 100644 --- a/fs/ksmbd/vfs.h +++ b/fs/ksmbd/vfs.h @@ -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, diff --git a/fs/ksmbd/vfs_cache.c b/fs/ksmbd/vfs_cache.c index 054a7d2e0f48..2d0138e72d78 100644 --- a/fs/ksmbd/vfs_cache.c +++ b/fs/ksmbd/vfs_cache.c @@ -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); diff --git a/fs/namei.c b/fs/namei.c index 984d4f7b47dc..025fe5c52a24 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -254,6 +254,7 @@ getname_kernel(const char * filename) return result; } +EXPORT_SYMBOL(getname_kernel); void putname(struct filename *name) { @@ -271,6 +272,7 @@ void putname(struct filename *name) } else __putname(name); } +EXPORT_SYMBOL(putname); /** * check_acl - perform ACL permission checking @@ -1581,8 +1583,9 @@ static struct dentry *lookup_dcache(const struct qstr *name, * when directory is guaranteed to have no in-lookup children * at all. */ -static struct dentry *__lookup_hash(const struct qstr *name, - struct dentry *base, unsigned int flags) +struct dentry *lookup_one_qstr_excl(const struct qstr *name, + struct dentry *base, + unsigned int flags) { struct dentry *dentry = lookup_dcache(name, base, flags); struct dentry *old; @@ -1606,6 +1609,7 @@ static struct dentry *__lookup_hash(const struct qstr *name, } return dentry; } +EXPORT_SYMBOL(lookup_one_qstr_excl); static struct dentry *lookup_fast(struct nameidata *nd) { @@ -2532,16 +2536,17 @@ static int path_parentat(struct nameidata *nd, unsigned flags, } /* Note: this does not consume "name" */ -static int filename_parentat(int dfd, struct filename *name, - unsigned int flags, struct path *parent, - struct qstr *last, int *type) +static int __filename_parentat(int dfd, struct filename *name, + unsigned int flags, struct path *parent, + struct qstr *last, int *type, + const struct path *root) { int retval; struct nameidata nd; if (IS_ERR(name)) return PTR_ERR(name); - set_nameidata(&nd, dfd, name, NULL); + set_nameidata(&nd, dfd, name, root); retval = path_parentat(&nd, flags | LOOKUP_RCU, parent); if (unlikely(retval == -ECHILD)) retval = path_parentat(&nd, flags, parent); @@ -2556,6 +2561,13 @@ static int filename_parentat(int dfd, struct filename *name, return retval; } +static int filename_parentat(int dfd, struct filename *name, + unsigned int flags, struct path *parent, + struct qstr *last, int *type) +{ + return __filename_parentat(dfd, name, flags, parent, last, type, NULL); +} + /* does lookup, returns the object with parent locked */ static struct dentry *__kern_path_locked(struct filename *name, struct path *path) { @@ -2571,7 +2583,7 @@ static struct dentry *__kern_path_locked(struct filename *name, struct path *pat return ERR_PTR(-EINVAL); } inode_lock_nested(path->dentry->d_inode, I_MUTEX_PARENT); - d = __lookup_hash(&last, path->dentry, 0); + d = lookup_one_qstr_excl(&last, path->dentry, 0); if (IS_ERR(d)) { inode_unlock(path->dentry->d_inode); path_put(path); @@ -2599,6 +2611,24 @@ int kern_path(const char *name, unsigned int flags, struct path *path) } EXPORT_SYMBOL(kern_path); +/** + * vfs_path_parent_lookup - lookup a parent path relative to a dentry-vfsmount pair + * @filename: filename structure + * @flags: lookup flags + * @parent: pointer to struct path to fill + * @last: last component + * @type: type of the last component + * @root: pointer to struct path of the base directory + */ +int vfs_path_parent_lookup(struct filename *filename, unsigned int flags, + struct path *parent, struct qstr *last, int *type, + const struct path *root) +{ + return __filename_parentat(AT_FDCWD, filename, flags, parent, last, + type, root); +} +EXPORT_SYMBOL(vfs_path_parent_lookup); + /** * vfs_path_lookup - lookup a file path relative to a dentry-vfsmount pair * @dentry: pointer to dentry of the base directory @@ -3852,7 +3882,8 @@ static struct dentry *filename_create(int dfd, struct filename *name, if (last.name[last.len] && !want_dir) create_flags = 0; inode_lock_nested(path->dentry->d_inode, I_MUTEX_PARENT); - dentry = __lookup_hash(&last, path->dentry, reval_flag | create_flags); + dentry = lookup_one_qstr_excl(&last, path->dentry, + reval_flag | create_flags); if (IS_ERR(dentry)) goto unlock; @@ -4212,7 +4243,7 @@ int do_rmdir(int dfd, struct filename *name) goto exit2; inode_lock_nested(path.dentry->d_inode, I_MUTEX_PARENT); - dentry = __lookup_hash(&last, path.dentry, lookup_flags); + dentry = lookup_one_qstr_excl(&last, path.dentry, lookup_flags); error = PTR_ERR(dentry); if (IS_ERR(dentry)) goto exit3; @@ -4345,7 +4376,7 @@ int do_unlinkat(int dfd, struct filename *name) goto exit2; retry_deleg: inode_lock_nested(path.dentry->d_inode, I_MUTEX_PARENT); - dentry = __lookup_hash(&last, path.dentry, lookup_flags); + dentry = lookup_one_qstr_excl(&last, path.dentry, lookup_flags); error = PTR_ERR(dentry); if (!IS_ERR(dentry)) { @@ -4909,7 +4940,8 @@ int do_renameat2(int olddfd, struct filename *from, int newdfd, retry_deleg: trap = lock_rename(new_path.dentry, old_path.dentry); - old_dentry = __lookup_hash(&old_last, old_path.dentry, lookup_flags); + old_dentry = lookup_one_qstr_excl(&old_last, old_path.dentry, + lookup_flags); error = PTR_ERR(old_dentry); if (IS_ERR(old_dentry)) goto exit3; @@ -4917,7 +4949,8 @@ int do_renameat2(int olddfd, struct filename *from, int newdfd, error = -ENOENT; if (d_is_negative(old_dentry)) goto exit4; - new_dentry = __lookup_hash(&new_last, new_path.dentry, lookup_flags | target_flags); + new_dentry = lookup_one_qstr_excl(&new_last, new_path.dentry, + lookup_flags | target_flags); error = PTR_ERR(new_dentry); if (IS_ERR(new_dentry)) goto exit4; diff --git a/include/linux/namei.h b/include/linux/namei.h index 4c070fb29a91..85b12a17a23b 100644 --- a/include/linux/namei.h +++ b/include/linux/namei.h @@ -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 *); -- 2.25.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v8 3/3] ksmbd: fix racy issue from using ->d_parent and ->d_name 2023-03-15 22:34 ` [PATCH v8 3/3] ksmbd: fix racy issue from using ->d_parent and ->d_name Namjae Jeon @ 2023-03-16 0:30 ` kernel test robot 2023-03-16 0:56 ` Namjae Jeon 2023-03-16 7:30 ` kernel test robot 1 sibling, 1 reply; 14+ messages in thread From: kernel test robot @ 2023-03-16 0:30 UTC (permalink / raw) To: Namjae Jeon; +Cc: oe-kbuild-all 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/202303160854.zjX0rhqS-lkp@intel.com/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 <lkp@intel.com> | Link: https://lore.kernel.org/oe-kbuild-all/202303160854.zjX0rhqS-lkp@intel.com/ 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 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v8 3/3] ksmbd: fix racy issue from using ->d_parent and ->d_name 2023-03-16 0:30 ` kernel test robot @ 2023-03-16 0:56 ` Namjae Jeon 0 siblings, 0 replies; 14+ messages in thread From: Namjae Jeon @ 2023-03-16 0:56 UTC (permalink / raw) To: kernel test robot; +Cc: oe-kbuild-all 2023-03-16 9:30 GMT+09:00, kernel test robot <lkp@intel.com>: > Hi Namjae, Hi, First, Thanks for your report! I will fix it on next version. Thanks! > > 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/202303160854.zjX0rhqS-lkp@intel.com/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 <lkp@intel.com> > | Link: > https://lore.kernel.org/oe-kbuild-all/202303160854.zjX0rhqS-lkp@intel.com/ > > 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 > > -- > 0-DAY CI Kernel Test Service > https://github.com/intel/lkp-tests > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v8 3/3] ksmbd: fix racy issue from using ->d_parent and ->d_name 2023-03-15 22:34 ` [PATCH v8 3/3] ksmbd: fix racy issue from using ->d_parent and ->d_name Namjae Jeon 2023-03-16 0:30 ` kernel test robot @ 2023-03-16 7:30 ` kernel test robot 1 sibling, 0 replies; 14+ messages in thread From: kernel test robot @ 2023-03-16 7:30 UTC (permalink / raw) To: Namjae Jeon; +Cc: llvm, oe-kbuild-all 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/202303161501.AIXGVwna-lkp@intel.com/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 <lkp@intel.com> | Link: https://lore.kernel.org/oe-kbuild-all/202303161501.AIXGVwna-lkp@intel.com/ 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 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v8 0/3] ksmbd patches included vfs changes 2023-03-15 22:34 [PATCH v8 0/3] ksmbd patches included vfs changes Namjae Jeon ` (2 preceding siblings ...) 2023-03-15 22:34 ` [PATCH v8 3/3] ksmbd: fix racy issue from using ->d_parent and ->d_name Namjae Jeon @ 2023-04-21 2:35 ` Al Viro 2023-04-21 2:38 ` Al Viro 2023-04-21 7:26 ` Namjae Jeon 2023-06-13 10:57 ` Amir Goldstein 4 siblings, 2 replies; 14+ messages in thread From: Al Viro @ 2023-04-21 2:35 UTC (permalink / raw) To: Namjae Jeon Cc: linux-fsdevel, linux-cifs, smfrench, senozhatsky, tom, brauner 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v8 0/3] ksmbd patches included vfs changes 2023-04-21 2:35 ` [PATCH v8 0/3] ksmbd patches included vfs changes Al Viro @ 2023-04-21 2:38 ` Al Viro 2023-04-21 7:26 ` Namjae Jeon 1 sibling, 0 replies; 14+ messages in thread From: Al Viro @ 2023-04-21 2:38 UTC (permalink / raw) To: Namjae Jeon Cc: linux-fsdevel, linux-cifs, smfrench, senozhatsky, tom, brauner On Fri, Apr 21, 2023 at 03:35:00AM +0100, Al Viro wrote: > 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v8 0/3] ksmbd patches included vfs changes 2023-04-21 2:35 ` [PATCH v8 0/3] ksmbd patches included vfs changes Al Viro 2023-04-21 2:38 ` Al Viro @ 2023-04-21 7:26 ` Namjae Jeon 1 sibling, 0 replies; 14+ messages in thread From: Namjae Jeon @ 2023-04-21 7:26 UTC (permalink / raw) To: Al Viro; +Cc: linux-fsdevel, linux-cifs, smfrench, senozhatsky, tom, brauner 2023-04-21 11:35 GMT+09:00, Al Viro <viro@zeniv.linux.org.uk>: > 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. Okay. Can I add your acked-by in third patch ? Thank you! > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v8 0/3] ksmbd patches included vfs changes 2023-03-15 22:34 [PATCH v8 0/3] ksmbd patches included vfs changes Namjae Jeon ` (3 preceding siblings ...) 2023-04-21 2:35 ` [PATCH v8 0/3] ksmbd patches included vfs changes Al Viro @ 2023-06-13 10:57 ` Amir Goldstein 2023-06-13 12:24 ` Namjae Jeon 4 siblings, 1 reply; 14+ messages in thread From: Amir Goldstein @ 2023-06-13 10:57 UTC (permalink / raw) To: Namjae Jeon Cc: viro, linux-fsdevel, linux-cifs, smfrench, senozhatsky, tom, brauner 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. On Thu, Mar 16, 2023 at 12:37 AM Namjae Jeon <linkinjeon@kernel.org> wrote: > > 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 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v8 0/3] ksmbd patches included vfs changes 2023-06-13 10:57 ` Amir Goldstein @ 2023-06-13 12:24 ` Namjae Jeon 0 siblings, 0 replies; 14+ messages in thread From: Namjae Jeon @ 2023-06-13 12:24 UTC (permalink / raw) To: Amir Goldstein Cc: viro, linux-fsdevel, linux-cifs, smfrench, senozhatsky, tom, brauner 2023-06-13 19:57 GMT+09:00, Amir Goldstein <amir73il@gmail.com>: > Hi Namjae, Hi Amir, > > 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! > > Thanks, > Amir. > > On Thu, Mar 16, 2023 at 12:37 AM Namjae Jeon <linkinjeon@kernel.org> wrote: >> >> 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 >> > ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2023-06-13 12:24 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-03-15 22:34 [PATCH v8 0/3] ksmbd patches included vfs changes Namjae Jeon 2023-03-15 22:34 ` [PATCH v8 1/3] ksmbd: remove internal.h include Namjae Jeon 2023-03-17 8:55 ` Christian Brauner 2023-03-15 22:34 ` [PATCH v8 2/3] fs: introduce lock_rename_child() helper Namjae Jeon 2023-03-16 0:46 ` [lkp] [+118 bytes kernel size regression] [i386-tinyconfig] [6e6937ac88] " kernel test robot 2023-03-15 22:34 ` [PATCH v8 3/3] ksmbd: fix racy issue from using ->d_parent and ->d_name Namjae Jeon 2023-03-16 0:30 ` kernel test robot 2023-03-16 0:56 ` Namjae Jeon 2023-03-16 7:30 ` kernel test robot 2023-04-21 2:35 ` [PATCH v8 0/3] ksmbd patches included vfs changes Al Viro 2023-04-21 2:38 ` Al Viro 2023-04-21 7:26 ` Namjae Jeon 2023-06-13 10:57 ` Amir Goldstein 2023-06-13 12:24 ` Namjae Jeon
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.