Skip to content

Commit f13c6c8

Browse files
neilbrownnamjaejeon
neilbrown
authored and
namjaejeon
committed
smb/server: add ksmbd_vfs_kern_path()
The function ksmbd_vfs_kern_path_locked() seems to serve two functions and as a result has an odd interface. On success it returns with the parent directory locked and with write access on that filesystem requested, but it may have crossed over a mount point to return the path, which makes the lock and the write access irrelevant. This patches separates the functionality into two functions: - ksmbd_vfs_kern_path() does not lock the parent, does not request write access, but does cross mount points - ksmbd_vfs_kern_path_locked() does not cross mount points but does lock the parent and request write access. The parent_path parameter is no longer needed. For the _locked case the final path is sufficient to drop write access and to unlock the parent (using path->dentry->d_parent which is safe while the lock is held). There were 3 caller of ksmbd_vfs_kern_path_locked(). - smb2_create_link() needs to remove the target if it existed and needs the lock and the write-access, so it continues to use ksmbd_vfs_kern_path_locked(). It would not make sense to cross mount points in this case. - smb2_open() is the only user that needs to cross mount points and it has no need for the lock or write access, so it now uses ksmbd_vfs_kern_path() - smb2_creat() does not need to cross mountpoints as it is accessing a file that it has just created on *this* filesystem. But also it does not need the lock or write access because by the time ksmbd_vfs_kern_path_locked() was called it has already created the file. So it could use either interface. It is simplest to use ksmbd_vfs_kern_path(). ksmbd_vfs_kern_path_unlock() is still needed after ksmbd_vfs_kern_path_locked() but it doesn't require the parent_path any more. After a successful call to ksmbd_vfs_kern_path(), only path_put() is needed to release the path. Signed-off-by: NeilBrown <[email protected]> Signed-off-by: Namjae Jeon <[email protected]>
1 parent b75df2b commit f13c6c8

File tree

3 files changed

+118
-98
lines changed

3 files changed

+118
-98
lines changed

‎smb2pdu.c

Lines changed: 5 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2626,7 +2626,7 @@ static void smb2_update_xattrs(struct ksmbd_tree_connect *tcon,
26262626
}
26272627

26282628
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 4, 0)
2629-
static int smb2_creat(struct ksmbd_work *work, struct path *parent_path,
2629+
static int smb2_creat(struct ksmbd_work *work,
26302630
struct path *path, char *name, int open_flags,
26312631
umode_t posix_mode, bool is_dir)
26322632
#else
@@ -2659,11 +2659,7 @@ static int smb2_creat(struct ksmbd_work *work, struct path *path, char *name,
26592659
return rc;
26602660
}
26612661

2662-
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 4, 0)
2663-
rc = ksmbd_vfs_kern_path_locked(work, name, 0, parent_path, path, 0);
2664-
#else
26652662
rc = ksmbd_vfs_kern_path(work, name, 0, path, 0);
2666-
#endif
26672663
if (rc) {
26682664
pr_err("cannot get linux path (%s), err = %d\n",
26692665
name, rc);
@@ -2941,9 +2937,6 @@ int smb2_open(struct ksmbd_work *work)
29412937
struct smb2_create_req *req;
29422938
struct smb2_create_rsp *rsp;
29432939
struct path path;
2944-
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 4, 0)
2945-
struct path parent_path;
2946-
#endif
29472940
struct ksmbd_share_config *share = tcon->share_conf;
29482941
struct ksmbd_file *fp = NULL;
29492942
struct file *filp = NULL;
@@ -3203,12 +3196,8 @@ int smb2_open(struct ksmbd_work *work)
32033196
goto err_out2;
32043197
}
32053198

3206-
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 4, 0)
3207-
rc = ksmbd_vfs_kern_path_locked(work, name, LOOKUP_NO_SYMLINKS,
3208-
&parent_path, &path, 1);
3209-
#else
3210-
rc = ksmbd_vfs_kern_path(work, name, LOOKUP_NO_SYMLINKS, &path, 1);
3211-
#endif
3199+
rc = ksmbd_vfs_kern_path(work, name, LOOKUP_NO_SYMLINKS,
3200+
&path, 1);
32123201
if (!rc) {
32133202
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 4, 0)
32143203
file_present = true;
@@ -3357,11 +3346,7 @@ int smb2_open(struct ksmbd_work *work)
33573346

