| Message ID | 20210923092459.740044-1-hyc.lee@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | ksmbd: use LOOKUP_BENEATH to prevent the out of share access | expand |
2021-09-23 18:24 GMT+09:00, Hyunchul Lee <hyc.lee@gmail.com>: > instead of removing '..' in a given path, call > kern_path with LOOKUP_BENEATH flag to prevent > the out of share access. > > ran various test on this: > smb2-cat-async smb://127.0.0.1/homes/../out_of_share > smb2-cat-async smb://127.0.0.1/homes/foo/../../out_of_share > smbclient //127.0.0.1/homes -c "mkdir ../foo2" > smbclient //127.0.0.1/homes -c "rename bar ../bar" > > Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> > Cc: Ralph Boehme <slow@samba.org> > Cc: Steve French <smfrench@gmail.com> > Cc: Namjae Jeon <linkinjeon@kernel.org> > Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com> > --- > fs/ksmbd/misc.c | 76 ++++++---------------------------------------- > fs/ksmbd/misc.h | 3 +- > fs/ksmbd/smb2pdu.c | 51 ++++++++++++++----------------- > fs/ksmbd/vfs.c | 67 +++++++++++++++++++++++++--------------- > fs/ksmbd/vfs.h | 3 +- > 5 files changed, 78 insertions(+), 122 deletions(-) > > diff --git a/fs/ksmbd/misc.c b/fs/ksmbd/misc.c > index 3eac3c01749f..0b307ca28a19 100644 > --- a/fs/ksmbd/misc.c > +++ b/fs/ksmbd/misc.c > @@ -191,77 +191,19 @@ int get_nlink(struct kstat *st) > return nlink; > } > > -char *ksmbd_conv_path_to_unix(char *path) > +void ksmbd_conv_path_to_unix(char *path) > { > - size_t path_len, remain_path_len, out_path_len; > - char *out_path, *out_next; > - int i, pre_dotdot_cnt = 0, slash_cnt = 0; > - bool is_last; > - > strreplace(path, '\\', '/'); > - path_len = strlen(path); > - remain_path_len = path_len; > - if (path_len == 0) > - return ERR_PTR(-EINVAL); > - > - out_path = kzalloc(path_len + 2, GFP_KERNEL); > - if (!out_path) > - return ERR_PTR(-ENOMEM); > - out_path_len = 0; > - out_next = out_path; > - > - do { > - char *name = path + path_len - remain_path_len; > - char *next = strchrnul(name, '/'); > - size_t name_len = next - name; > - > - is_last = !next[0]; > - if (name_len == 2 && name[0] == '.' && name[1] == '.') { > - pre_dotdot_cnt++; > - /* handle the case that path ends with "/.." */ > - if (is_last) > - goto follow_dotdot; > - } else { > - if (pre_dotdot_cnt) { > -follow_dotdot: > - slash_cnt = 0; > - for (i = out_path_len - 1; i >= 0; i--) { > - if (out_path[i] == '/' && > - ++slash_cnt == pre_dotdot_cnt + 1) > - break; > - } > - > - if (i < 0 && > - slash_cnt != pre_dotdot_cnt) { > - kfree(out_path); > - return ERR_PTR(-EINVAL); > - } > - > - out_next = &out_path[i+1]; > - *out_next = '\0'; > - out_path_len = i + 1; > - > - } > - > - if (name_len != 0 && > - !(name_len == 1 && name[0] == '.') && > - !(name_len == 2 && name[0] == '.' && name[1] == '.')) { > - next[0] = '\0'; > - sprintf(out_next, "%s/", name); > - out_next += name_len + 1; > - out_path_len += name_len + 1; > - next[0] = '/'; > - } > - pre_dotdot_cnt = 0; > - } > +} > > - remain_path_len -= name_len + 1; > - } while (!is_last); > +void ksmbd_strip_last_slash(char *path) > +{ > + int len = strlen(path); > > - if (out_path_len > 0) > - out_path[out_path_len-1] = '\0'; > - path[path_len] = '\0'; > - return out_path; > + while (len && path[len - 1] == '/') { > + path[len - 1] = '\0'; > + len--; > + } > } > > void ksmbd_conv_path_to_windows(char *path) > diff --git a/fs/ksmbd/misc.h b/fs/ksmbd/misc.h > index b7b10139ada2..af8717d4d85b 100644 > --- a/fs/ksmbd/misc.h > +++ b/fs/ksmbd/misc.h > @@ -16,7 +16,8 @@ int ksmbd_validate_filename(char *filename); > int parse_stream_name(char *filename, char **stream_name, int *s_type); > char *convert_to_nt_pathname(char *filename, char *sharepath); > int get_nlink(struct kstat *st); > -char *ksmbd_conv_path_to_unix(char *path); > +void ksmbd_conv_path_to_unix(char *path); > +void ksmbd_strip_last_slash(char *path); > void ksmbd_conv_path_to_windows(char *path); > char *ksmbd_extract_sharename(char *treename); > char *convert_to_unix_name(struct ksmbd_share_config *share, char *name); > diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c > index a4b237d4042b..7c5d205bdb22 100644 > --- a/fs/ksmbd/smb2pdu.c > +++ b/fs/ksmbd/smb2pdu.c > @@ -642,7 +642,7 @@ static char * > smb2_get_name(struct ksmbd_share_config *share, const char *src, > const int maxlen, struct nls_table *local_nls) > { > - char *name, *norm_name, *unixname; > + char *name, *unixname; > > name = smb_strndup_from_utf16(src, maxlen, 1, local_nls); > if (IS_ERR(name)) { > @@ -651,15 +651,11 @@ smb2_get_name(struct ksmbd_share_config *share, const > char *src, > } > > /* change it to absolute unix name */ > - norm_name = ksmbd_conv_path_to_unix(name); > - if (IS_ERR(norm_name)) { > - kfree(name); > - return norm_name; > - } > - kfree(name); > + ksmbd_conv_path_to_unix(name); > + ksmbd_strip_last_slash(name); > > - unixname = convert_to_unix_name(share, norm_name); > - kfree(norm_name); > + unixname = convert_to_unix_name(share, name); > + kfree(name); > if (!unixname) { > pr_err("can not convert absolute name\n"); > return ERR_PTR(-ENOMEM); > @@ -2412,7 +2408,7 @@ static int smb2_creat(struct ksmbd_work *work, struct > path *path, char *name, > return rc; > } > > - rc = ksmbd_vfs_kern_path(name, 0, path, 0); > + rc = ksmbd_vfs_kern_path(work, name, 0, path, 0); > if (rc) { > pr_err("cannot get linux path (%s), err = %d\n", > name, rc); > @@ -2487,7 +2483,7 @@ int smb2_open(struct ksmbd_work *work) > struct oplock_info *opinfo; > __le32 *next_ptr = NULL; > int req_op_level = 0, open_flags = 0, may_flags = 0, file_info = 0; > - int rc = 0, len = 0; > + int rc = 0; > int contxt_cnt = 0, query_disk_id = 0; > int maximal_access_ctxt = 0, posix_ctxt = 0; > int s_type = 0; > @@ -2559,17 +2555,12 @@ int smb2_open(struct ksmbd_work *work) > goto err_out1; > } > } else { > - len = strlen(share->path); > - ksmbd_debug(SMB, "share path len %d\n", len); > - name = kmalloc(len + 1, GFP_KERNEL); > + name = kstrdup(share->path, GFP_KERNEL); > if (!name) { > rsp->hdr.Status = STATUS_NO_MEMORY; > rc = -ENOMEM; > goto err_out1; > } > - > - memcpy(name, share->path, len); > - *(name + len) = '\0'; > } > > req_op_level = req->RequestedOplockLevel; > @@ -2692,7 +2683,7 @@ int smb2_open(struct ksmbd_work *work) > goto err_out1; > } > > - rc = ksmbd_vfs_kern_path(name, LOOKUP_NO_SYMLINKS, &path, 1); > + rc = ksmbd_vfs_kern_path(work, name, LOOKUP_NO_SYMLINKS, &path, 1); > if (!rc) { > if (req->CreateOptions & FILE_DELETE_ON_CLOSE_LE) { > /* > @@ -2721,7 +2712,7 @@ int smb2_open(struct ksmbd_work *work) > } > > if (rc) { > - if (rc == -EACCES) { > + if (rc != -ENOENT) { > ksmbd_debug(SMB, > "User does not have right permission\n"); > goto err_out; > @@ -3229,7 +3220,7 @@ int smb2_open(struct ksmbd_work *work) > rsp->hdr.Status = STATUS_INVALID_PARAMETER; > else if (rc == -EOPNOTSUPP) > rsp->hdr.Status = STATUS_NOT_SUPPORTED; > - else if (rc == -EACCES || rc == -ESTALE) > + else if (rc == -EACCES || rc == -ESTALE || rc == -EXDEV) > rsp->hdr.Status = STATUS_ACCESS_DENIED; > else if (rc == -ENOENT) > rsp->hdr.Status = STATUS_OBJECT_NAME_INVALID; > @@ -4801,7 +4792,7 @@ static int smb2_get_info_filesystem(struct ksmbd_work > *work, > int rc = 0, len; > int fs_infoclass_size = 0; > > - rc = ksmbd_vfs_kern_path(share->path, LOOKUP_NO_SYMLINKS, &path, 0); > + rc = ksmbd_vfs_kern_path(work, share->path, LOOKUP_NO_SYMLINKS, &path, > 0); > if (rc) { > pr_err("cannot create vfs path\n"); > return -EIO; > @@ -5378,10 +5369,12 @@ static int smb2_rename(struct ksmbd_work *work, > } > > ksmbd_debug(SMB, "new name %s\n", new_name); > - rc = ksmbd_vfs_kern_path(new_name, LOOKUP_NO_SYMLINKS, &path, 1); > - if (rc) > + rc = ksmbd_vfs_kern_path(work, new_name, LOOKUP_NO_SYMLINKS, &path, 1); > + if (rc) { > + if (rc != -ENOENT) > + goto out; > file_present = false; > - else > + } else > path_put(&path); > > if (ksmbd_share_veto_filename(share, new_name)) { > @@ -5456,10 +5449,12 @@ static int smb2_create_link(struct ksmbd_work > *work, > } > > ksmbd_debug(SMB, "target name is %s\n", target_name); > - rc = ksmbd_vfs_kern_path(link_name, LOOKUP_NO_SYMLINKS, &path, 0); > - if (rc) > + rc = ksmbd_vfs_kern_path(work, link_name, LOOKUP_NO_SYMLINKS, &path, 0); > + if (rc) { > + if (rc != -ENOENT) > + goto out; > file_present = false; > - else > + } else > path_put(&path); > > if (file_info->ReplaceIfExists) { > @@ -5975,7 +5970,7 @@ int smb2_set_info(struct ksmbd_work *work) > return 0; > > err_out: > - if (rc == -EACCES || rc == -EPERM) > + if (rc == -EACCES || rc == -EPERM || rc == -EXDEV) > rsp->hdr.Status = STATUS_ACCESS_DENIED; > else if (rc == -EINVAL) > rsp->hdr.Status = STATUS_INVALID_PARAMETER; > diff --git a/fs/ksmbd/vfs.c b/fs/ksmbd/vfs.c > index 3733e4944c1d..c5ff3d1e1ca4 100644 > --- a/fs/ksmbd/vfs.c > +++ b/fs/ksmbd/vfs.c > @@ -19,6 +19,8 @@ > #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" > @@ -1213,33 +1215,48 @@ static int ksmbd_vfs_lookup_in_dir(struct path *dir, > char *name, size_t namelen) > * > * Return: 0 on success, otherwise error > */ > -int ksmbd_vfs_kern_path(char *name, unsigned int flags, struct path *path, > - bool caseless) > +int ksmbd_vfs_kern_path(struct ksmbd_work *work, char *name, > + unsigned int flags, struct path *path, bool caseless) > { > + struct ksmbd_share_config *share_conf = work->tcon->share_conf; > + char *filepath; > int err; > > - if (name[0] != '/') > + /* name must start with the share path */ > + if (strlen(name) < share_conf->path_sz || > + strncmp(name, share_conf->path, share_conf->path_sz) != 0) > return -EINVAL; > + else if (strlen(name) == share_conf->path_sz) { > + *path = share_conf->vfs_path; > + path_get(path); > + return 0; > + } > + > + filepath = kstrdup(name + share_conf->path_sz + 1, It seems to be for skipping the share path. When converting name, we don't need append path name given from request to share path if we use vfs_path_lookup(). we can also optimize convert_to_nt_pathname() function if share path is not included in name. Thanks! > + GFP_KERNEL); > + if (!filepath) > + return -ENOMEM; > > - err = kern_path(name, flags, path); > - if (!err) > + flags |= LOOKUP_BENEATH; > + err = vfs_path_lookup(share_conf->vfs_path.dentry, > + share_conf->vfs_path.mnt, > + filepath, > + flags, > + path); > + if (!err) { > + kfree(filepath); > return 0; > + } > > if (caseless) { > - char *filepath; > struct path parent; > size_t path_len, remain_len; > > - filepath = kstrdup(name, GFP_KERNEL); > - if (!filepath) > - return -ENOMEM; > - > path_len = strlen(filepath); > - remain_len = path_len - 1; > + remain_len = path_len; > > - err = kern_path("/", flags, &parent); > - if (err) > - goto out; > + parent = share_conf->vfs_path; > + path_get(&parent); > > while (d_can_lookup(parent.dentry)) { > char *filename = filepath + path_len - remain_len; > @@ -1252,21 +1269,21 @@ int ksmbd_vfs_kern_path(char *name, unsigned int > flags, struct path *path, > > err = ksmbd_vfs_lookup_in_dir(&parent, filename, > filename_len); > - if (err) { > - path_put(&parent); > + path_put(&parent); > + if (err) > goto out; > - } > > - path_put(&parent); > next[0] = '\0'; > > - err = kern_path(filepath, flags, &parent); > + err = vfs_path_lookup(share_conf->vfs_path.dentry, > + share_conf->vfs_path.mnt, > + filepath, > + flags, > + &parent); > if (err) > goto out; > - > - if (is_last) { > - path->mnt = parent.mnt; > - path->dentry = parent.dentry; > + else if (is_last) { > + *path = parent; > goto out; > } > > @@ -1276,9 +1293,9 @@ int ksmbd_vfs_kern_path(char *name, unsigned int > flags, struct path *path, > > path_put(&parent); > err = -EINVAL; > -out: > - kfree(filepath); > } > +out: > + kfree(filepath); > return err; > } > > diff --git a/fs/ksmbd/vfs.h b/fs/ksmbd/vfs.h > index 85db50abdb24..7089c39e9271 100644 > --- a/fs/ksmbd/vfs.h > +++ b/fs/ksmbd/vfs.h > @@ -152,7 +152,8 @@ 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 user_namespace *user_ns, > struct dentry *dentry, char *attr_name); > -int ksmbd_vfs_kern_path(char *name, unsigned int flags, struct path *path, > +int ksmbd_vfs_kern_path(struct ksmbd_work *work, > + char *name, unsigned int flags, struct path *path, > bool caseless); > int ksmbd_vfs_empty_dir(struct ksmbd_file *fp); > void ksmbd_vfs_set_fadvise(struct file *filp, __le32 option); > -- > 2.25.1 > >
diff --git a/fs/ksmbd/misc.c b/fs/ksmbd/misc.c index 3eac3c01749f..0b307ca28a19 100644 --- a/fs/ksmbd/misc.c +++ b/fs/ksmbd/misc.c @@ -191,77 +191,19 @@ int get_nlink(struct kstat *st) return nlink; } -char *ksmbd_conv_path_to_unix(char *path) +void ksmbd_conv_path_to_unix(char *path) { - size_t path_len, remain_path_len, out_path_len; - char *out_path, *out_next; - int i, pre_dotdot_cnt = 0, slash_cnt = 0; - bool is_last; - strreplace(path, '\\', '/'); - path_len = strlen(path); - remain_path_len = path_len; - if (path_len == 0) - return ERR_PTR(-EINVAL); - - out_path = kzalloc(path_len + 2, GFP_KERNEL); - if (!out_path) - return ERR_PTR(-ENOMEM); - out_path_len = 0; - out_next = out_path; - - do { - char *name = path + path_len - remain_path_len; - char *next = strchrnul(name, '/'); - size_t name_len = next - name; - - is_last = !next[0]; - if (name_len == 2 && name[0] == '.' && name[1] == '.') { - pre_dotdot_cnt++; - /* handle the case that path ends with "/.." */ - if (is_last) - goto follow_dotdot; - } else { - if (pre_dotdot_cnt) { -follow_dotdot: - slash_cnt = 0; - for (i = out_path_len - 1; i >= 0; i--) { - if (out_path[i] == '/' && - ++slash_cnt == pre_dotdot_cnt + 1) - break; - } - - if (i < 0 && - slash_cnt != pre_dotdot_cnt) { - kfree(out_path); - return ERR_PTR(-EINVAL); - } - - out_next = &out_path[i+1]; - *out_next = '\0'; - out_path_len = i + 1; - - } - - if (name_len != 0 && - !(name_len == 1 && name[0] == '.') && - !(name_len == 2 && name[0] == '.' && name[1] == '.')) { - next[0] = '\0'; - sprintf(out_next, "%s/", name); - out_next += name_len + 1; - out_path_len += name_len + 1; - next[0] = '/'; - } - pre_dotdot_cnt = 0; - } +} - remain_path_len -= name_len + 1; - } while (!is_last); +void ksmbd_strip_last_slash(char *path) +{ + int len = strlen(path); - if (out_path_len > 0) - out_path[out_path_len-1] = '\0'; - path[path_len] = '\0'; - return out_path; + while (len && path[len - 1] == '/') { + path[len - 1] = '\0'; + len--; + } } void ksmbd_conv_path_to_windows(char *path) diff --git a/fs/ksmbd/misc.h b/fs/ksmbd/misc.h index b7b10139ada2..af8717d4d85b 100644 --- a/fs/ksmbd/misc.h +++ b/fs/ksmbd/misc.h @@ -16,7 +16,8 @@ int ksmbd_validate_filename(char *filename); int parse_stream_name(char *filename, char **stream_name, int *s_type); char *convert_to_nt_pathname(char *filename, char *sharepath); int get_nlink(struct kstat *st); -char *ksmbd_conv_path_to_unix(char *path); +void ksmbd_conv_path_to_unix(char *path); +void ksmbd_strip_last_slash(char *path); void ksmbd_conv_path_to_windows(char *path); char *ksmbd_extract_sharename(char *treename); char *convert_to_unix_name(struct ksmbd_share_config *share, char *name); diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index a4b237d4042b..7c5d205bdb22 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -642,7 +642,7 @@ static char * smb2_get_name(struct ksmbd_share_config *share, const char *src, const int maxlen, struct nls_table *local_nls) { - char *name, *norm_name, *unixname; + char *name, *unixname; name = smb_strndup_from_utf16(src, maxlen, 1, local_nls); if (IS_ERR(name)) { @@ -651,15 +651,11 @@ smb2_get_name(struct ksmbd_share_config *share, const char *src, } /* change it to absolute unix name */ - norm_name = ksmbd_conv_path_to_unix(name); - if (IS_ERR(norm_name)) { - kfree(name); - return norm_name; - } - kfree(name); + ksmbd_conv_path_to_unix(name); + ksmbd_strip_last_slash(name); - unixname = convert_to_unix_name(share, norm_name); - kfree(norm_name); + unixname = convert_to_unix_name(share, name); + kfree(name); if (!unixname) { pr_err("can not convert absolute name\n"); return ERR_PTR(-ENOMEM); @@ -2412,7 +2408,7 @@ static int smb2_creat(struct ksmbd_work *work, struct path *path, char *name, return rc; } - rc = ksmbd_vfs_kern_path(name, 0, path, 0); + rc = ksmbd_vfs_kern_path(work, name, 0, path, 0); if (rc) { pr_err("cannot get linux path (%s), err = %d\n", name, rc); @@ -2487,7 +2483,7 @@ int smb2_open(struct ksmbd_work *work) struct oplock_info *opinfo; __le32 *next_ptr = NULL; int req_op_level = 0, open_flags = 0, may_flags = 0, file_info = 0; - int rc = 0, len = 0; + int rc = 0; int contxt_cnt = 0, query_disk_id = 0; int maximal_access_ctxt = 0, posix_ctxt = 0; int s_type = 0; @@ -2559,17 +2555,12 @@ int smb2_open(struct ksmbd_work *work) goto err_out1; } } else { - len = strlen(share->path); - ksmbd_debug(SMB, "share path len %d\n", len); - name = kmalloc(len + 1, GFP_KERNEL); + name = kstrdup(share->path, GFP_KERNEL); if (!name) { rsp->hdr.Status = STATUS_NO_MEMORY; rc = -ENOMEM; goto err_out1; } - - memcpy(name, share->path, len); - *(name + len) = '\0'; } req_op_level = req->RequestedOplockLevel; @@ -2692,7 +2683,7 @@ int smb2_open(struct ksmbd_work *work) goto err_out1; } - rc = ksmbd_vfs_kern_path(name, LOOKUP_NO_SYMLINKS, &path, 1); + rc = ksmbd_vfs_kern_path(work, name, LOOKUP_NO_SYMLINKS, &path, 1); if (!rc) { if (req->CreateOptions & FILE_DELETE_ON_CLOSE_LE) { /* @@ -2721,7 +2712,7 @@ int smb2_open(struct ksmbd_work *work) } if (rc) { - if (rc == -EACCES) { + if (rc != -ENOENT) { ksmbd_debug(SMB, "User does not have right permission\n"); goto err_out; @@ -3229,7 +3220,7 @@ int smb2_open(struct ksmbd_work *work) rsp->hdr.Status = STATUS_INVALID_PARAMETER; else if (rc == -EOPNOTSUPP) rsp->hdr.Status = STATUS_NOT_SUPPORTED; - else if (rc == -EACCES || rc == -ESTALE) + else if (rc == -EACCES || rc == -ESTALE || rc == -EXDEV) rsp->hdr.Status = STATUS_ACCESS_DENIED; else if (rc == -ENOENT) rsp->hdr.Status = STATUS_OBJECT_NAME_INVALID; @@ -4801,7 +4792,7 @@ static int smb2_get_info_filesystem(struct ksmbd_work *work, int rc = 0, len; int fs_infoclass_size = 0; - rc = ksmbd_vfs_kern_path(share->path, LOOKUP_NO_SYMLINKS, &path, 0); + rc = ksmbd_vfs_kern_path(work, share->path, LOOKUP_NO_SYMLINKS, &path, 0); if (rc) { pr_err("cannot create vfs path\n"); return -EIO; @@ -5378,10 +5369,12 @@ static int smb2_rename(struct ksmbd_work *work, } ksmbd_debug(SMB, "new name %s\n", new_name); - rc = ksmbd_vfs_kern_path(new_name, LOOKUP_NO_SYMLINKS, &path, 1); - if (rc) + rc = ksmbd_vfs_kern_path(work, new_name, LOOKUP_NO_SYMLINKS, &path, 1); + if (rc) { + if (rc != -ENOENT) + goto out; file_present = false; - else + } else path_put(&path); if (ksmbd_share_veto_filename(share, new_name)) { @@ -5456,10 +5449,12 @@ static int smb2_create_link(struct ksmbd_work *work, } ksmbd_debug(SMB, "target name is %s\n", target_name); - rc = ksmbd_vfs_kern_path(link_name, LOOKUP_NO_SYMLINKS, &path, 0); - if (rc) + rc = ksmbd_vfs_kern_path(work, link_name, LOOKUP_NO_SYMLINKS, &path, 0); + if (rc) { + if (rc != -ENOENT) + goto out; file_present = false; - else + } else path_put(&path); if (file_info->ReplaceIfExists) { @@ -5975,7 +5970,7 @@ int smb2_set_info(struct ksmbd_work *work) return 0; err_out: - if (rc == -EACCES || rc == -EPERM) + if (rc == -EACCES || rc == -EPERM || rc == -EXDEV) rsp->hdr.Status = STATUS_ACCESS_DENIED; else if (rc == -EINVAL) rsp->hdr.Status = STATUS_INVALID_PARAMETER; diff --git a/fs/ksmbd/vfs.c b/fs/ksmbd/vfs.c index 3733e4944c1d..c5ff3d1e1ca4 100644 --- a/fs/ksmbd/vfs.c +++ b/fs/ksmbd/vfs.c @@ -19,6 +19,8 @@ #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" @@ -1213,33 +1215,48 @@ static int ksmbd_vfs_lookup_in_dir(struct path *dir, char *name, size_t namelen) * * Return: 0 on success, otherwise error */ -int ksmbd_vfs_kern_path(char *name, unsigned int flags, struct path *path, - bool caseless) +int ksmbd_vfs_kern_path(struct ksmbd_work *work, char *name, + unsigned int flags, struct path *path, bool caseless) { + struct ksmbd_share_config *share_conf = work->tcon->share_conf; + char *filepath; int err; - if (name[0] != '/') + /* name must start with the share path */ + if (strlen(name) < share_conf->path_sz || + strncmp(name, share_conf->path, share_conf->path_sz) != 0) return -EINVAL; + else if (strlen(name) == share_conf->path_sz) { + *path = share_conf->vfs_path; + path_get(path); + return 0; + } + + filepath = kstrdup(name + share_conf->path_sz + 1, + GFP_KERNEL); + if (!filepath) + return -ENOMEM; - err = kern_path(name, flags, path); - if (!err) + flags |= LOOKUP_BENEATH; + err = vfs_path_lookup(share_conf->vfs_path.dentry, + share_conf->vfs_path.mnt, + filepath, + flags, + path); + if (!err) { + kfree(filepath); return 0; + } if (caseless) { - char *filepath; struct path parent; size_t path_len, remain_len; - filepath = kstrdup(name, GFP_KERNEL); - if (!filepath) - return -ENOMEM; - path_len = strlen(filepath); - remain_len = path_len - 1; + remain_len = path_len; - err = kern_path("/", flags, &parent); - if (err) - goto out; + parent = share_conf->vfs_path; + path_get(&parent); while (d_can_lookup(parent.dentry)) { char *filename = filepath + path_len - remain_len; @@ -1252,21 +1269,21 @@ int ksmbd_vfs_kern_path(char *name, unsigned int flags, struct path *path, err = ksmbd_vfs_lookup_in_dir(&parent, filename, filename_len); - if (err) { - path_put(&parent); + path_put(&parent); + if (err) goto out; - } - path_put(&parent); next[0] = '\0'; - err = kern_path(filepath, flags, &parent); + err = vfs_path_lookup(share_conf->vfs_path.dentry, + share_conf->vfs_path.mnt, + filepath, + flags, + &parent); if (err) goto out; - - if (is_last) { - path->mnt = parent.mnt; - path->dentry = parent.dentry; + else if (is_last) { + *path = parent; goto out; } @@ -1276,9 +1293,9 @@ int ksmbd_vfs_kern_path(char *name, unsigned int flags, struct path *path, path_put(&parent); err = -EINVAL; -out: - kfree(filepath); } +out: + kfree(filepath); return err; } diff --git a/fs/ksmbd/vfs.h b/fs/ksmbd/vfs.h index 85db50abdb24..7089c39e9271 100644 --- a/fs/ksmbd/vfs.h +++ b/fs/ksmbd/vfs.h @@ -152,7 +152,8 @@ 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 user_namespace *user_ns, struct dentry *dentry, char *attr_name); -int ksmbd_vfs_kern_path(char *name, unsigned int flags, struct path *path, +int ksmbd_vfs_kern_path(struct ksmbd_work *work, + char *name, unsigned int flags, struct path *path, bool caseless); int ksmbd_vfs_empty_dir(struct ksmbd_file *fp); void ksmbd_vfs_set_fadvise(struct file *filp, __le32 option);
instead of removing '..' in a given path, call kern_path with LOOKUP_BENEATH flag to prevent the out of share access. ran various test on this: smb2-cat-async smb://127.0.0.1/homes/../out_of_share smb2-cat-async smb://127.0.0.1/homes/foo/../../out_of_share smbclient //127.0.0.1/homes -c "mkdir ../foo2" smbclient //127.0.0.1/homes -c "rename bar ../bar" Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Ralph Boehme <slow@samba.org> Cc: Steve French <smfrench@gmail.com> Cc: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com> --- fs/ksmbd/misc.c | 76 ++++++---------------------------------------- fs/ksmbd/misc.h | 3 +- fs/ksmbd/smb2pdu.c | 51 ++++++++++++++----------------- fs/ksmbd/vfs.c | 67 +++++++++++++++++++++++++--------------- fs/ksmbd/vfs.h | 3 +- 5 files changed, 78 insertions(+), 122 deletions(-)