From da1e7ada5b62859b3a9d236a44035ae9d8f3f7e1 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Mon, 23 Aug 2021 17:13:47 +0200 Subject: ksmbd: fix lookup on idmapped mounts It's great that the new in-kernel ksmbd server will support idmapped mounts out of the box! However, lookup is currently broken. Lookup helpers such as lookup_one_len() call inode_permission() internally to ensure that the caller is privileged over the inode of the base dentry they are trying to lookup under. So the permission checking here is currently wrong. Linux v5.15 will gain a new lookup helper lookup_one() that does take idmappings into account. I've added it as part of my patch series to make btrfs support idmapped mounts. The new helper is in linux-next as part of David's (Sterba) btrfs for-next branch as commit c972214c133b ("namei: add mapping aware lookup helper"). I've said it before during one of my first reviews: I would very much recommend adding fstests to [1]. It already seems to have very rudimentary cifs support. There is a completely generic idmapped mount testsuite that supports idmapped mounts. [1]: https://git.kernel.org/pub/scm/fs/xfs/xfsprogs-dev.git/ Cc: Colin Ian King Cc: Steve French Cc: Christoph Hellwig Cc: Namjae Jeon Cc: Hyunchul Lee Cc: Sergey Senozhatsky Cc: David Sterba Cc: linux-cifs@vger.kernel.org Signed-off-by: Christian Brauner Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smb2pdu.c | 18 +++++++++++------- fs/ksmbd/vfs.c | 43 ++++++++++++++++++++++++------------------- fs/ksmbd/vfs.h | 3 ++- 3 files changed, 37 insertions(+), 27 deletions(-) (limited to 'fs') diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index d329ea49fa14..a400dd292af1 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -3543,9 +3543,9 @@ static int process_query_dir_entries(struct smb2_query_dir_private *priv) return -EINVAL; lock_dir(priv->dir_fp); - dent = lookup_one_len(priv->d_info->name, - priv->dir_fp->filp->f_path.dentry, - priv->d_info->name_len); + dent = lookup_one(user_ns, priv->d_info->name, + priv->dir_fp->filp->f_path.dentry, + priv->d_info->name_len); unlock_dir(priv->dir_fp); if (IS_ERR(dent)) { @@ -5246,7 +5246,9 @@ int smb2_echo(struct ksmbd_work *work) return 0; } -static int smb2_rename(struct ksmbd_work *work, struct ksmbd_file *fp, +static int smb2_rename(struct ksmbd_work *work, + struct ksmbd_file *fp, + struct user_namespace *user_ns, struct smb2_file_rename_info *file_info, struct nls_table *local_nls) { @@ -5310,7 +5312,7 @@ static int smb2_rename(struct ksmbd_work *work, struct ksmbd_file *fp, if (rc) goto out; - rc = ksmbd_vfs_setxattr(file_mnt_user_ns(fp->filp), + rc = ksmbd_vfs_setxattr(user_ns, fp->filp->f_path.dentry, xattr_stream_name, NULL, 0, 0); @@ -5624,6 +5626,7 @@ static int set_end_of_file_info(struct ksmbd_work *work, struct ksmbd_file *fp, static int set_rename_info(struct ksmbd_work *work, struct ksmbd_file *fp, char *buf) { + struct user_namespace *user_ns; struct ksmbd_file *parent_fp; struct dentry *parent; struct dentry *dentry = fp->filp->f_path.dentry; @@ -5634,11 +5637,12 @@ static int set_rename_info(struct ksmbd_work *work, struct ksmbd_file *fp, return -EACCES; } + user_ns = file_mnt_user_ns(fp->filp); if (ksmbd_stream_fd(fp)) goto next; parent = dget_parent(dentry); - ret = ksmbd_vfs_lock_parent(parent, dentry); + ret = ksmbd_vfs_lock_parent(user_ns, parent, dentry); if (ret) { dput(parent); return ret; @@ -5655,7 +5659,7 @@ static int set_rename_info(struct ksmbd_work *work, struct ksmbd_file *fp, } } next: - return smb2_rename(work, fp, + return smb2_rename(work, fp, user_ns, (struct smb2_file_rename_info *)buf, work->sess->conn->local_nls); } diff --git a/fs/ksmbd/vfs.c b/fs/ksmbd/vfs.c index aee28ee6b19c..2bb506d1fb32 100644 --- a/fs/ksmbd/vfs.c +++ b/fs/ksmbd/vfs.c @@ -69,14 +69,15 @@ static void ksmbd_vfs_inherit_owner(struct ksmbd_work *work, * * the reference count of @parent isn't incremented. */ -int ksmbd_vfs_lock_parent(struct dentry *parent, struct dentry *child) +int ksmbd_vfs_lock_parent(struct user_namespace *user_ns, struct dentry *parent, + struct dentry *child) { struct dentry *dentry; int ret = 0; inode_lock_nested(d_inode(parent), I_MUTEX_PARENT); - dentry = lookup_one_len(child->d_name.name, parent, - child->d_name.len); + dentry = lookup_one(user_ns, child->d_name.name, parent, + child->d_name.len); if (IS_ERR(dentry)) { ret = PTR_ERR(dentry); goto out_err; @@ -102,7 +103,7 @@ int ksmbd_vfs_may_delete(struct user_namespace *user_ns, int ret; parent = dget_parent(dentry); - ret = ksmbd_vfs_lock_parent(parent, dentry); + ret = ksmbd_vfs_lock_parent(user_ns, parent, dentry); if (ret) { dput(parent); return ret; @@ -137,7 +138,7 @@ int ksmbd_vfs_query_maximal_access(struct user_namespace *user_ns, *daccess |= FILE_EXECUTE_LE; parent = dget_parent(dentry); - ret = ksmbd_vfs_lock_parent(parent, dentry); + ret = ksmbd_vfs_lock_parent(user_ns, parent, dentry); if (ret) { dput(parent); return ret; @@ -197,6 +198,7 @@ int ksmbd_vfs_create(struct ksmbd_work *work, const char *name, umode_t mode) */ int ksmbd_vfs_mkdir(struct ksmbd_work *work, const char *name, umode_t mode) { + struct user_namespace *user_ns; struct path path; struct dentry *dentry; int err; @@ -210,16 +212,16 @@ int ksmbd_vfs_mkdir(struct ksmbd_work *work, const char *name, umode_t mode) return err; } + user_ns = mnt_user_ns(path.mnt); mode |= S_IFDIR; - err = vfs_mkdir(mnt_user_ns(path.mnt), d_inode(path.dentry), - dentry, mode); + err = vfs_mkdir(user_ns, d_inode(path.dentry), dentry, mode); if (err) { goto out; } else if (d_unhashed(dentry)) { struct dentry *d; - d = lookup_one_len(dentry->d_name.name, dentry->d_parent, - dentry->d_name.len); + d = lookup_one(user_ns, dentry->d_name.name, dentry->d_parent, + dentry->d_name.len); if (IS_ERR(d)) { err = PTR_ERR(d); goto out; @@ -582,6 +584,7 @@ int ksmbd_vfs_fsync(struct ksmbd_work *work, u64 fid, u64 p_id) */ int ksmbd_vfs_remove_file(struct ksmbd_work *work, char *name) { + struct user_namespace *user_ns; struct path path; struct dentry *parent; int err; @@ -601,8 +604,9 @@ int ksmbd_vfs_remove_file(struct ksmbd_work *work, char *name) return err; } + user_ns = mnt_user_ns(path.mnt); parent = dget_parent(path.dentry); - err = ksmbd_vfs_lock_parent(parent, path.dentry); + err = ksmbd_vfs_lock_parent(user_ns, parent, path.dentry); if (err) { dput(parent); path_put(&path); @@ -616,14 +620,12 @@ int ksmbd_vfs_remove_file(struct ksmbd_work *work, char *name) } if (S_ISDIR(d_inode(path.dentry)->i_mode)) { - err = vfs_rmdir(mnt_user_ns(path.mnt), d_inode(parent), - path.dentry); + err = vfs_rmdir(user_ns, d_inode(parent), path.dentry); if (err && err != -ENOTEMPTY) ksmbd_debug(VFS, "%s: rmdir failed, err %d\n", name, err); } else { - err = vfs_unlink(mnt_user_ns(path.mnt), d_inode(parent), - path.dentry, NULL); + err = vfs_unlink(user_ns, d_inode(parent), path.dentry, NULL); if (err) ksmbd_debug(VFS, "%s: unlink failed, err %d\n", name, err); @@ -748,7 +750,8 @@ static int __ksmbd_vfs_rename(struct ksmbd_work *work, if (ksmbd_override_fsids(work)) return -ENOMEM; - dst_dent = lookup_one_len(dst_name, dst_dent_parent, strlen(dst_name)); + dst_dent = lookup_one(dst_user_ns, 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); @@ -779,6 +782,7 @@ out: int ksmbd_vfs_fp_rename(struct ksmbd_work *work, struct ksmbd_file *fp, char *newname) { + struct user_namespace *user_ns; struct path dst_path; struct dentry *src_dent_parent, *dst_dent_parent; struct dentry *src_dent, *trap_dent, *src_child; @@ -808,8 +812,9 @@ int ksmbd_vfs_fp_rename(struct ksmbd_work *work, struct ksmbd_file *fp, trap_dent = lock_rename(src_dent_parent, dst_dent_parent); dget(src_dent); dget(dst_dent_parent); - src_child = lookup_one_len(src_dent->d_name.name, src_dent_parent, - src_dent->d_name.len); + user_ns = file_mnt_user_ns(fp->filp); + src_child = lookup_one(user_ns, 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; @@ -823,7 +828,7 @@ int ksmbd_vfs_fp_rename(struct ksmbd_work *work, struct ksmbd_file *fp, dput(src_child); err = __ksmbd_vfs_rename(work, - file_mnt_user_ns(fp->filp), + user_ns, src_dent_parent, src_dent, mnt_user_ns(dst_path.mnt), @@ -1109,7 +1114,7 @@ int ksmbd_vfs_unlink(struct user_namespace *user_ns, { int err = 0; - err = ksmbd_vfs_lock_parent(dir, dentry); + err = ksmbd_vfs_lock_parent(user_ns, dir, dentry); if (err) return err; dget(dentry); diff --git a/fs/ksmbd/vfs.h b/fs/ksmbd/vfs.h index cb0cba0d5d07..85db50abdb24 100644 --- a/fs/ksmbd/vfs.h +++ b/fs/ksmbd/vfs.h @@ -107,7 +107,8 @@ struct ksmbd_kstat { __le32 file_attributes; }; -int ksmbd_vfs_lock_parent(struct dentry *parent, struct dentry *child); +int ksmbd_vfs_lock_parent(struct user_namespace *user_ns, struct dentry *parent, + struct dentry *child); int ksmbd_vfs_may_delete(struct user_namespace *user_ns, struct dentry *dentry); int ksmbd_vfs_query_maximal_access(struct user_namespace *user_ns, struct dentry *dentry, __le32 *daccess); -- cgit v1.2.3 From 475d6f98804c09a48b5c815f7bd466fb4c1e743e Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Mon, 23 Aug 2021 17:13:48 +0200 Subject: ksmbd: fix translation in smb2_populate_readdir_entry() When transferring ownership information to the client the k*ids are translated into raw *ids before they are sent over the wire. The function currently erroneously translates the k*ids according to the mount's idmapping. Instead, reporting the owning *ids to userspace the underlying k*ids need to be mapped up in the caller's user namespace. This is how stat() works. The caller in this instance is ksmbd itself and ksmbd always runs in the initial user namespace. Translate according to that. The idmapping of the mount is already taken into account by the lower filesystem and so kstat->*id will contain the mapped k*ids. Switch to from_k*id_munged() which ensures that the overflow*id is returned instead of the (*id_t)-1 when the k*id can't be translated. Cc: Steve French Cc: Christoph Hellwig Cc: Namjae Jeon Cc: Hyunchul Lee Cc: Sergey Senozhatsky Cc: linux-cifs@vger.kernel.org Signed-off-by: Christian Brauner Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smb2pdu.c | 6 ++---- fs/ksmbd/smb_common.c | 4 +--- fs/ksmbd/smb_common.h | 1 - 3 files changed, 3 insertions(+), 8 deletions(-) (limited to 'fs') diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index a400dd292af1..559bfa2623f2 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -3324,7 +3324,6 @@ static int dentry_name(struct ksmbd_dir_info *d_info, int info_level) */ static int smb2_populate_readdir_entry(struct ksmbd_conn *conn, int info_level, struct ksmbd_dir_info *d_info, - struct user_namespace *user_ns, struct ksmbd_kstat *ksmbd_kstat) { int next_entry_offset = 0; @@ -3478,9 +3477,9 @@ static int smb2_populate_readdir_entry(struct ksmbd_conn *conn, int info_level, S_ISDIR(ksmbd_kstat->kstat->mode) ? ATTR_DIRECTORY_LE : ATTR_ARCHIVE_LE; if (d_info->hide_dot_file && d_info->name[0] == '.') posix_info->DosAttributes |= ATTR_HIDDEN_LE; - id_to_sid(from_kuid(user_ns, ksmbd_kstat->kstat->uid), + id_to_sid(from_kuid_munged(&init_user_ns, ksmbd_kstat->kstat->uid), SIDNFS_USER, (struct smb_sid *)&posix_info->SidBuffer[0]); - id_to_sid(from_kgid(user_ns, ksmbd_kstat->kstat->gid), + id_to_sid(from_kgid_munged(&init_user_ns, ksmbd_kstat->kstat->gid), SIDNFS_GROUP, (struct smb_sid *)&posix_info->SidBuffer[20]); memcpy(posix_info->name, conv_name, conv_len); posix_info->name_len = cpu_to_le32(conv_len); @@ -3571,7 +3570,6 @@ static int process_query_dir_entries(struct smb2_query_dir_private *priv) rc = smb2_populate_readdir_entry(priv->work->conn, priv->info_level, priv->d_info, - user_ns, &ksmbd_kstat); dput(dent); if (rc) diff --git a/fs/ksmbd/smb_common.c b/fs/ksmbd/smb_common.c index b108b918ec84..43d3123d8b62 100644 --- a/fs/ksmbd/smb_common.c +++ b/fs/ksmbd/smb_common.c @@ -291,7 +291,6 @@ int ksmbd_populate_dot_dotdot_entries(struct ksmbd_work *work, int info_level, char *search_pattern, int (*fn)(struct ksmbd_conn *, int, struct ksmbd_dir_info *, - struct user_namespace *, struct ksmbd_kstat *)) { int i, rc = 0; @@ -322,8 +321,7 @@ int ksmbd_populate_dot_dotdot_entries(struct ksmbd_work *work, int info_level, user_ns, dir->filp->f_path.dentry->d_parent, &ksmbd_kstat); - rc = fn(conn, info_level, d_info, - user_ns, &ksmbd_kstat); + rc = fn(conn, info_level, d_info, &ksmbd_kstat); if (rc) break; if (d_info->out_buf_len <= 0) diff --git a/fs/ksmbd/smb_common.h b/fs/ksmbd/smb_common.h index eb667d85558e..57c667c1be06 100644 --- a/fs/ksmbd/smb_common.h +++ b/fs/ksmbd/smb_common.h @@ -511,7 +511,6 @@ int ksmbd_populate_dot_dotdot_entries(struct ksmbd_work *work, int (*fn)(struct ksmbd_conn *, int, struct ksmbd_dir_info *, - struct user_namespace *, struct ksmbd_kstat *)); int ksmbd_extract_shortname(struct ksmbd_conn *conn, -- cgit v1.2.3 From 3cdc20e72c3dacf22382fd15d3154e48d0efdec9 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Mon, 23 Aug 2021 17:13:49 +0200 Subject: ksmbd: fix translation in create_posix_rsp_buf() When transferring ownership information to the client the k*ids are translated into raw *ids before they are sent over the wire. The function currently erroneously translates the k*ids according to the mount's idmapping. Instead, reporting the owning *ids to userspace the underlying k*ids need to be mapped up in the caller's user namespace. This is how stat() works. The caller in this instance is ksmbd itself and ksmbd always runs in the initial user namespace. Translate according to that taking any potential idmapped mounts into account. Switch to from_k*id_munged() which ensures that the overflow*id is returned instead of the (*id_t)-1 when the k*id can't be translated. Cc: Steve French Cc: Christoph Hellwig Cc: Namjae Jeon Cc: Hyunchul Lee Cc: Sergey Senozhatsky Cc: linux-cifs@vger.kernel.org Signed-off-by: Christian Brauner Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/oplock.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'fs') diff --git a/fs/ksmbd/oplock.c b/fs/ksmbd/oplock.c index 6ace6c2f22dc..16b6236d1bd2 100644 --- a/fs/ksmbd/oplock.c +++ b/fs/ksmbd/oplock.c @@ -1614,9 +1614,11 @@ void create_posix_rsp_buf(char *cc, struct ksmbd_file *fp) buf->nlink = cpu_to_le32(inode->i_nlink); buf->reparse_tag = cpu_to_le32(fp->volatile_id); buf->mode = cpu_to_le32(inode->i_mode); - id_to_sid(from_kuid(user_ns, inode->i_uid), + id_to_sid(from_kuid_munged(&init_user_ns, + i_uid_into_mnt(user_ns, inode)), SIDNFS_USER, (struct smb_sid *)&buf->SidBuffer[0]); - id_to_sid(from_kgid(user_ns, inode->i_gid), + id_to_sid(from_kgid_munged(&init_user_ns, + i_gid_into_mnt(user_ns, inode)), SIDNFS_GROUP, (struct smb_sid *)&buf->SidBuffer[20]); } -- cgit v1.2.3 From 43205ca7192aa5de46775fbf7a043222e76abac5 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Mon, 23 Aug 2021 17:13:50 +0200 Subject: ksmbd: fix translation in ksmbd_acls_fattr() When creating new filesystem objects ksmbd translates between k*ids and s*ids. For this it often uses struct smb_fattr and stashes the k*ids in cf_uid and cf_gid. Let cf_uid and cf_gid always contain the final information taking any potential idmapped mounts into account. When finally translation cf_*id into s*ids translate them into the user namespace of ksmbd since that is the relevant user namespace here. Cc: Steve French Cc: Christoph Hellwig Cc: Namjae Jeon Cc: Hyunchul Lee Cc: Sergey Senozhatsky Cc: linux-cifs@vger.kernel.org Signed-off-by: Christian Brauner Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smb2pdu.c | 12 +++++++----- fs/ksmbd/smbacl.c | 8 ++++---- 2 files changed, 11 insertions(+), 9 deletions(-) (limited to 'fs') diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 559bfa2623f2..1b0a9242be88 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -2381,10 +2381,12 @@ static int smb2_create_sd_buffer(struct ksmbd_work *work, le32_to_cpu(sd_buf->ccontext.DataLength), true); } -static void ksmbd_acls_fattr(struct smb_fattr *fattr, struct inode *inode) +static void ksmbd_acls_fattr(struct smb_fattr *fattr, + struct user_namespace *mnt_userns, + struct inode *inode) { - fattr->cf_uid = inode->i_uid; - fattr->cf_gid = inode->i_gid; + fattr->cf_uid = i_uid_into_mnt(mnt_userns, inode); + fattr->cf_gid = i_gid_into_mnt(mnt_userns, inode); fattr->cf_mode = inode->i_mode; fattr->cf_acls = NULL; fattr->cf_dacls = NULL; @@ -2893,7 +2895,7 @@ int smb2_open(struct ksmbd_work *work) struct smb_ntsd *pntsd; int pntsd_size, ace_num = 0; - ksmbd_acls_fattr(&fattr, inode); + ksmbd_acls_fattr(&fattr, user_ns, inode); if (fattr.cf_acls) ace_num = fattr.cf_acls->a_count; if (fattr.cf_dacls) @@ -5006,7 +5008,7 @@ static int smb2_get_info_sec(struct ksmbd_work *work, user_ns = file_mnt_user_ns(fp->filp); inode = file_inode(fp->filp); - ksmbd_acls_fattr(&fattr, inode); + ksmbd_acls_fattr(&fattr, user_ns, inode); if (test_share_config_flag(work->tcon->share_conf, KSMBD_SHARE_FLAG_ACL_XATTR)) diff --git a/fs/ksmbd/smbacl.c b/fs/ksmbd/smbacl.c index 5456e3ad943e..a7025b31d2f2 100644 --- a/fs/ksmbd/smbacl.c +++ b/fs/ksmbd/smbacl.c @@ -723,7 +723,7 @@ static void set_mode_dacl(struct user_namespace *user_ns, } /* owner RID */ - uid = from_kuid(user_ns, fattr->cf_uid); + uid = from_kuid(&init_user_ns, fattr->cf_uid); if (uid) sid = &server_conf.domain_sid; else @@ -739,7 +739,7 @@ static void set_mode_dacl(struct user_namespace *user_ns, ace_size = fill_ace_for_sid(pace, &sid_unix_groups, ACCESS_ALLOWED, 0, fattr->cf_mode, 0070); pace->sid.sub_auth[pace->sid.num_subauth++] = - cpu_to_le32(from_kgid(user_ns, fattr->cf_gid)); + cpu_to_le32(from_kgid(&init_user_ns, fattr->cf_gid)); pace->size = cpu_to_le16(ace_size + 4); size += le16_to_cpu(pace->size); pace = (struct smb_ace *)((char *)pndace + size); @@ -880,7 +880,7 @@ int build_sec_desc(struct user_namespace *user_ns, if (!nowner_sid_ptr) return -ENOMEM; - uid = from_kuid(user_ns, fattr->cf_uid); + uid = from_kuid(&init_user_ns, fattr->cf_uid); if (!uid) sid_type = SIDUNIX_USER; id_to_sid(uid, sid_type, nowner_sid_ptr); @@ -891,7 +891,7 @@ int build_sec_desc(struct user_namespace *user_ns, return -ENOMEM; } - gid = from_kgid(user_ns, fattr->cf_gid); + gid = from_kgid(&init_user_ns, fattr->cf_gid); id_to_sid(gid, SIDUNIX_GROUP, ngroup_sid_ptr); offset = sizeof(struct smb_ntsd); -- cgit v1.2.3 From 0e844efebdf9c03aed9ae1894f22762a8aee1a3b Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Mon, 23 Aug 2021 17:13:51 +0200 Subject: ksmbd: fix translation in acl entries The ksmbd server performs translation of posix acls to smb acls. Currently the translation is wrong since the idmapping of the mount is used to map the ids into raw userspace ids but what is relevant is the user namespace of ksmbd itself. The user namespace of ksmbd itself which is the initial user namespace. The operation is similar to asking "What *ids would a userspace process see given that k*id in the relevant user namespace?". Before the final translation we need to apply the idmapping of the mount in case any is used. Add two simple helpers for ksmbd. Cc: Steve French Cc: Christoph Hellwig Cc: Namjae Jeon Cc: Hyunchul Lee Cc: Sergey Senozhatsky Cc: linux-cifs@vger.kernel.org Signed-off-by: Christian Brauner Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smbacl.c | 14 ++++++-------- fs/ksmbd/smbacl.h | 25 +++++++++++++++++++++++++ fs/ksmbd/vfs.c | 4 ++-- 3 files changed, 33 insertions(+), 10 deletions(-) (limited to 'fs') diff --git a/fs/ksmbd/smbacl.c b/fs/ksmbd/smbacl.c index a7025b31d2f2..3307ca776eb1 100644 --- a/fs/ksmbd/smbacl.c +++ b/fs/ksmbd/smbacl.c @@ -587,14 +587,14 @@ static void set_posix_acl_entries_dacl(struct user_namespace *user_ns, uid_t uid; unsigned int sid_type = SIDOWNER; - uid = from_kuid(user_ns, pace->e_uid); + uid = posix_acl_uid_translate(user_ns, pace); if (!uid) sid_type = SIDUNIX_USER; id_to_sid(uid, sid_type, sid); } else if (pace->e_tag == ACL_GROUP) { gid_t gid; - gid = from_kgid(user_ns, pace->e_gid); + gid = posix_acl_gid_translate(user_ns, pace); id_to_sid(gid, SIDUNIX_GROUP, sid); } else if (pace->e_tag == ACL_OTHER && !nt_aces_num) { smb_copy_sid(sid, &sid_everyone); @@ -653,12 +653,12 @@ posix_default_acl: if (pace->e_tag == ACL_USER) { uid_t uid; - uid = from_kuid(user_ns, pace->e_uid); + uid = posix_acl_uid_translate(user_ns, pace); id_to_sid(uid, SIDCREATOR_OWNER, sid); } else if (pace->e_tag == ACL_GROUP) { gid_t gid; - gid = from_kgid(user_ns, pace->e_gid); + gid = posix_acl_gid_translate(user_ns, pace); id_to_sid(gid, SIDCREATOR_GROUP, sid); } else { kfree(sid); @@ -1234,11 +1234,9 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, struct path *path, pa_entry = posix_acls->a_entries; for (i = 0; i < posix_acls->a_count; i++, pa_entry++) { if (pa_entry->e_tag == ACL_USER) - id = from_kuid(user_ns, - pa_entry->e_uid); + id = posix_acl_uid_translate(user_ns, pa_entry); else if (pa_entry->e_tag == ACL_GROUP) - id = from_kgid(user_ns, - pa_entry->e_gid); + id = posix_acl_gid_translate(user_ns, pa_entry); else continue; diff --git a/fs/ksmbd/smbacl.h b/fs/ksmbd/smbacl.h index 940f686a1d95..73e08cad412b 100644 --- a/fs/ksmbd/smbacl.h +++ b/fs/ksmbd/smbacl.h @@ -209,4 +209,29 @@ int set_info_sec(struct ksmbd_conn *conn, struct ksmbd_tree_connect *tcon, bool type_check); void id_to_sid(unsigned int cid, uint sidtype, struct smb_sid *ssid); void ksmbd_init_domain(u32 *sub_auth); + +static inline uid_t posix_acl_uid_translate(struct user_namespace *mnt_userns, + struct posix_acl_entry *pace) +{ + kuid_t kuid; + + /* If this is an idmapped mount, apply the idmapping. */ + kuid = kuid_into_mnt(mnt_userns, pace->e_uid); + + /* Translate the kuid into a userspace id ksmbd would see. */ + return from_kuid(&init_user_ns, kuid); +} + +static inline gid_t posix_acl_gid_translate(struct user_namespace *mnt_userns, + struct posix_acl_entry *pace) +{ + kgid_t kgid; + + /* If this is an idmapped mount, apply the idmapping. */ + kgid = kgid_into_mnt(mnt_userns, pace->e_gid); + + /* Translate the kgid into a userspace id ksmbd would see. */ + return from_kgid(&init_user_ns, kgid); +} + #endif /* _SMBACL_H */ diff --git a/fs/ksmbd/vfs.c b/fs/ksmbd/vfs.c index 2bb506d1fb32..b047f2980d96 100644 --- a/fs/ksmbd/vfs.c +++ b/fs/ksmbd/vfs.c @@ -1390,14 +1390,14 @@ static struct xattr_smb_acl *ksmbd_vfs_make_xattr_posix_acl(struct user_namespac switch (pa_entry->e_tag) { case ACL_USER: xa_entry->type = SMB_ACL_USER; - xa_entry->uid = from_kuid(user_ns, pa_entry->e_uid); + xa_entry->uid = posix_acl_uid_translate(user_ns, pa_entry); break; case ACL_USER_OBJ: xa_entry->type = SMB_ACL_USER_OBJ; break; case ACL_GROUP: xa_entry->type = SMB_ACL_GROUP; - xa_entry->gid = from_kgid(user_ns, pa_entry->e_gid); + xa_entry->gid = posix_acl_gid_translate(user_ns, pa_entry); break; case ACL_GROUP_OBJ: xa_entry->type = SMB_ACL_GROUP_OBJ; -- cgit v1.2.3 From f0bb29d5c65b492ab82cce7b1e1dd00cbca28601 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Tue, 24 Aug 2021 20:07:18 +0900 Subject: ksmbd: fix subauth 0 handling in sid_to_id() It's not obvious why subauth 0 would be excluded from translation. This would lead to wrong results whenever a non-identity idmapping is used. Cc: Steve French Cc: Christoph Hellwig Cc: Namjae Jeon Cc: Hyunchul Lee Cc: Sergey Senozhatsky Cc: linux-cifs@vger.kernel.org Signed-off-by: Christian Brauner Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smbacl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fs') diff --git a/fs/ksmbd/smbacl.c b/fs/ksmbd/smbacl.c index 3307ca776eb1..e49e9bcdec80 100644 --- a/fs/ksmbd/smbacl.c +++ b/fs/ksmbd/smbacl.c @@ -274,7 +274,7 @@ static int sid_to_id(struct user_namespace *user_ns, uid_t id; id = le32_to_cpu(psid->sub_auth[psid->num_subauth - 1]); - if (id > 0) { + if (id >= 0) { uid = make_kuid(user_ns, id); if (uid_valid(uid) && kuid_has_mapping(user_ns, uid)) { fattr->cf_uid = uid; @@ -286,7 +286,7 @@ static int sid_to_id(struct user_namespace *user_ns, gid_t id; id = le32_to_cpu(psid->sub_auth[psid->num_subauth - 1]); - if (id > 0) { + if (id >= 0) { gid = make_kgid(user_ns, id); if (gid_valid(gid) && kgid_has_mapping(user_ns, gid)) { fattr->cf_gid = gid; -- cgit v1.2.3 From 55cd04d75e635ac915bf54586ec64057249508ec Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Tue, 24 Aug 2021 20:10:20 +0900 Subject: ksmbd: fix translation in sid_to_id() The sid_to_id() functions is relevant when changing ownership of filesystem objects based on acl information. In this case we need to first translate the relevant s*ids into k*ids in ksmbd's user namespace and account for any idmapped mounts. Requesting a change in ownership requires the inverse translation to be applied when we would report ownership to userspace. So k*id_from_mnt() must be used here. Cc: Steve French Cc: Christoph Hellwig Cc: Namjae Jeon Cc: Hyunchul Lee Cc: Sergey Senozhatsky Cc: linux-cifs@vger.kernel.org Signed-off-by: Christian Brauner Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smbacl.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) (limited to 'fs') diff --git a/fs/ksmbd/smbacl.c b/fs/ksmbd/smbacl.c index e49e9bcdec80..ef5896297607 100644 --- a/fs/ksmbd/smbacl.c +++ b/fs/ksmbd/smbacl.c @@ -275,8 +275,15 @@ static int sid_to_id(struct user_namespace *user_ns, id = le32_to_cpu(psid->sub_auth[psid->num_subauth - 1]); if (id >= 0) { - uid = make_kuid(user_ns, id); - if (uid_valid(uid) && kuid_has_mapping(user_ns, uid)) { + /* + * Translate raw sid into kuid in the server's user + * namespace. + */ + uid = make_kuid(&init_user_ns, id); + + /* If this is an idmapped mount, apply the idmapping. */ + uid = kuid_from_mnt(user_ns, uid); + if (uid_valid(uid)) { fattr->cf_uid = uid; rc = 0; } @@ -287,8 +294,15 @@ static int sid_to_id(struct user_namespace *user_ns, id = le32_to_cpu(psid->sub_auth[psid->num_subauth - 1]); if (id >= 0) { - gid = make_kgid(user_ns, id); - if (gid_valid(gid) && kgid_has_mapping(user_ns, gid)) { + /* + * Translate raw sid into kgid in the server's user + * namespace. + */ + gid = make_kgid(&init_user_ns, id); + + /* If this is an idmapped mount, apply the idmapping. */ + gid = kgid_from_mnt(user_ns, gid); + if (gid_valid(gid)) { fattr->cf_gid = gid; rc = 0; } -- cgit v1.2.3 From 9467a0ce486c87a8f06ad492afa8b95686bff61f Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Mon, 23 Aug 2021 17:13:54 +0200 Subject: ndr: fix translation in ndr_encode_posix_acl() The sid_to_id() helper encodes raw ownership information suitable for s*id handling. This is conceptually equivalent to reporting ownership information via stat to userspace. In this case the consumer is ksmbd instead of a regular user. So when encoding raw ownership information suitable for s*id handling later we need to map the id up according to the user namespace of ksmbd itself taking any idmapped mounts into account. Cc: Steve French Cc: Christoph Hellwig Cc: Namjae Jeon Cc: Hyunchul Lee Cc: Sergey Senozhatsky Cc: linux-cifs@vger.kernel.org Signed-off-by: Christian Brauner Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/ndr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fs') diff --git a/fs/ksmbd/ndr.c b/fs/ksmbd/ndr.c index 2243a2c64b37..e6a574fa7709 100644 --- a/fs/ksmbd/ndr.c +++ b/fs/ksmbd/ndr.c @@ -259,8 +259,8 @@ int ndr_encode_posix_acl(struct ndr *n, ndr_write_int32(n, 0); } - ndr_write_int64(n, from_kuid(user_ns, inode->i_uid)); - ndr_write_int64(n, from_kgid(user_ns, inode->i_gid)); + ndr_write_int64(n, from_kuid(&init_user_ns, i_uid_into_mnt(user_ns, inode))); + ndr_write_int64(n, from_kgid(&init_user_ns, i_gid_into_mnt(user_ns, inode))); ndr_write_int32(n, inode->i_mode); if (acl) { -- cgit v1.2.3 From eb5784f0c6efbe0db720ad7e34e097cea51c1afc Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Mon, 23 Aug 2021 17:13:55 +0200 Subject: ksmbd: ensure error is surfaced in set_file_basic_info() It seems the error was accidently ignored until now. Make sure it is surfaced. Cc: Steve French Cc: Christoph Hellwig Cc: Namjae Jeon Cc: Hyunchul Lee Cc: Sergey Senozhatsky Cc: linux-cifs@vger.kernel.org Signed-off-by: Christian Brauner Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smb2pdu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 1b0a9242be88..1148e52a4037 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -5531,7 +5531,7 @@ static int set_file_basic_info(struct ksmbd_file *fp, char *buf, rc = notify_change(user_ns, dentry, &attrs, NULL); inode_unlock(inode); } - return 0; + return rc; } static int set_file_allocation_info(struct ksmbd_work *work, -- cgit v1.2.3 From db7fb6fe3d7a8eb05f2b74c6252771c9362f3b74 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Thu, 26 Aug 2021 10:07:05 +0900 Subject: ksmbd: remove setattr preparations in set_file_basic_info() Permission checking and copying over ownership information is the task of the underlying filesystem not ksmbd. The order is also wrong here. This modifies the inode before notify_change(). If notify_change() fails this will have changed ownership nonetheless. All of this is unnecessary though since the underlying filesystem's ->setattr handler will do all this (if required) by itself. Cc: Steve French Cc: Christoph Hellwig Cc: Namjae Jeon Cc: Hyunchul Lee Cc: Sergey Senozhatsky Cc: linux-cifs@vger.kernel.org Signed-off-by: Christian Brauner Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smb2pdu.c | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) (limited to 'fs') diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 1148e52a4037..e2224b91d4a5 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -5440,7 +5440,7 @@ static int set_file_basic_info(struct ksmbd_file *fp, char *buf, { struct smb2_file_all_info *file_info; struct iattr attrs; - struct iattr temp_attrs; + struct timespec64 ctime; struct file *filp; struct inode *inode; struct user_namespace *user_ns; @@ -5464,11 +5464,11 @@ static int set_file_basic_info(struct ksmbd_file *fp, char *buf, } if (file_info->ChangeTime) { - temp_attrs.ia_ctime = ksmbd_NTtimeToUnix(file_info->ChangeTime); - attrs.ia_ctime = temp_attrs.ia_ctime; + attrs.ia_ctime = ksmbd_NTtimeToUnix(file_info->ChangeTime); + ctime = attrs.ia_ctime; attrs.ia_valid |= ATTR_CTIME; } else { - temp_attrs.ia_ctime = inode->i_ctime; + ctime = inode->i_ctime; } if (file_info->LastWriteTime) { @@ -5507,13 +5507,6 @@ static int set_file_basic_info(struct ksmbd_file *fp, char *buf, rc = 0; } - /* - * HACK : set ctime here to avoid ctime changed - * when file_info->ChangeTime is zero. - */ - attrs.ia_ctime = temp_attrs.ia_ctime; - attrs.ia_valid |= ATTR_CTIME; - if (attrs.ia_valid) { struct dentry *dentry = filp->f_path.dentry; struct inode *inode = d_inode(dentry); @@ -5521,14 +5514,12 @@ static int set_file_basic_info(struct ksmbd_file *fp, char *buf, if (IS_IMMUTABLE(inode) || IS_APPEND(inode)) return -EACCES; - rc = setattr_prepare(user_ns, dentry, &attrs); - if (rc) - return -EINVAL; - inode_lock(inode); - setattr_copy(user_ns, inode, &attrs); - attrs.ia_valid &= ~ATTR_CTIME; rc = notify_change(user_ns, dentry, &attrs, NULL); + if (!rc) { + inode->i_ctime = ctime; + mark_inode_dirty(inode); + } inode_unlock(inode); } return rc; -- cgit v1.2.3 From 28a5d3de9d65058f7edf8e5aaaf6b0a8d8f4a29f Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Thu, 26 Aug 2021 17:05:17 +0900 Subject: ksmbd: defer notify_change() call When ownership is changed we might in certain scenarios loose the ability to alter the inode after we changed ownership. This can e.g. happen when we are on an idmapped mount where uid 0 is mapped to uid 1000 and uid 1000 is mapped to uid 0. A caller with fs*id 1000 will be able to create files as *id 1000 on disk. They will also be able to change ownership of files owned by *id 0 to *id 1000 but they won't be able to change ownership in the other direction. This means acl operations following notify_change() would fail. Move the notify_change() call after the acls have been updated. This guarantees that we don't end up with spurious "hash value diff" warnings later on because we managed to change ownership but didn't manage to alter acls. Cc: Steve French Cc: Christoph Hellwig Cc: Namjae Jeon Cc: Hyunchul Lee Cc: Sergey Senozhatsky Cc: linux-cifs@vger.kernel.org Signed-off-by: Christian Brauner Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smbacl.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) (limited to 'fs') diff --git a/fs/ksmbd/smbacl.c b/fs/ksmbd/smbacl.c index ef5896297607..16da99a9963c 100644 --- a/fs/ksmbd/smbacl.c +++ b/fs/ksmbd/smbacl.c @@ -1334,22 +1334,31 @@ int set_info_sec(struct ksmbd_conn *conn, struct ksmbd_tree_connect *tcon, newattrs.ia_valid |= ATTR_MODE; newattrs.ia_mode = (inode->i_mode & ~0777) | (fattr.cf_mode & 0777); - inode_lock(inode); - rc = notify_change(user_ns, path->dentry, &newattrs, NULL); - inode_unlock(inode); - if (rc) - goto out; - ksmbd_vfs_remove_acl_xattrs(user_ns, path->dentry); /* Update posix acls */ if (IS_ENABLED(CONFIG_FS_POSIX_ACL) && fattr.cf_dacls) { rc = set_posix_acl(user_ns, inode, ACL_TYPE_ACCESS, fattr.cf_acls); - if (S_ISDIR(inode->i_mode) && fattr.cf_dacls) + if (rc < 0) + ksmbd_debug(SMB, + "Set posix acl(ACL_TYPE_ACCESS) failed, rc : %d\n", + rc); + if (S_ISDIR(inode->i_mode) && fattr.cf_dacls) { rc = set_posix_acl(user_ns, inode, ACL_TYPE_DEFAULT, fattr.cf_dacls); + if (rc) + ksmbd_debug(SMB, + "Set posix acl(ACL_TYPE_DEFAULT) failed, rc : %d\n", + rc); + } } + inode_lock(inode); + rc = notify_change(user_ns, path->dentry, &newattrs, NULL); + inode_unlock(inode); + if (rc) + goto out; + /* Check it only calling from SD BUFFER context */ if (type_check && !(le16_to_cpu(pntsd->type) & DACL_PRESENT)) goto out; -- cgit v1.2.3 From d475866eeed89cc44ed54e0cd296537a68667b1b Mon Sep 17 00:00:00 2001 From: Per Forlin Date: Mon, 30 Aug 2021 13:23:04 +0900 Subject: ksmbd: Reduce error log 'speed is unknown' to debug This log happens on servers with a network bridge since the bridge does not have a specified link speed. This is not a real error so change the error log to debug instead. Signed-off-by: Per Forlin Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smb2pdu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fs') diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index e2224b91d4a5..a350e1cef7f4 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -7111,8 +7111,8 @@ static int fsctl_query_iface_info_ioctl(struct ksmbd_conn *conn, netdev->ethtool_ops->get_link_ksettings(netdev, &cmd); speed = cmd.base.speed; } else { - pr_err("%s %s\n", netdev->name, - "speed is unknown, defaulting to 1Gb/sec"); + ksmbd_debug(SMB, "%s %s\n", netdev->name, + "speed is unknown, defaulting to 1Gb/sec"); speed = SPEED_1000; } -- cgit v1.2.3 From 72d6cbb533d4309734606027fe083c4edb0aa7aa Mon Sep 17 00:00:00 2001 From: Hyunchul Lee Date: Mon, 30 Aug 2021 13:27:43 +0900 Subject: ksmbd: smbd: fix dma mapping error in smb_direct_post_send_data Becase smb direct header is mapped and msg->num_sge already is incremented, the decrement should be removed from the condition. Signed-off-by: Hyunchul Lee Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/transport_rdma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/ksmbd/transport_rdma.c b/fs/ksmbd/transport_rdma.c index 58f530056ac0..52b2556e76b1 100644 --- a/fs/ksmbd/transport_rdma.c +++ b/fs/ksmbd/transport_rdma.c @@ -1168,7 +1168,7 @@ static int smb_direct_post_send_data(struct smb_direct_transport *t, pr_err("failed to map buffer\n"); ret = -ENOMEM; goto err; - } else if (sg_cnt + msg->num_sge > SMB_DIRECT_MAX_SEND_SGES - 1) { + } else if (sg_cnt + msg->num_sge > SMB_DIRECT_MAX_SEND_SGES) { pr_err("buffer not fitted into sges\n"); ret = -E2BIG; ib_dma_unmap_sg(t->cm_id->device, sg, sg_cnt, -- cgit v1.2.3 From 687c59e702f48e0eca91455d3ef3197b7b8a8314 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Tue, 31 Aug 2021 22:47:02 +0900 Subject: ksmbd: remove unused ksmbd_file_table_flush function ksmbd_file_table_flush is a leftover from SMB1. This function is no longer needed as SMB1 has been removed from ksmbd. Reported-by: Dan Carpenter Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/vfs_cache.c | 16 ---------------- fs/ksmbd/vfs_cache.h | 1 - 2 files changed, 17 deletions(-) (limited to 'fs') diff --git a/fs/ksmbd/vfs_cache.c b/fs/ksmbd/vfs_cache.c index 92d8c61ffd2a..29c1db66bd0f 100644 --- a/fs/ksmbd/vfs_cache.c +++ b/fs/ksmbd/vfs_cache.c @@ -666,22 +666,6 @@ void ksmbd_free_global_file_table(void) ksmbd_destroy_file_table(&global_ft); } -int ksmbd_file_table_flush(struct ksmbd_work *work) -{ - struct ksmbd_file *fp = NULL; - unsigned int id; - int ret; - - read_lock(&work->sess->file_table.lock); - idr_for_each_entry(work->sess->file_table.idr, fp, id) { - ret = ksmbd_vfs_fsync(work, fp->volatile_id, KSMBD_NO_FID); - if (ret) - break; - } - read_unlock(&work->sess->file_table.lock); - return ret; -} - int ksmbd_init_file_table(struct ksmbd_file_table *ft) { ft->idr = kzalloc(sizeof(struct idr), GFP_KERNEL); diff --git a/fs/ksmbd/vfs_cache.h b/fs/ksmbd/vfs_cache.h index 70dfe6a99f13..448576fbe4b7 100644 --- a/fs/ksmbd/vfs_cache.h +++ b/fs/ksmbd/vfs_cache.h @@ -152,7 +152,6 @@ void ksmbd_close_session_fds(struct ksmbd_work *work); int ksmbd_close_inode_fds(struct ksmbd_work *work, struct inode *inode); int ksmbd_init_global_file_table(void); void ksmbd_free_global_file_table(void); -int ksmbd_file_table_flush(struct ksmbd_work *work); void ksmbd_set_fd_limit(unsigned long limit); /* -- cgit v1.2.3 From 303fff2b8c77a85c62dbde3b27c24b084144c04c Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Fri, 3 Sep 2021 07:09:44 +0900 Subject: ksmbd: add validation for ndr read/write functions If ndr->length is smaller than expected size, ksmbd can access invalid access in ndr->data. This patch add validation to check ndr->offset is over ndr->length. and added exception handling to check return value of ndr read/write function. Cc: Dan Carpenter Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/ndr.c | 383 +++++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 274 insertions(+), 109 deletions(-) (limited to 'fs') diff --git a/fs/ksmbd/ndr.c b/fs/ksmbd/ndr.c index e6a574fa7709..a476de291f62 100644 --- a/fs/ksmbd/ndr.c +++ b/fs/ksmbd/ndr.c @@ -28,37 +28,60 @@ static int try_to_realloc_ndr_blob(struct ndr *n, size_t sz) return 0; } -static void ndr_write_int16(struct ndr *n, __u16 value) +static int ndr_write_int16(struct ndr *n, __u16 value) { - if (n->length <= n->offset + sizeof(value)) - try_to_realloc_ndr_blob(n, sizeof(value)); + if (n->length <= n->offset + sizeof(value)) { + int ret; + + ret = try_to_realloc_ndr_blob(n, sizeof(value)); + if (ret) + return ret; + } *(__le16 *)ndr_get_field(n) = cpu_to_le16(value); n->offset += sizeof(value); + return 0; } -static void ndr_write_int32(struct ndr *n, __u32 value) +static int ndr_write_int32(struct ndr *n, __u32 value) { - if (n->length <= n->offset + sizeof(value)) - try_to_realloc_ndr_blob(n, sizeof(value)); + if (n->length <= n->offset + sizeof(value)) { + int ret; + + ret = try_to_realloc_ndr_blob(n, sizeof(value)); + if (ret) + return ret; + } *(__le32 *)ndr_get_field(n) = cpu_to_le32(value); n->offset += sizeof(value); + return 0; } -static void ndr_write_int64(struct ndr *n, __u64 value) +static int ndr_write_int64(struct ndr *n, __u64 value) { - if (n->length <= n->offset + sizeof(value)) - try_to_realloc_ndr_blob(n, sizeof(value)); + if (n->length <= n->offset + sizeof(value)) { + int ret; + + ret = try_to_realloc_ndr_blob(n, sizeof(value)); + if (ret) + return ret; + } *(__le64 *)ndr_get_field(n) = cpu_to_le64(value); n->offset += sizeof(value); + return 0; } static int ndr_write_bytes(struct ndr *n, void *value, size_t sz) { - if (n->length <= n->offset + sz) - try_to_realloc_ndr_blob(n, sz); + if (n->length <= n->offset + sz) { + int ret; + + ret = try_to_realloc_ndr_blob(n, sz); + if (ret) + return ret; + } memcpy(ndr_get_field(n), value, sz); n->offset += sz; @@ -70,8 +93,13 @@ static int ndr_write_string(struct ndr *n, char *value) size_t sz; sz = strlen(value) + 1; - if (n->length <= n->offset + sz) - try_to_realloc_ndr_blob(n, sz); + if (n->length <= n->offset + sz) { + int ret; + + ret = try_to_realloc_ndr_blob(n, sz); + if (ret) + return ret; + } memcpy(ndr_get_field(n), value, sz); n->offset += sz; @@ -81,9 +109,14 @@ static int ndr_write_string(struct ndr *n, char *value) static int ndr_read_string(struct ndr *n, void *value, size_t sz) { - int len = strnlen(ndr_get_field(n), sz); + int len; - memcpy(value, ndr_get_field(n), len); + if (n->offset + sz > n->length) + return -EINVAL; + + len = strnlen(ndr_get_field(n), sz); + if (value) + memcpy(value, ndr_get_field(n), len); len++; n->offset += len; n->offset = ALIGN(n->offset, 2); @@ -92,41 +125,52 @@ static int ndr_read_string(struct ndr *n, void *value, size_t sz) static int ndr_read_bytes(struct ndr *n, void *value, size_t sz) { - memcpy(value, ndr_get_field(n), sz); + if (n->offset + sz > n->length) + return -EINVAL; + + if (value) + memcpy(value, ndr_get_field(n), sz); n->offset += sz; return 0; } -static __u16 ndr_read_int16(struct ndr *n) +static int ndr_read_int16(struct ndr *n, __u16 *value) { - __u16 ret; + if (n->offset + sizeof(__u16) > n->length) + return -EINVAL; - ret = le16_to_cpu(*(__le16 *)ndr_get_field(n)); + if (value) + *value = le16_to_cpu(*(__le16 *)ndr_get_field(n)); n->offset += sizeof(__u16); - return ret; + return 0; } -static __u32 ndr_read_int32(struct ndr *n) +static int ndr_read_int32(struct ndr *n, __u32 *value) { - __u32 ret; + if (n->offset + sizeof(__u32) > n->length) + return 0; - ret = le32_to_cpu(*(__le32 *)ndr_get_field(n)); + if (value) + *value = le32_to_cpu(*(__le32 *)ndr_get_field(n)); n->offset += sizeof(__u32); - return ret; + return 0; } -static __u64 ndr_read_int64(struct ndr *n) +static int ndr_read_int64(struct ndr *n, __u64 *value) { - __u64 ret; + if (n->offset + sizeof(__u64) > n->length) + return -EINVAL; - ret = le64_to_cpu(*(__le64 *)ndr_get_field(n)); + if (value) + *value = le64_to_cpu(*(__le64 *)ndr_get_field(n)); n->offset += sizeof(__u64); - return ret; + return 0; } int ndr_encode_dos_attr(struct ndr *n, struct xattr_dos_attrib *da) { char hex_attr[12] = {0}; + int ret; n->offset = 0; n->length = 1024; @@ -136,97 +180,161 @@ int ndr_encode_dos_attr(struct ndr *n, struct xattr_dos_attrib *da) if (da->version == 3) { snprintf(hex_attr, 10, "0x%x", da->attr); - ndr_write_string(n, hex_attr); + ret = ndr_write_string(n, hex_attr); } else { - ndr_write_string(n, ""); + ret = ndr_write_string(n, ""); } - ndr_write_int16(n, da->version); - ndr_write_int32(n, da->version); + if (ret) + return ret; + + ret = ndr_write_int16(n, da->version); + if (ret) + return ret; + + ret = ndr_write_int32(n, da->version); + if (ret) + return ret; + + ret = ndr_write_int32(n, da->flags); + if (ret) + return ret; + + ret = ndr_write_int32(n, da->attr); + if (ret) + return ret; - ndr_write_int32(n, da->flags); - ndr_write_int32(n, da->attr); if (da->version == 3) { - ndr_write_int32(n, da->ea_size); - ndr_write_int64(n, da->size); - ndr_write_int64(n, da->alloc_size); + ret = ndr_write_int32(n, da->ea_size); + if (ret) + return ret; + ret = ndr_write_int64(n, da->size); + if (ret) + return ret; + ret = ndr_write_int64(n, da->alloc_size); } else { - ndr_write_int64(n, da->itime); + ret = ndr_write_int64(n, da->itime); } - ndr_write_int64(n, da->create_time); + if (ret) + return ret; + + ret = ndr_write_int64(n, da->create_time); + if (ret) + return ret; + if (da->version == 3) - ndr_write_int64(n, da->change_time); - return 0; + ret = ndr_write_int64(n, da->change_time); + return ret; } int ndr_decode_dos_attr(struct ndr *n, struct xattr_dos_attrib *da) { - char *hex_attr; - int version2; - - hex_attr = kzalloc(n->length, GFP_KERNEL); - if (!hex_attr) - return -ENOMEM; + char hex_attr[12]; + unsigned int version2; + int ret; n->offset = 0; - ndr_read_string(n, hex_attr, n->length); - kfree(hex_attr); - da->version = ndr_read_int16(n); + ret = ndr_read_string(n, hex_attr, sizeof(hex_attr)); + if (ret) + return ret; + + ret = ndr_read_int16(n, &da->version); + if (ret) + return ret; if (da->version != 3 && da->version != 4) { pr_err("v%d version is not supported\n", da->version); return -EINVAL; } - version2 = ndr_read_int32(n); + ret = ndr_read_int32(n, &version2); + if (ret) + return ret; + if (da->version != version2) { pr_err("ndr version mismatched(version: %d, version2: %d)\n", da->version, version2); return -EINVAL; } - ndr_read_int32(n); - da->attr = ndr_read_int32(n); + ret = ndr_read_int32(n, NULL); + if (ret) + return ret; + + ret = ndr_read_int32(n, &da->attr); + if (ret) + return ret; + if (da->version == 4) { - da->itime = ndr_read_int64(n); - da->create_time = ndr_read_int64(n); + ret = ndr_read_int64(n, &da->itime); + if (ret) + return ret; + + ret = ndr_read_int64(n, &da->create_time); } else { - ndr_read_int32(n); - ndr_read_int64(n); - ndr_read_int64(n); - da->create_time = ndr_read_int64(n); - ndr_read_int64(n); + ret = ndr_read_int32(n, NULL); + if (ret) + return ret; + + ndr_read_int64(n, NULL); + if (ret) + return ret; + + ndr_read_int64(n, NULL); + if (ret) + return ret; + + ret = ndr_read_int64(n, &da->create_time); + if (ret) + return ret; + + ret = ndr_read_int64(n, NULL); } - return 0; + return ret; } static int ndr_encode_posix_acl_entry(struct ndr *n, struct xattr_smb_acl *acl) { - int i; + int i, ret; + + ret = ndr_write_int32(n, acl->count); + if (ret) + return ret; - ndr_write_int32(n, acl->count); n->offset = ALIGN(n->offset, 8); - ndr_write_int32(n, acl->count); - ndr_write_int32(n, 0); + ret = ndr_write_int32(n, acl->count); + if (ret) + return ret; + + ret = ndr_write_int32(n, 0); + if (ret) + return ret; for (i = 0; i < acl->count; i++) { n->offset = ALIGN(n->offset, 8); - ndr_write_int16(n, acl->entries[i].type); - ndr_write_int16(n, acl->entries[i].type); + ret = ndr_write_int16(n, acl->entries[i].type); + if (ret) + return ret; + + ret = ndr_write_int16(n, acl->entries[i].type); + if (ret) + return ret; if (acl->entries[i].type == SMB_ACL_USER) { n->offset = ALIGN(n->offset, 8); - ndr_write_int64(n, acl->entries[i].uid); + ret = ndr_write_int64(n, acl->entries[i].uid); } else if (acl->entries[i].type == SMB_ACL_GROUP) { n->offset = ALIGN(n->offset, 8); - ndr_write_int64(n, acl->entries[i].gid); + ret = ndr_write_int64(n, acl->entries[i].gid); } + if (ret) + return ret; /* push permission */ - ndr_write_int32(n, acl->entries[i].perm); + ret = ndr_write_int32(n, acl->entries[i].perm); } - return 0; + return ret; } int ndr_encode_posix_acl(struct ndr *n, @@ -235,7 +343,8 @@ int ndr_encode_posix_acl(struct ndr *n, struct xattr_smb_acl *acl, struct xattr_smb_acl *def_acl) { - int ref_id = 0x00020000; + unsigned int ref_id = 0x00020000; + int ret; n->offset = 0; n->length = 1024; @@ -245,35 +354,46 @@ int ndr_encode_posix_acl(struct ndr *n, if (acl) { /* ACL ACCESS */ - ndr_write_int32(n, ref_id); + ret = ndr_write_int32(n, ref_id); ref_id += 4; } else { - ndr_write_int32(n, 0); + ret = ndr_write_int32(n, 0); } + if (ret) + return ret; if (def_acl) { /* DEFAULT ACL ACCESS */ - ndr_write_int32(n, ref_id); + ret = ndr_write_int32(n, ref_id); ref_id += 4; } else { - ndr_write_int32(n, 0); + ret = ndr_write_int32(n, 0); } - - ndr_write_int64(n, from_kuid(&init_user_ns, i_uid_into_mnt(user_ns, inode))); - ndr_write_int64(n, from_kgid(&init_user_ns, i_gid_into_mnt(user_ns, inode))); - ndr_write_int32(n, inode->i_mode); + if (ret) + return ret; + + ret = ndr_write_int64(n, from_kuid(&init_user_ns, i_uid_into_mnt(user_ns, inode))); + if (ret) + return ret; + ret = ndr_write_int64(n, from_kgid(&init_user_ns, i_gid_into_mnt(user_ns, inode))); + if (ret) + return ret; + ret = ndr_write_int32(n, inode->i_mode); + if (ret) + return ret; if (acl) { - ndr_encode_posix_acl_entry(n, acl); - if (def_acl) - ndr_encode_posix_acl_entry(n, def_acl); + ret = ndr_encode_posix_acl_entry(n, acl); + if (def_acl && !ret) + ret = ndr_encode_posix_acl_entry(n, def_acl); } - return 0; + return ret; } int ndr_encode_v4_ntacl(struct ndr *n, struct xattr_ntacl *acl) { - int ref_id = 0x00020004; + unsigned int ref_id = 0x00020004; + int ret; n->offset = 0; n->length = 2048; @@ -281,36 +401,65 @@ int ndr_encode_v4_ntacl(struct ndr *n, struct xattr_ntacl *acl) if (!n->data) return -ENOMEM; - ndr_write_int16(n, acl->version); - ndr_write_int32(n, acl->version); - ndr_write_int16(n, 2); - ndr_write_int32(n, ref_id); + ret = ndr_write_int16(n, acl->version); + if (ret) + return ret; + + ret = ndr_write_int32(n, acl->version); + if (ret) + return ret; + + ret = ndr_write_int16(n, 2); + if (ret) + return ret; + + ret = ndr_write_int32(n, ref_id); + if (ret) + return ret; /* push hash type and hash 64bytes */ - ndr_write_int16(n, acl->hash_type); - ndr_write_bytes(n, acl->hash, XATTR_SD_HASH_SIZE); - ndr_write_bytes(n, acl->desc, acl->desc_len); - ndr_write_int64(n, acl->current_time); - ndr_write_bytes(n, acl->posix_acl_hash, XATTR_SD_HASH_SIZE); + ret = ndr_write_int16(n, acl->hash_type); + if (ret) + return ret; - /* push ndr for security descriptor */ - ndr_write_bytes(n, acl->sd_buf, acl->sd_size); + ret = ndr_write_bytes(n, acl->hash, XATTR_SD_HASH_SIZE); + if (ret) + return ret; - return 0; + ret = ndr_write_bytes(n, acl->desc, acl->desc_len); + if (ret) + return ret; + + ret = ndr_write_int64(n, acl->current_time); + if (ret) + return ret; + + ret = ndr_write_bytes(n, acl->posix_acl_hash, XATTR_SD_HASH_SIZE); + if (ret) + return ret; + + /* push ndr for security descriptor */ + ret = ndr_write_bytes(n, acl->sd_buf, acl->sd_size); + return ret; } int ndr_decode_v4_ntacl(struct ndr *n, struct xattr_ntacl *acl) { - int version2; + unsigned int version2; + int ret; n->offset = 0; - acl->version = ndr_read_int16(n); + ret = ndr_read_int16(n, &acl->version); + if (ret) + return ret; if (acl->version != 4) { pr_err("v%d version is not supported\n", acl->version); return -EINVAL; } - version2 = ndr_read_int32(n); + ret = ndr_read_int32(n, &version2); + if (ret) + return ret; if (acl->version != version2) { pr_err("ndr version mismatched(version: %d, version2: %d)\n", acl->version, version2); @@ -318,11 +467,22 @@ int ndr_decode_v4_ntacl(struct ndr *n, struct xattr_ntacl *acl) } /* Read Level */ - ndr_read_int16(n); + ret = ndr_read_int16(n, NULL); + if (ret) + return ret; + /* Read Ref Id */ - ndr_read_int32(n); - acl->hash_type = ndr_read_int16(n); - ndr_read_bytes(n, acl->hash, XATTR_SD_HASH_SIZE); + ret = ndr_read_int32(n, NULL); + if (ret) + return ret; + + ret = ndr_read_int16(n, &acl->hash_type); + if (ret) + return ret; + + ret = ndr_read_bytes(n, acl->hash, XATTR_SD_HASH_SIZE); + if (ret) + return ret; ndr_read_bytes(n, acl->desc, 10); if (strncmp(acl->desc, "posix_acl", 9)) { @@ -331,15 +491,20 @@ int ndr_decode_v4_ntacl(struct ndr *n, struct xattr_ntacl *acl) } /* Read Time */ - ndr_read_int64(n); + ret = ndr_read_int64(n, NULL); + if (ret) + return ret; + /* Read Posix ACL hash */ - ndr_read_bytes(n, acl->posix_acl_hash, XATTR_SD_HASH_SIZE); + ret = ndr_read_bytes(n, acl->posix_acl_hash, XATTR_SD_HASH_SIZE); + if (ret) + return ret; + acl->sd_size = n->length - n->offset; acl->sd_buf = kzalloc(acl->sd_size, GFP_KERNEL); if (!acl->sd_buf) return -ENOMEM; - ndr_read_bytes(n, acl->sd_buf, acl->sd_size); - - return 0; + ret = ndr_read_bytes(n, acl->sd_buf, acl->sd_size); + return ret; } -- cgit v1.2.3 From 36bbeb336584c3551bac72592bcaeeebb430fef1 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Mon, 6 Sep 2021 14:44:38 +0100 Subject: ksmbd: add missing assignments to ret on ndr_read_int64 read calls Currently there are two ndr_read_int64 calls where ret is being checked for failure but ret is not being assigned a return value from the call. Static analyis is reporting the checks on ret as dead code. Fix this. Addresses-Coverity: ("Logical dead code") Reviewed-by: Sergey Senozhatsky Signed-off-by: Colin Ian King Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/ndr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fs') diff --git a/fs/ksmbd/ndr.c b/fs/ksmbd/ndr.c index a476de291f62..8317f7ca402b 100644 --- a/fs/ksmbd/ndr.c +++ b/fs/ksmbd/ndr.c @@ -275,11 +275,11 @@ int ndr_decode_dos_attr(struct ndr *n, struct xattr_dos_attrib *da) if (ret) return ret; - ndr_read_int64(n, NULL); + ret = ndr_read_int64(n, NULL); if (ret) return ret; - ndr_read_int64(n, NULL); + ret = ndr_read_int64(n, NULL); if (ret) return ret; -- cgit v1.2.3 From 4ffd5264e8ecb20e1826b9474c19738fdecd67e6 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Tue, 7 Sep 2021 08:15:21 +0900 Subject: ksmbd: fix read of uninitialized variable ret in set_file_basic_info Addresses-Coverity reported Uninitialized variables warninig : /fs/ksmbd/smb2pdu.c: 5525 in set_file_basic_info() 5519 if (!rc) { 5520 inode->i_ctime = ctime; 5521 mark_inode_dirty(inode); 5522 } 5523 inode_unlock(inode); 5524 } >>> CID 1506805: Uninitialized variables (UNINIT) >>> Using uninitialized value "rc". 5525 return rc; 5526 } 5527 5528 static int set_file_allocation_info(struct ksmbd_work *work, 5529 struct ksmbd_file *fp, char *buf) 5530 { Addresses-Coverity: ("Uninitialized variable") Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smb2pdu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index a350e1cef7f4..c86164dc70bb 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -5444,7 +5444,7 @@ static int set_file_basic_info(struct ksmbd_file *fp, char *buf, struct file *filp; struct inode *inode; struct user_namespace *user_ns; - int rc; + int rc = 0; if (!(fp->daccess & FILE_WRITE_ATTRIBUTES_LE)) return -EACCES; -- cgit v1.2.3 From 4cf0ccd033d9cedef870eb8598a55851e680a173 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Tue, 7 Sep 2021 08:16:26 +0900 Subject: ksmbd: fix control flow issues in sid_to_id() Addresses-Coverity reported Control flow issues in sid_to_id() /fs/ksmbd/smbacl.c: 277 in sid_to_id() 271 272 if (sidtype == SIDOWNER) { 273 kuid_t uid; 274 uid_t id; 275 276 id = le32_to_cpu(psid->sub_auth[psid->num_subauth - 1]); >>> CID 1506810: Control flow issues (NO_EFFECT) >>> This greater-than-or-equal-to-zero comparison of an unsigned value >>> is always true. "id >= 0U". 277 if (id >= 0) { 278 /* 279 * Translate raw sid into kuid in the server's user 280 * namespace. 281 */ 282 uid = make_kuid(&init_user_ns, id); Addresses-Coverity: ("Control flow issues") Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smbacl.c | 48 ++++++++++++++++++++++-------------------------- 1 file changed, 22 insertions(+), 26 deletions(-) (limited to 'fs') diff --git a/fs/ksmbd/smbacl.c b/fs/ksmbd/smbacl.c index 16da99a9963c..0a95cdec8c80 100644 --- a/fs/ksmbd/smbacl.c +++ b/fs/ksmbd/smbacl.c @@ -274,38 +274,34 @@ static int sid_to_id(struct user_namespace *user_ns, uid_t id; id = le32_to_cpu(psid->sub_auth[psid->num_subauth - 1]); - if (id >= 0) { - /* - * Translate raw sid into kuid in the server's user - * namespace. - */ - uid = make_kuid(&init_user_ns, id); - - /* If this is an idmapped mount, apply the idmapping. */ - uid = kuid_from_mnt(user_ns, uid); - if (uid_valid(uid)) { - fattr->cf_uid = uid; - rc = 0; - } + /* + * Translate raw sid into kuid in the server's user + * namespace. + */ + uid = make_kuid(&init_user_ns, id); + + /* If this is an idmapped mount, apply the idmapping. */ + uid = kuid_from_mnt(user_ns, uid); + if (uid_valid(uid)) { + fattr->cf_uid = uid; + rc = 0; } } else { kgid_t gid; gid_t id; id = le32_to_cpu(psid->sub_auth[psid->num_subauth - 1]); - if (id >= 0) { - /* - * Translate raw sid into kgid in the server's user - * namespace. - */ - gid = make_kgid(&init_user_ns, id); - - /* If this is an idmapped mount, apply the idmapping. */ - gid = kgid_from_mnt(user_ns, gid); - if (gid_valid(gid)) { - fattr->cf_gid = gid; - rc = 0; - } + /* + * Translate raw sid into kgid in the server's user + * namespace. + */ + gid = make_kgid(&init_user_ns, id); + + /* If this is an idmapped mount, apply the idmapping. */ + gid = kgid_from_mnt(user_ns, gid); + if (gid_valid(gid)) { + fattr->cf_gid = gid; + rc = 0; } } -- cgit v1.2.3