33583347
/*create file if not present */
33593348
if (!file_present) {
3360-
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 4, 0)
3361-
rc = smb2_creat(work, &parent_path, &path, name, open_flags,
3362-
#else
33633349
rc = smb2_creat(work, &path, name, open_flags,
3364-
#endif
33653350
posix_mode,
33663351
req->CreateOptions & FILE_DIRECTORY_FILE_LE);
33673352
if (rc) {
@@ -3609,13 +3594,8 @@ int smb2_open(struct ksmbd_work *work)
36093594
goto err_out;
36103595
}
36113596

3612-
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 4, 0)
3613-
if (file_present || created)
3614-
ksmbd_vfs_kern_path_unlock(&parent_path, &path);
3615-
#else
36163597
if (file_present || created)
36173598
path_put(&path);
3618-
#endif
36193599

36203600
if (!S_ISDIR(file_inode(filp)->i_mode) && open_flags & O_TRUNC &&
36213601
!fp->attrib_only && !stream_name) {
@@ -3899,13 +3879,8 @@ int smb2_open(struct ksmbd_work *work)
38993879
}
39003880

39013881
err_out:
3902-
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 4, 0)
3903-
if (rc && (file_present || created))
3904-
ksmbd_vfs_kern_path_unlock(&parent_path, &path);
3905-
#else
39063882
if (rc && (file_present || created))
39073883
path_put(&path);
3908-
#endif
39093884
err_out1:
39103885
ksmbd_revert_fsids(work);
39113886
err_out2:
@@ -6488,9 +6463,6 @@ static int smb2_create_link(struct ksmbd_work *work,
64886463
{
64896464
char *link_name = NULL, *target_name = NULL, *pathname = NULL;
64906465
struct path path;
6491-
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 4, 0)
6492-
struct path parent_path;
6493-
#endif
64946466
int rc;
64956467

64966468
if (buf_len < (u64)sizeof(struct smb2_file_link_info) +
@@ -6520,7 +6492,7 @@ static int smb2_create_link(struct ksmbd_work *work,
65206492
ksmbd_debug(SMB, "target name is %s\n", target_name);
65216493
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 4, 0)
65226494
rc = ksmbd_vfs_kern_path_locked(work, link_name, LOOKUP_NO_SYMLINKS,
6523-
&parent_path, &path, 0);
6495+
&path, 0);
65246496
#else
65256497
rc = ksmbd_vfs_kern_path(work, link_name, LOOKUP_NO_SYMLINKS, &path, 0);
65266498
#endif
@@ -6549,7 +6521,7 @@ static int smb2_create_link(struct ksmbd_work *work,
65496521
goto out;
65506522
}
65516523
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 4, 0)
6552-
ksmbd_vfs_kern_path_unlock(&parent_path, &path);
6524+
ksmbd_vfs_kern_path_unlock(&path);
65536525
#endif
65546526
}
65556527
rc = ksmbd_vfs_link(work, target_name, link_name);

‎vfs.c

Lines changed: 108 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,9 @@ int ksmbd_vfs_lock_parent(struct dentry *parent, struct dentry *child)
9292
return 0;
9393
}
9494

