Skip to content

Commit 4e45cca

Browse files
neilbrownsmfrench
neilbrown
authored and
smfrench
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 <neil@brown.name> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
1 parent d5fc140 commit 4e45cca

3 files changed

Lines changed: 111 additions & 71 deletions

File tree

‎fs/smb/server/smb2pdu.c‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2581,7 +2581,7 @@ static void smb2_update_xattrs(struct ksmbd_tree_connect *tcon,
25812581
}
25822582
}
25832583

2584-
static int smb2_creat(struct ksmbd_work *work, struct path *parent_path,
2584+
static int smb2_creat(struct ksmbd_work *work,
25852585
struct path *path, char *name, int open_flags,
25862586
umode_t posix_mode, bool is_dir)
25872587
{
@@ -2610,7 +2610,7 @@ static int smb2_creat(struct ksmbd_work *work, struct path *parent_path,
26102610
return rc;
26112611
}
26122612

2613-
rc = ksmbd_vfs_kern_path_locked(work, name, 0, parent_path, path, 0);
2613+
rc = ksmbd_vfs_kern_path(work, name, 0, path, 0);
26142614
if (rc) {
26152615
pr_err("cannot get linux path (%s), err = %d\n",
26162616
name, rc);
@@ -2860,7 +2860,7 @@ int smb2_open(struct ksmbd_work *work)
28602860
struct ksmbd_tree_connect *tcon = work->tcon;
28612861
struct smb2_create_req *req;
28622862
struct smb2_create_rsp *rsp;
2863-
struct path path, parent_path;
2863+
struct path path;
28642864
struct ksmbd_share_config *share = tcon->share_conf;
28652865
struct ksmbd_file *fp = NULL;
28662866
struct file *filp = NULL;
@@ -3116,8 +3116,8 @@ int smb2_open(struct ksmbd_work *work)
31163116
goto err_out2;
31173117
}
31183118

3119-
rc = ksmbd_vfs_kern_path_locked(work, name, LOOKUP_NO_SYMLINKS,
3120-
&parent_path, &path, 1);
3119+
rc = ksmbd_vfs_kern_path(work, name, LOOKUP_NO_SYMLINKS,
3120+
&path, 1);
31213121
if (!rc) {
31223122
file_present = true;
31233123

@@ -3238,7 +3238,7 @@ int smb2_open(struct ksmbd_work *work)
32383238

32393239
/*create file if not present */
32403240
if (!file_present) {
3241-
rc = smb2_creat(work, &parent_path, &path, name, open_flags,
3241+
rc = smb2_creat(work, &path, name, open_flags,
32423242
posix_mode,
32433243
req->CreateOptions & FILE_DIRECTORY_FILE_LE);
32443244
if (rc) {
@@ -3443,7 +3443,7 @@ int smb2_open(struct ksmbd_work *work)
34433443
}
34443444

34453445
if (file_present || created)
3446-
ksmbd_vfs_kern_path_unlock(&parent_path, &path);
3446+
path_put(&path);
34473447

34483448
if (!S_ISDIR(file_inode(filp)->i_mode) && open_flags & O_TRUNC &&
34493449
!fp->attrib_only && !stream_name) {
@@ -3724,7 +3724,7 @@ int smb2_open(struct ksmbd_work *work)
37243724

37253725
err_out:
37263726
if (rc && (file_present || created))
3727-
ksmbd_vfs_kern_path_unlock(&parent_path, &path);
3727+
path_put(&path);
37283728

37293729
err_out1:
37303730
ksmbd_revert_fsids(work);
@@ -6036,7 +6036,7 @@ static int smb2_create_link(struct ksmbd_work *work,
60366036
struct nls_table *local_nls)
60376037
{
60386038
char *link_name = NULL, *target_name = NULL, *pathname = NULL;
6039-
struct path path, parent_path;
6039+
struct path path;
60406040
int rc;
60416041

60426042
if (buf_len < (u64)sizeof(struct smb2_file_link_info) +
@@ -6065,7 +6065,7 @@ static int smb2_create_link(struct ksmbd_work *work,
60656065

60666066
ksmbd_debug(SMB, "target name is %s\n", target_name);
60676067
rc = ksmbd_vfs_kern_path_locked(work, link_name, LOOKUP_NO_SYMLINKS,
6068-
&parent_path, &path, 0);
6068+
&path, 0);
60696069
if (rc) {
60706070
if (rc != -ENOENT)
60716071
goto out;
@@ -6083,7 +6083,7 @@ static int smb2_create_link(struct ksmbd_work *work,
60836083
ksmbd_debug(SMB, "link already exists\n");
60846084
goto out;
60856085
}
6086-
ksmbd_vfs_kern_path_unlock(&parent_path, &path);
6086+
ksmbd_vfs_kern_path_unlock(&path);
60876087
}
60886088
rc = ksmbd_vfs_link(work, target_name, link_name);
60896089
if (rc)

‎fs/smb/server/vfs.c‎

Lines changed: 95 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,12 @@ int ksmbd_vfs_lock_parent(struct dentry *parent, struct dentry *child)
6666
return 0;
6767
}
6868

69-
static int ksmbd_vfs_path_lookup_locked(struct ksmbd_share_config *share_conf,
70-
char *pathname, unsigned int flags,
71-
struct path *parent_path,
72-
struct path *path)
69+
static int ksmbd_vfs_path_lookup(struct ksmbd_share_config *share_conf,
70+
char *pathname, unsigned int flags,
71+
struct path *path, bool do_lock)
7372
{
7473
struct qstr last;
75-
struct filename *filename;
74+
struct filename *filename __free(putname) = NULL;
7675
struct path *root_share_path = &share_conf->vfs_path;
7776
int err, type;
7877
struct dentry *d;
@@ -89,51 +88,57 @@ static int ksmbd_vfs_path_lookup_locked(struct ksmbd_share_config *share_conf,
8988
return PTR_ERR(filename);
9089

9190
err = vfs_path_parent_lookup(filename, flags,
92-
parent_path, &last, &type,
91+
path, &last, &type,
9392
root_share_path);
94-
if (err) {
95-
putname(filename);
93+
if (err)
9694
return err;
97-
}
9895

9996
if (unlikely(type != LAST_NORM)) {
100-
path_put(parent_path);
101-
putname(filename);
97+
path_put(path);
10298
return -ENOENT;
10399
}
104100

105-
err = mnt_want_write(parent_path->mnt);
106-
if (err) {
107-
path_put(parent_path);
108-
putname(filename);
101+
if (do_lock) {
102+
err = mnt_want_write(path->mnt);
103+
if (err) {
104+
path_put(path);
105+
return -ENOENT;
106+
}
107+
108+
inode_lock_nested(path->dentry->d_inode, I_MUTEX_PARENT);
109+
d = lookup_one_qstr_excl(&last, path->dentry, 0);
110+
111+
if (!IS_ERR(d)) {
112+
dput(path->dentry);
113+
path->dentry = d;
114+
return 0;
115+
}
116+
inode_unlock(path->dentry->d_inode);
117+
mnt_drop_write(path->mnt);
118+
path_put(path);
109119
return -ENOENT;
110120
}
111121

112-
inode_lock_nested(parent_path->dentry->d_inode, I_MUTEX_PARENT);
113-
d = lookup_one_qstr_excl(&last, parent_path->dentry, 0);
114-
if (IS_ERR(d))
115-
goto err_out;
116-
122+
d = lookup_noperm_unlocked(&last, path->dentry);
123+
if (!IS_ERR(d) && d_is_negative(d)) {
124+
dput(d);
125+
d = ERR_PTR(-ENOENT);
126+
}
127+
if (IS_ERR(d)) {
128+
path_put(path);
129+
return -ENOENT;
130+
}
131+
dput(path->dentry);
117132
path->dentry = d;
118-
path->mnt = mntget(parent_path->mnt);
119133

120134
if (test_share_config_flag(share_conf, KSMBD_SHARE_FLAG_CROSSMNT)) {
121135
err = follow_down(path, 0);
122136
if (err < 0) {
123137
path_put(path);
124-
goto err_out;
138+
return -ENOENT;
125139
}
126140
}
127-
128-
putname(filename);
129141
return 0;
130-
131-
err_out:
132-
inode_unlock(d_inode(parent_path->dentry));
133-
mnt_drop_write(parent_path->mnt);
134-
path_put(parent_path);
135-
putname(filename);
136-
return -ENOENT;
137142
}
138143

139144
void ksmbd_vfs_query_maximal_access(struct mnt_idmap *idmap,
@@ -1198,38 +1203,28 @@ static int ksmbd_vfs_lookup_in_dir(const struct path *dir, char *name,
11981203
return ret;
11991204
}
12001205

1201-
/**
1202-
* ksmbd_vfs_kern_path_locked() - lookup a file and get path info
1203-
* @work: work
1204-
* @filepath: file path that is relative to share
1205-
* @flags: lookup flags
1206-
* @parent_path: if lookup succeed, return parent_path info
1207-
* @path: if lookup succeed, return path info
1208-
* @caseless: caseless filename lookup
1209-
*
1210-
* Return: 0 on success, otherwise error
1211-
*/
1212-
int ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char *filepath,
1213-
unsigned int flags, struct path *parent_path,
1214-
struct path *path, bool caseless)
1206+
static
1207+
int __ksmbd_vfs_kern_path(struct ksmbd_work *work, char *filepath,
1208+
unsigned int flags,
1209+
struct path *path, bool caseless, bool do_lock)
12151210
{
12161211
struct ksmbd_share_config *share_conf = work->tcon->share_conf;
1212+
struct path parent_path;
12171213
size_t path_len, remain_len;
12181214
int err;
12191215

12201216
retry:
1221-
err = ksmbd_vfs_path_lookup_locked(share_conf, filepath, flags, parent_path,
1222-
path);
1217+
err = ksmbd_vfs_path_lookup(share_conf, filepath, flags, path, do_lock);
12231218
if (!err || !caseless)
12241219
return err;
12251220

12261221
path_len = strlen(filepath);
12271222
remain_len = path_len;
12281223

1229-
*parent_path = share_conf->vfs_path;
1230-
path_get(parent_path);
1224+
parent_path = share_conf->vfs_path;
1225+
path_get(&parent_path);
12311226

1232-
while (d_can_lookup(parent_path->dentry)) {
1227+
while (d_can_lookup(parent_path.dentry)) {
12331228
char *filename = filepath + path_len - remain_len;
12341229
char *next = strchrnul(filename, '/');
12351230
size_t filename_len = next - filename;
@@ -1238,10 +1233,10 @@ int ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char *filepath,
12381233
if (filename_len == 0)
12391234
break;
12401235

1241-
err = ksmbd_vfs_lookup_in_dir(parent_path, filename,
1236+
err = ksmbd_vfs_lookup_in_dir(&parent_path, filename,
12421237
filename_len,
12431238
work->conn->um);
1244-
path_put(parent_path);
1239+
path_put(&parent_path);
12451240
if (err)
12461241
goto out;
12471242
if (is_last) {
@@ -1254,7 +1249,7 @@ int ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char *filepath,
12541249
share_conf->vfs_path.mnt,
12551250
filepath,
12561251
flags,
1257-
parent_path);
1252+
&parent_path);
12581253
next[0] = '/';
12591254
if (err)
12601255
goto out;
@@ -1263,17 +1258,59 @@ int ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char *filepath,
12631258
}
12641259

12651260
err = -EINVAL;
1266-
path_put(parent_path);
1261+
path_put(&parent_path);
12671262
out:
12681263
return err;
12691264
}
12701265

1271-
void ksmbd_vfs_kern_path_unlock(struct path *parent_path, struct path *path)
1266+
/**
1267+
* ksmbd_vfs_kern_path() - lookup a file and get path info
1268+
* @work: work
1269+
* @filepath: file path that is relative to share
1270+
* @flags: lookup flags
1271+
* @path: if lookup succeed, return path info
1272+
* @caseless: caseless filename lookup
1273+
*
1274+
* Perform the lookup, possibly crossing over any mount point.
1275+
* On return no locks will be held and write-access to filesystem
1276+
* won't have been checked.
1277+
* Return: 0 if file was found, otherwise error
1278+
*/
1279+
int ksmbd_vfs_kern_path(struct ksmbd_work *work, char *filepath,
1280+
unsigned int flags,
1281+
struct path *path, bool caseless)
1282+
{
1283+
return __ksmbd_vfs_kern_path(work, filepath, flags, path,
1284+
caseless, false);
1285+
}
1286+
1287+
/**
1288+
* ksmbd_vfs_kern_path_locked() - lookup a file and get path info
1289+
* @work: work
1290+
* @filepath: file path that is relative to share
1291+
* @flags: lookup flags
1292+
* @path: if lookup succeed, return path info
1293+
* @caseless: caseless filename lookup
1294+
*
1295+
* Perform the lookup, but don't cross over any mount point.
1296+
* On return the parent of path->dentry will be locked and write-access to
1297+
* filesystem will have been gained.
1298+
* Return: 0 on if file was found, otherwise error
1299+
*/
1300+
int ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char *filepath,
1301+
unsigned int flags,
1302+
struct path *path, bool caseless)
1303+
{
1304+
return __ksmbd_vfs_kern_path(work, filepath, flags, path,
1305+
caseless, true);
1306+
}
1307+
1308+
void ksmbd_vfs_kern_path_unlock(struct path *path)
12721309
{
1273-
inode_unlock(d_inode(parent_path->dentry));
1274-
mnt_drop_write(parent_path->mnt);
1310+
/* While lock is still held, ->d_parent is safe */
1311+
inode_unlock(d_inode(path->dentry->d_parent));
1312+
mnt_drop_write(path->mnt);
12751313
path_put(path);
1276-
path_put(parent_path);
12771314
}
12781315

12791316
struct dentry *ksmbd_vfs_kern_path_create(struct ksmbd_work *work,

‎fs/smb/server/vfs.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,13 @@ int ksmbd_vfs_xattr_stream_name(char *stream_name, char **xattr_stream_name,
117117
int ksmbd_vfs_remove_xattr(struct mnt_idmap *idmap,
118118
const struct path *path, char *attr_name,
119119
bool get_write);
120+
int ksmbd_vfs_kern_path(struct ksmbd_work *work, char *name,
121+
unsigned int flags,
122+
struct path *path, bool caseless);
120123
int ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char *name,
121-
unsigned int flags, struct path *parent_path,
124+
unsigned int flags,
122125
struct path *path, bool caseless);
123-
void ksmbd_vfs_kern_path_unlock(struct path *parent_path, struct path *path);
126+
void ksmbd_vfs_kern_path_unlock(struct path *path);
124127
struct dentry *ksmbd_vfs_kern_path_create(struct ksmbd_work *work,
125128
const char *name,
126129
unsigned int flags,

0 commit comments

Comments
 (0)