95-
static int ksmbd_vfs_path_lookup_locked(struct ksmbd_share_config *share_conf,
96-
char *pathname, unsigned int flags,
97-
struct path *parent_path,
98-
struct path *path)
95+
static int ksmbd_vfs_path_lookup(struct ksmbd_share_config *share_conf,
96+
char *pathname, unsigned int flags,
97+
struct path *path, bool do_lock)
9998
{
10099
struct qstr last;
101100
struct filename *filename;
@@ -115,58 +114,73 @@ static int ksmbd_vfs_path_lookup_locked(struct ksmbd_share_config *share_conf,
115114
return PTR_ERR(filename);
116115

117116
err = vfs_path_parent_lookup(filename, flags,
118-
parent_path, &last, &type,
117+
path, &last, &type,
119118
root_share_path);
120-
if (err) {
121-
putname(filename);
119+
putname(filename);
120+
if (err)
122121
return err;
123-
}
124122

125123
if (unlikely(type != LAST_NORM)) {
126-
path_put(parent_path);
127-
putname(filename);
124+
path_put(path);
128125
return -ENOENT;
129126
}
130127

131-
err = mnt_want_write(parent_path->mnt);
132-
if (err) {
133-
path_put(parent_path);
134-
putname(filename);
135-
return -ENOENT;
136-
}
128+
if (do_lock) {
129+
err = mnt_want_write(path->mnt);
130+
if (err) {
131+
path_put(path);
132+
return -ENOENT;
133+
}
137134

138-
inode_lock_nested(parent_path->dentry->d_inode, I_MUTEX_PARENT);
139-
d = lookup_one_qstr_excl(&last, parent_path->dentry, 0);
140-
if (IS_ERR(d))
141-
goto err_out;
135+
inode_lock_nested(path->dentry->d_inode, I_MUTEX_PARENT);
136+
d = lookup_one_qstr_excl(&last, path->dentry, 0);
142137

138+
if (!IS_ERR(d)) {
143139
#if LINUX_VERSION_CODE < KERNEL_VERSION(6, 15, 0)
144-
if (d_is_negative(d)) {
145-
dput(d);
146-
goto err_out;
147-
}
140+
if (d_is_negative(d)) {
141+
dput(d);
142+
inode_unlock(path->dentry->d_inode);
143+
mnt_drop_write(path->mnt);
144+
path_put(path);
145+
return -ENOENT;
146+
}
148147
#endif
148+
dput(path->dentry);
149+
path->dentry = d;
150+
return 0;
151+
}
152+
inode_unlock(path->dentry->d_inode);
153+
mnt_drop_write(path->mnt);
154+
path_put(path);
155+
return -ENOENT;
156+
}
149157

158+
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 16, 0)
159+
d = lookup_noperm_unlocked(&last, path->dentry);
160+
#else
161+
inode_lock_nested(path->dentry->d_inode, I_MUTEX_PARENT);
162+
d = lookup_one_qstr_excl(&last, path->dentry, 0);
163+
inode_unlock(path->dentry->d_inode);
164+
#endif
165+
if (!IS_ERR(d) && d_is_negative(d)) {
166+
dput(d);
167+
d = ERR_PTR(-ENOENT);
168+
}
169+
if (IS_ERR(d)) {
170+
path_put(path);
171+
return -ENOENT;
172+
}
173+
dput(path->dentry);
150174
path->dentry = d;
151-
path->mnt = mntget(parent_path->mnt);
152175

153176
if (test_share_config_flag(share_conf, KSMBD_SHARE_FLAG_CROSSMNT)) {
154177
err = follow_down(path, 0);
155178
if (err < 0) {
156179
path_put(path);
157-
goto err_out;
180+
return -ENOENT;
158181
}
159182
}
160-
161-
putname(filename);
162183
return 0;
163-
164-
err_out:
165-
inode_unlock(d_inode(parent_path->dentry));
166-
mnt_drop_write(parent_path->mnt);
167-
path_put(parent_path);
168-
putname(filename);
169-
return -ENOENT;
170184
}
171185
#else
172186
/**
@@ -2836,38 +2850,28 @@ static int ksmbd_vfs_lookup_in_dir(const struct path *dir, char *name,
28362850
}
28372851

28382852
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 4, 0)
2839-
/**
2840-
* ksmbd_vfs_kern_path_locked() - lookup a file and get path info
2841-
* @work: work
2842-
* @filepath: file path that is relative to share
2843-
* @flags: lookup flags
2844-
* @parent_path: if lookup succeed, return parent_path info
2845-
* @path: if lookup succeed, return path info
2846-
* @caseless: caseless filename lookup
2847-
*
2848-
* Return: 0 on success, otherwise error
2849-
*/
2850-
int ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char *filepath,
2851-
unsigned int flags, struct path *parent_path,
2852-
struct path *path, bool caseless)
2853+
static
2854+
int __ksmbd_vfs_kern_path(struct ksmbd_work *work, char *filepath,
2855+
unsigned int flags,
2856+
struct path *path, bool caseless, bool do_lock)
28532857
{
28542858
struct ksmbd_share_config *share_conf = work->tcon->share_conf;
2859+
struct path parent_path;
28552860
size_t path_len, remain_len;
28562861
int err;
28572862

28582863
retry:
2859-
err = ksmbd_vfs_path_lookup_locked(share_conf, filepath, flags, parent_path,
2860-
path);
2864+
err = ksmbd_vfs_path_lookup(share_conf, filepath, flags, path, do_lock);
28612865
if (!err || !caseless)
28622866
return err;
28632867

28642868
path_len = strlen(filepath);
28652869
remain_len = path_len;
28662870

2867-
*parent_path = share_conf->vfs_path;
2868-
path_get(parent_path);
2871+
parent_path = share_conf->vfs_path;
2872+
path_get(&parent_path);
28692873

2870-
while (d_can_lookup(parent_path->dentry)) {
2874+
while (d_can_lookup(parent_path.dentry)) {
28712875
char *filename = filepath + path_len - remain_len;
28722876
char *next = strchrnul(filename, '/');
28732877
size_t filename_len = next - filename;
@@ -2876,10 +2880,10 @@ int ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char *filepath,
28762880
if (filename_len == 0)
28772881
break;
28782882

2879-
err = ksmbd_vfs_lookup_in_dir(parent_path, filename,
2883+
err = ksmbd_vfs_lookup_in_dir(&parent_path, filename,
28802884
filename_len,
28812885
work->conn->um);
2882-
path_put(parent_path);
2886+
path_put(&parent_path);
28832887
if (err)
28842888
goto out;
28852889
if (is_last) {
@@ -2892,7 +2896,7 @@ int ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char *filepath,
28922896
share_conf->vfs_path.mnt,
28932897
filepath,
28942898
flags,
2895-
parent_path);
2899+
&parent_path);
28962900
next[0] = '/';
28972901
if (err)
28982902
goto out;
@@ -2901,19 +2905,60 @@ int ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char *filepath,
29012905
}
29022906

29032907
err = -EINVAL;
2904-
path_put(parent_path);
2908+
path_put(&parent_path);
29052909
out:
29062910
return err;
29072911
}
29082912

2909-
void ksmbd_vfs_kern_path_unlock(struct path *parent_path, struct path *path)
2913+
/**
2914+
* ksmbd_vfs_kern_path() - lookup a file and get path info
2915+
* @work: work
2916+
* @filepath: file path that is relative to share
2917+
* @flags: lookup flags
2918+
* @path: if lookup succeed, return path info
2919+
* @caseless: caseless filename lookup
2920+
*
2921+
* Perform the lookup, possibly crossing over any mount point.
2922+
* On return no locks will be held and write-access to filesystem
2923+
* won't have been checked.
2924+
* Return: 0 if file was found, otherwise error
2925+
*/
2926+
int ksmbd_vfs_kern_path(struct ksmbd_work *work, char *filepath,
2927+
unsigned int flags,
2928+
struct path *path, bool caseless)
29102929
{
2911-
inode_unlock(d_inode(parent_path->dentry));
2912-
mnt_drop_write(parent_path->mnt);
2913-
path_put(path);
2914-
path_put(parent_path);
2930+
return __ksmbd_vfs_kern_path(work, filepath, flags, path,
2931+
caseless, false);
29152932
}
29162933

2934+
/**
2935+
* ksmbd_vfs_kern_path_locked() - lookup a file and get path info
2936+
* @work: work
2937+
* @filepath: file path that is relative to share
2938+
* @flags: lookup flags
2939+
* @path: if lookup succeed, return path info
2940+
* @caseless: caseless filename lookup
2941+
*
2942+
* Perform the lookup, but don't cross over any mount point.
2943+
* On return the parent of path->dentry will be locked and write-access to
2944+
* filesystem will have been gained.
2945+
* Return: 0 on if file was found, otherwise error
2946+
*/
2947+
int ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char *filepath,
2948+
unsigned int flags,
2949+
struct path *path, bool caseless)
2950+
{
2951+
return __ksmbd_vfs_kern_path(work, filepath, flags, path,
2952+
caseless, true);
2953+
}
2954+
2955+
void ksmbd_vfs_kern_path_unlock(struct path *path)
2956+
{
2957+
/* While lock is still held, ->d_parent is safe */
2958+
inode_unlock(d_inode(path->dentry->d_parent));
2959+
mnt_drop_write(path->mnt);
2960+
path_put(path);
2961+
}
29172962
#else
29182963
/**
29192964
* ksmbd_vfs_kern_path() - lookup a file and get path info

‎vfs.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,10 +238,13 @@ int ksmbd_vfs_remove_xattr(struct user_namespace *user_ns,
238238
const struct path *path, char *attr_name,
239239
bool get_write);
240240
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 4, 0)
241+
int ksmbd_vfs_kern_path(struct ksmbd_work *work, char *name,
242+
unsigned int flags,
243+
struct path *path, bool caseless);
241244
int ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char *name,
242-
unsigned int flags, struct path *parent_path,
245+
unsigned int flags,
243246
struct path *path, bool caseless);
244-
void ksmbd_vfs_kern_path_unlock(struct path *parent_path, struct path *path);
247+
void ksmbd_vfs_kern_path_unlock(struct path *path);
245248
#else
246249
int ksmbd_vfs_kern_path(struct ksmbd_work *work,
247250
char *name, unsigned int flags, struct path *path,

0 commit comments

Comments
 (0)