From 6cfbcf2f40e371ce36c030addc539597d058b3a9 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Wed, 7 Jul 2021 14:55:31 +0900 Subject: [PATCH 01/12] ksmbd: remove unneeded NULL check in for_each_netdev netdev can never be NULL in for_each_netdev loop. This patch remove unneeded NULL check. Reported-by: Coverity Scan Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smb2pdu.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 994b95b6b3c2e..2811dfabfa75a 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -7004,11 +7004,6 @@ static int fsctl_query_iface_info_ioctl(struct ksmbd_conn *conn, rtnl_lock(); for_each_netdev(&init_net, netdev) { - if (unlikely(!netdev)) { - rtnl_unlock(); - return -EINVAL; - } - if (netdev->type == ARPHRD_LOOPBACK) continue; From b8fc94cdb144467d88f35344076fd3621af93a17 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Wed, 7 Jul 2021 14:56:44 +0900 Subject: [PATCH 02/12] ksmbd: fix read on the uninitialized send_ctx If st->status is not SMB_DIRECT_CS_CONNECTED, It will jump done label and accessing the uninitialized send_ctxi by smb_direct_flush_send_list will cause kernel oops. This patch just return -ENOTCONN to avoid it. Reported-by: Coverity Scan Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/transport_rdma.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/fs/ksmbd/transport_rdma.c b/fs/ksmbd/transport_rdma.c index 171fb3dd018aa..d5728c84a15ae 100644 --- a/fs/ksmbd/transport_rdma.c +++ b/fs/ksmbd/transport_rdma.c @@ -1207,10 +1207,8 @@ static int smb_direct_writev(struct ksmbd_transport *t, struct kvec vec; struct smb_direct_send_ctx send_ctx; - if (st->status != SMB_DIRECT_CS_CONNECTED) { - ret = -ENOTCONN; - goto done; - } + if (st->status != SMB_DIRECT_CS_CONNECTED) + return -ENOTCONN; //FIXME: skip RFC1002 header.. buflen -= 4; From dac0ec6e1b4a876abb61b6cd2ec589f8e87e95c9 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Wed, 7 Jul 2021 14:57:24 +0900 Subject: [PATCH 03/12] ksmbd: fix memory leak smb2_populate_readdir_entry() Add missing kfree(conv_name) on error path. Reported-by: Coverity Scan Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smb2pdu.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 2811dfabfa75a..ea406ff1a8c1a 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -3291,7 +3291,7 @@ static int smb2_populate_readdir_entry(struct ksmbd_conn *conn, int info_level, char *conv_name; int conv_len; void *kstat; - int struct_sz; + int struct_sz, rc = 0; conv_name = ksmbd_convert_dir_info_name(d_info, conn->local_nls, @@ -3301,8 +3301,8 @@ static int smb2_populate_readdir_entry(struct ksmbd_conn *conn, int info_level, /* Somehow the name has only terminating NULL bytes */ if (conv_len < 0) { - kfree(conv_name); - return -EINVAL; + rc = -EINVAL; + goto free_conv_name; } struct_sz = readdir_info_level_struct_sz(info_level); @@ -3311,7 +3311,8 @@ static int smb2_populate_readdir_entry(struct ksmbd_conn *conn, int info_level, if (next_entry_offset > d_info->out_buf_len) { d_info->out_buf_len = 0; - return -ENOSPC; + rc = -ENOSPC; + goto free_conv_name; } kstat = d_info->wptr; @@ -3453,14 +3454,15 @@ static int smb2_populate_readdir_entry(struct ksmbd_conn *conn, int info_level, d_info->data_count += next_entry_offset; d_info->out_buf_len -= next_entry_offset; d_info->wptr += next_entry_offset; - kfree(conv_name); ksmbd_debug(SMB, "info_level : %d, buf_len :%d, next_offset : %d, data_count : %d\n", info_level, d_info->out_buf_len, next_entry_offset, d_info->data_count); - return 0; +free_conv_name: + kfree(conv_name); + return rc; } struct smb2_query_dir_private { From a9071e3c8659d777eb6527e1d377021381d1b5ec Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Wed, 7 Jul 2021 15:01:21 +0900 Subject: [PATCH 04/12] ksmbd: fix memory leak in smb_inherit_dacl() Add two labels to fix memory leak in smb_inherit_dacl(). Reported-by: Coverity Scan Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smbacl.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/fs/ksmbd/smbacl.c b/fs/ksmbd/smbacl.c index 86ef2a701cc2e..fa99d950a6f25 100644 --- a/fs/ksmbd/smbacl.c +++ b/fs/ksmbd/smbacl.c @@ -962,25 +962,29 @@ int smb_inherit_dacl(struct ksmbd_conn *conn, struct dentry *parent = path->dentry->d_parent; struct user_namespace *user_ns = mnt_user_ns(path->mnt); int inherited_flags = 0, flags = 0, i, ace_cnt = 0, nt_size = 0; - int rc = -ENOENT, num_aces, dacloffset, pntsd_type, acl_len; + int rc = 0, num_aces, dacloffset, pntsd_type, acl_len; char *aces_base; bool is_dir = S_ISDIR(d_inode(path->dentry)->i_mode); acl_len = ksmbd_vfs_get_sd_xattr(conn, user_ns, parent, &parent_pntsd); if (acl_len <= 0) - return rc; + return -ENOENT; dacloffset = le32_to_cpu(parent_pntsd->dacloffset); - if (!dacloffset) - goto out; + if (!dacloffset) { + rc = -EINVAL; + goto free_parent_pntsd; + } parent_pdacl = (struct smb_acl *)((char *)parent_pntsd + dacloffset); num_aces = le32_to_cpu(parent_pdacl->num_aces); pntsd_type = le16_to_cpu(parent_pntsd->type); aces_base = kmalloc(sizeof(struct smb_ace) * num_aces * 2, GFP_KERNEL); - if (!aces_base) - goto out; + if (!aces_base) { + rc = -ENOMEM; + goto free_parent_pntsd; + } aces = (struct smb_ace *)aces_base; parent_aces = (struct smb_ace *)((char *)parent_pdacl + @@ -1060,7 +1064,7 @@ int smb_inherit_dacl(struct ksmbd_conn *conn, nt_size, GFP_KERNEL); if (!pntsd) { rc = -ENOMEM; - goto out; + goto free_aces_base; } pntsd->revision = cpu_to_le16(1); @@ -1101,11 +1105,12 @@ int smb_inherit_dacl(struct ksmbd_conn *conn, ksmbd_vfs_set_sd_xattr(conn, user_ns, path->dentry, pntsd, pntsd_size); kfree(pntsd); - rc = 0; } +free_aces_base: kfree(aces_base); -out: +free_parent_pntsd: + kfree(parent_pntsd); return rc; } From 3867369ef8f760155da684e10d29e0bf9b733b48 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Thu, 8 Jul 2021 12:32:27 +0900 Subject: [PATCH 05/12] ksmbd: change data type of volatile/persistent id to u64 This patch change data type of volatile/persistent id to u64 to make issue from idr_find and idr_remove(). !HAS_FILE_ID check will protect integer overflow issue from idr_find and idr_remove(). Reported-by: Dan Carpenter Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/ksmbd_work.h | 6 +++--- fs/ksmbd/smb2pdu.c | 37 +++++++++++++++++++------------------ fs/ksmbd/vfs_cache.c | 32 ++++++++++++++++---------------- fs/ksmbd/vfs_cache.h | 20 +++++++++----------- 4 files changed, 47 insertions(+), 48 deletions(-) diff --git a/fs/ksmbd/ksmbd_work.h b/fs/ksmbd/ksmbd_work.h index c655bf371ce54..f7156bc500496 100644 --- a/fs/ksmbd/ksmbd_work.h +++ b/fs/ksmbd/ksmbd_work.h @@ -43,9 +43,9 @@ struct ksmbd_work { * Current Local FID assigned compound response if SMB2 CREATE * command is present in compound request */ - unsigned int compound_fid; - unsigned int compound_pfid; - unsigned int compound_sid; + u64 compound_fid; + u64 compound_pfid; + u64 compound_sid; const struct cred *saved_cred; diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index ea406ff1a8c1a..18e275abc68fe 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -2809,7 +2809,7 @@ int smb2_open(struct ksmbd_work *work) /* Get Persistent-ID */ ksmbd_open_durable_fd(fp); - if (!HAS_FILE_ID(fp->persistent_id)) { + if (!has_file_id(fp->persistent_id)) { rc = -ENOMEM; goto err_out; } @@ -4577,15 +4577,15 @@ static int smb2_get_info_file(struct ksmbd_work *work, } if (work->next_smb2_rcv_hdr_off) { - if (!HAS_FILE_ID(le64_to_cpu(req->VolatileFileId))) { - ksmbd_debug(SMB, "Compound request set FID = %u\n", + if (!has_file_id(le64_to_cpu(req->VolatileFileId))) { + ksmbd_debug(SMB, "Compound request set FID = %llu\n", work->compound_fid); id = work->compound_fid; pid = work->compound_pfid; } } - if (!HAS_FILE_ID(id)) { + if (!has_file_id(id)) { id = le64_to_cpu(req->VolatileFileId); pid = le64_to_cpu(req->PersistentFileId); } @@ -4949,15 +4949,15 @@ static int smb2_get_info_sec(struct ksmbd_work *work, } if (work->next_smb2_rcv_hdr_off) { - if (!HAS_FILE_ID(le64_to_cpu(req->VolatileFileId))) { - ksmbd_debug(SMB, "Compound request set FID = %u\n", + if (!has_file_id(le64_to_cpu(req->VolatileFileId))) { + ksmbd_debug(SMB, "Compound request set FID = %llu\n", work->compound_fid); id = work->compound_fid; pid = work->compound_pfid; } } - if (!HAS_FILE_ID(id)) { + if (!has_file_id(id)) { id = le64_to_cpu(req->VolatileFileId); pid = le64_to_cpu(req->PersistentFileId); } @@ -5083,7 +5083,7 @@ static noinline int smb2_close_pipe(struct ksmbd_work *work) */ int smb2_close(struct ksmbd_work *work) { - unsigned int volatile_id = KSMBD_NO_FID; + u64 volatile_id = KSMBD_NO_FID; u64 sess_id; struct smb2_close_req *req; struct smb2_close_rsp *rsp; @@ -5119,15 +5119,16 @@ int smb2_close(struct ksmbd_work *work) } if (work->next_smb2_rcv_hdr_off && - !HAS_FILE_ID(le64_to_cpu(req->VolatileFileId))) { - if (!HAS_FILE_ID(work->compound_fid)) { + !has_file_id(le64_to_cpu(req->VolatileFileId))) { + if (!has_file_id(work->compound_fid)) { /* file already closed, return FILE_CLOSED */ ksmbd_debug(SMB, "file already closed\n"); rsp->hdr.Status = STATUS_FILE_CLOSED; err = -EBADF; goto out; } else { - ksmbd_debug(SMB, "Compound request set FID = %u:%u\n", + ksmbd_debug(SMB, + "Compound request set FID = %llu:%llu\n", work->compound_fid, work->compound_pfid); volatile_id = work->compound_fid; @@ -5139,7 +5140,7 @@ int smb2_close(struct ksmbd_work *work) } else { volatile_id = le64_to_cpu(req->VolatileFileId); } - ksmbd_debug(SMB, "volatile_id = %u\n", volatile_id); + ksmbd_debug(SMB, "volatile_id = %llu\n", volatile_id); rsp->StructureSize = cpu_to_le16(60); rsp->Reserved = 0; @@ -5789,8 +5790,8 @@ int smb2_set_info(struct ksmbd_work *work) if (work->next_smb2_rcv_hdr_off) { req = ksmbd_req_buf_next(work); rsp = ksmbd_resp_buf_next(work); - if (!HAS_FILE_ID(le64_to_cpu(req->VolatileFileId))) { - ksmbd_debug(SMB, "Compound request set FID = %u\n", + if (!has_file_id(le64_to_cpu(req->VolatileFileId))) { + ksmbd_debug(SMB, "Compound request set FID = %llu\n", work->compound_fid); id = work->compound_fid; pid = work->compound_pfid; @@ -5800,7 +5801,7 @@ int smb2_set_info(struct ksmbd_work *work) rsp = work->response_buf; } - if (!HAS_FILE_ID(id)) { + if (!has_file_id(id)) { id = le64_to_cpu(req->VolatileFileId); pid = le64_to_cpu(req->PersistentFileId); } @@ -7287,8 +7288,8 @@ int smb2_ioctl(struct ksmbd_work *work) if (work->next_smb2_rcv_hdr_off) { req = ksmbd_req_buf_next(work); rsp = ksmbd_resp_buf_next(work); - if (!HAS_FILE_ID(le64_to_cpu(req->VolatileFileId))) { - ksmbd_debug(SMB, "Compound request set FID = %u\n", + if (!has_file_id(le64_to_cpu(req->VolatileFileId))) { + ksmbd_debug(SMB, "Compound request set FID = %llu\n", work->compound_fid); id = work->compound_fid; } @@ -7297,7 +7298,7 @@ int smb2_ioctl(struct ksmbd_work *work) rsp = work->response_buf; } - if (!HAS_FILE_ID(id)) + if (!has_file_id(id)) id = le64_to_cpu(req->VolatileFileId); if (req->Flags != cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL)) { diff --git a/fs/ksmbd/vfs_cache.c b/fs/ksmbd/vfs_cache.c index 1941ad3f5aa50..c54c605637a06 100644 --- a/fs/ksmbd/vfs_cache.c +++ b/fs/ksmbd/vfs_cache.c @@ -277,7 +277,7 @@ static void __ksmbd_inode_close(struct ksmbd_file *fp) static void __ksmbd_remove_durable_fd(struct ksmbd_file *fp) { - if (!HAS_FILE_ID(fp->persistent_id)) + if (!has_file_id(fp->persistent_id)) return; write_lock(&global_ft.lock); @@ -287,7 +287,7 @@ static void __ksmbd_remove_durable_fd(struct ksmbd_file *fp) static void __ksmbd_remove_fd(struct ksmbd_file_table *ft, struct ksmbd_file *fp) { - if (!HAS_FILE_ID(fp->volatile_id)) + if (!has_file_id(fp->volatile_id)) return; write_lock(&fp->f_ci->m_lock); @@ -327,10 +327,13 @@ static struct ksmbd_file *ksmbd_fp_get(struct ksmbd_file *fp) } static struct ksmbd_file *__ksmbd_lookup_fd(struct ksmbd_file_table *ft, - unsigned int id) + u64 id) { struct ksmbd_file *fp; + if (!has_file_id(id)) + return NULL; + read_lock(&ft->lock); fp = idr_find(ft->idr, id); if (fp) @@ -359,12 +362,12 @@ static void set_close_state_blocked_works(struct ksmbd_file *fp) spin_unlock(&fp->f_lock); } -int ksmbd_close_fd(struct ksmbd_work *work, unsigned int id) +int ksmbd_close_fd(struct ksmbd_work *work, u64 id) { struct ksmbd_file *fp; struct ksmbd_file_table *ft; - if (!HAS_FILE_ID(id)) + if (!has_file_id(id)) return 0; ft = &work->sess->file_table; @@ -404,12 +407,12 @@ static bool __sanity_check(struct ksmbd_tree_connect *tcon, struct ksmbd_file *f return true; } -struct ksmbd_file *ksmbd_lookup_foreign_fd(struct ksmbd_work *work, unsigned int id) +struct ksmbd_file *ksmbd_lookup_foreign_fd(struct ksmbd_work *work, u64 id) { return __ksmbd_lookup_fd(&work->sess->file_table, id); } -struct ksmbd_file *ksmbd_lookup_fd_fast(struct ksmbd_work *work, unsigned int id) +struct ksmbd_file *ksmbd_lookup_fd_fast(struct ksmbd_work *work, u64 id) { struct ksmbd_file *fp = __ksmbd_lookup_fd(&work->sess->file_table, id); @@ -420,19 +423,16 @@ struct ksmbd_file *ksmbd_lookup_fd_fast(struct ksmbd_work *work, unsigned int id return NULL; } -struct ksmbd_file *ksmbd_lookup_fd_slow(struct ksmbd_work *work, unsigned int id, - unsigned int pid) +struct ksmbd_file *ksmbd_lookup_fd_slow(struct ksmbd_work *work, u64 id, + u64 pid) { struct ksmbd_file *fp; - if (!HAS_FILE_ID(id)) { + if (!has_file_id(id)) { id = work->compound_fid; pid = work->compound_pfid; } - if (!HAS_FILE_ID(id)) - return NULL; - fp = __ksmbd_lookup_fd(&work->sess->file_table, id); if (!__sanity_check(work->tcon, fp)) { ksmbd_fd_put(work, fp); @@ -494,7 +494,7 @@ struct ksmbd_file *ksmbd_lookup_fd_inode(struct inode *inode) #define OPEN_ID_TYPE_VOLATILE_ID (0) #define OPEN_ID_TYPE_PERSISTENT_ID (1) -static void __open_id_set(struct ksmbd_file *fp, unsigned int id, int type) +static void __open_id_set(struct ksmbd_file *fp, u64 id, int type) { if (type == OPEN_ID_TYPE_VOLATILE_ID) fp->volatile_id = id; @@ -505,7 +505,7 @@ static void __open_id_set(struct ksmbd_file *fp, unsigned int id, int type) static int __open_id(struct ksmbd_file_table *ft, struct ksmbd_file *fp, int type) { - unsigned int id = 0; + u64 id = 0; int ret; if (type == OPEN_ID_TYPE_VOLATILE_ID && fd_limit_depleted()) { @@ -515,7 +515,7 @@ static int __open_id(struct ksmbd_file_table *ft, struct ksmbd_file *fp, idr_preload(GFP_KERNEL); write_lock(&ft->lock); - ret = idr_alloc_cyclic(ft->idr, fp, 0, INT_MAX, GFP_NOWAIT); + ret = idr_alloc_cyclic(ft->idr, fp, 0, INT_MAX - 1, GFP_NOWAIT); if (ret >= 0) { id = ret; ret = 0; diff --git a/fs/ksmbd/vfs_cache.h b/fs/ksmbd/vfs_cache.h index 543494f664cb1..70e987293564e 100644 --- a/fs/ksmbd/vfs_cache.h +++ b/fs/ksmbd/vfs_cache.h @@ -22,7 +22,7 @@ #define FILE_GENERIC_EXECUTE 0X1200a0 #define KSMBD_START_FID 0 -#define KSMBD_NO_FID (UINT_MAX) +#define KSMBD_NO_FID (INT_MAX) #define SMB2_NO_FID (0xFFFFFFFFFFFFFFFFULL) struct ksmbd_conn; @@ -62,8 +62,8 @@ struct ksmbd_inode { struct ksmbd_file { struct file *filp; char *filename; - unsigned int persistent_id; - unsigned int volatile_id; + u64 persistent_id; + u64 volatile_id; spinlock_t f_lock; @@ -122,10 +122,8 @@ struct ksmbd_file_table { struct idr *idr; }; -static inline bool HAS_FILE_ID(unsigned long long req) +static inline bool has_file_id(u64 id) { - unsigned int id = (unsigned int)req; - return id < KSMBD_NO_FID; } @@ -136,11 +134,11 @@ static inline bool ksmbd_stream_fd(struct ksmbd_file *fp) int ksmbd_init_file_table(struct ksmbd_file_table *ft); void ksmbd_destroy_file_table(struct ksmbd_file_table *ft); -int ksmbd_close_fd(struct ksmbd_work *work, unsigned int id); -struct ksmbd_file *ksmbd_lookup_fd_fast(struct ksmbd_work *work, unsigned int id); -struct ksmbd_file *ksmbd_lookup_foreign_fd(struct ksmbd_work *work, unsigned int id); -struct ksmbd_file *ksmbd_lookup_fd_slow(struct ksmbd_work *work, unsigned int id, - unsigned int pid); +int ksmbd_close_fd(struct ksmbd_work *work, u64 id); +struct ksmbd_file *ksmbd_lookup_fd_fast(struct ksmbd_work *work, u64 id); +struct ksmbd_file *ksmbd_lookup_foreign_fd(struct ksmbd_work *work, u64 id); +struct ksmbd_file *ksmbd_lookup_fd_slow(struct ksmbd_work *work, u64 id, + u64 pid); void ksmbd_fd_put(struct ksmbd_work *work, struct ksmbd_file *fp); struct ksmbd_file *ksmbd_lookup_durable_fd(unsigned long long id); struct ksmbd_file *ksmbd_lookup_fd_cguid(char *cguid); From 0f6619aee86f11cee0c5063777c4febdf18cb28b Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Wed, 7 Jul 2021 13:15:40 +0300 Subject: [PATCH 06/12] ksmbd: delete some stray tabs These lines are intended one tab too far. Signed-off-by: Dan Carpenter Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smb2pdu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 18e275abc68fe..d817684312494 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -6699,7 +6699,7 @@ int smb2_lock(struct ksmbd_work *work) cmp_lock->start < smb_lock->end) { pr_err("previous lock conflict with zero byte lock range\n"); rsp->hdr.Status = STATUS_LOCK_NOT_GRANTED; - goto out; + goto out; } if (smb_lock->zero_len && !cmp_lock->zero_len && @@ -6707,7 +6707,7 @@ int smb2_lock(struct ksmbd_work *work) smb_lock->start < cmp_lock->end) { pr_err("current lock conflict with zero byte lock range\n"); rsp->hdr.Status = STATUS_LOCK_NOT_GRANTED; - goto out; + goto out; } if (((cmp_lock->start <= smb_lock->start && From 07781de9051859d2f38a9e199384c64bb1924c47 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Wed, 7 Jul 2021 13:15:32 +0300 Subject: [PATCH 07/12] ksmbd: use kasprintf() in ksmbd_vfs_xattr_stream_name() Simplify the code by using kasprintf(). This also silences a Smatch warning: fs/ksmbd/vfs.c:1725 ksmbd_vfs_xattr_stream_name() warn: inconsistent indenting Signed-off-by: Dan Carpenter Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/vfs.c | 27 ++++++--------------------- 1 file changed, 6 insertions(+), 21 deletions(-) diff --git a/fs/ksmbd/vfs.c b/fs/ksmbd/vfs.c index 7339d5c74aadf..38677c20d048c 100644 --- a/fs/ksmbd/vfs.c +++ b/fs/ksmbd/vfs.c @@ -1698,35 +1698,20 @@ ssize_t ksmbd_vfs_casexattr_len(struct user_namespace *user_ns, int ksmbd_vfs_xattr_stream_name(char *stream_name, char **xattr_stream_name, size_t *xattr_stream_name_size, int s_type) { - int stream_name_size; - char *xattr_stream_name_buf; - char *type; - int type_len; + char *type, *buf; if (s_type == DIR_STREAM) type = ":$INDEX_ALLOCATION"; else type = ":$DATA"; - type_len = strlen(type); - stream_name_size = strlen(stream_name); - *xattr_stream_name_size = stream_name_size + XATTR_NAME_STREAM_LEN + 1; - xattr_stream_name_buf = kmalloc(*xattr_stream_name_size + type_len, - GFP_KERNEL); - if (!xattr_stream_name_buf) + buf = kasprintf(GFP_KERNEL, "%s%s%s", + XATTR_NAME_STREAM, stream_name, type); + if (!buf) return -ENOMEM; - memcpy(xattr_stream_name_buf, XATTR_NAME_STREAM, XATTR_NAME_STREAM_LEN); - - if (stream_name_size) { - memcpy(&xattr_stream_name_buf[XATTR_NAME_STREAM_LEN], - stream_name, stream_name_size); - } - memcpy(&xattr_stream_name_buf[*xattr_stream_name_size - 1], type, type_len); - *xattr_stream_name_size += type_len; - - xattr_stream_name_buf[*xattr_stream_name_size - 1] = '\0'; - *xattr_stream_name = xattr_stream_name_buf; + *xattr_stream_name = buf; + *xattr_stream_name_size = strlen(buf) + 1; return 0; } From 4b92841ef27b56883fa4491a3d51db3eef68c481 Mon Sep 17 00:00:00 2001 From: Hyunchul Lee Date: Sat, 10 Jul 2021 09:31:08 +0900 Subject: [PATCH 08/12] ksmbd: fix the running request count decrement decrement the count of running requests after sending the last response for multi-response requests. Signed-off-by: Hyunchul Lee Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/connection.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c index 928e22e19def1..6e51e08addee8 100644 --- a/fs/ksmbd/connection.c +++ b/fs/ksmbd/connection.c @@ -120,7 +120,8 @@ int ksmbd_conn_try_dequeue_request(struct ksmbd_work *work) list_empty(&work->async_request_entry)) return 0; - atomic_dec(&conn->req_running); + if (!work->multiRsp) + atomic_dec(&conn->req_running); spin_lock(&conn->request_lock); if (!work->multiRsp) { list_del_init(&work->request_entry); From d63528eb0d43c4796c42aad56889dec12cf4e122 Mon Sep 17 00:00:00 2001 From: Hyunchul Lee Date: Sat, 10 Jul 2021 16:22:41 +0900 Subject: [PATCH 09/12] ksmbd: free ksmbd_lock when file is closed Append ksmbd_lock into the connection's lock list and the ksmbd_file's lock list. And when a file is closed, detach ksmbd_lock from these lists and free it. Signed-off-by: Hyunchul Lee Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/connection.c | 7 +- fs/ksmbd/connection.h | 6 ++ fs/ksmbd/smb2pdu.c | 154 ++++++++++++++++++++++++++---------------- fs/ksmbd/smb_common.c | 2 - fs/ksmbd/smb_common.h | 2 - fs/ksmbd/vfs_cache.c | 16 +++++ fs/ksmbd/vfs_cache.h | 4 +- 7 files changed, 125 insertions(+), 66 deletions(-) diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c index 6e51e08addee8..8430848bea452 100644 --- a/fs/ksmbd/connection.c +++ b/fs/ksmbd/connection.c @@ -19,8 +19,8 @@ static DEFINE_MUTEX(init_lock); static struct ksmbd_conn_ops default_conn_ops; -static LIST_HEAD(conn_list); -static DEFINE_RWLOCK(conn_list_lock); +LIST_HEAD(conn_list); +DEFINE_RWLOCK(conn_list_lock); /** * ksmbd_conn_free() - free resources of the connection instance @@ -70,6 +70,9 @@ struct ksmbd_conn *ksmbd_conn_alloc(void) spin_lock_init(&conn->credits_lock); ida_init(&conn->async_ida); + spin_lock_init(&conn->llist_lock); + INIT_LIST_HEAD(&conn->lock_list); + write_lock(&conn_list_lock); list_add(&conn->conns_list, &conn_list); write_unlock(&conn_list_lock); diff --git a/fs/ksmbd/connection.h b/fs/ksmbd/connection.h index 98108b41f7397..487c2024b0d56 100644 --- a/fs/ksmbd/connection.h +++ b/fs/ksmbd/connection.h @@ -79,6 +79,9 @@ struct ksmbd_conn { char *ntlmssp_cryptkey; }; + spinlock_t llist_lock; + struct list_head lock_list; + struct preauth_integrity_info *preauth_info; bool need_neg; @@ -138,6 +141,9 @@ struct ksmbd_transport { #define KSMBD_TCP_SEND_TIMEOUT (5 * HZ) #define KSMBD_TCP_PEER_SOCKADDR(c) ((struct sockaddr *)&((c)->peer_addr)) +extern struct list_head conn_list; +extern rwlock_t conn_list_lock; + bool ksmbd_conn_alive(struct ksmbd_conn *conn); void ksmbd_conn_wait_idle(struct ksmbd_conn *conn); struct ksmbd_conn *ksmbd_conn_alloc(void); diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index d817684312494..99e2368ae672f 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -6513,8 +6513,9 @@ static struct ksmbd_lock *smb2_lock_init(struct file_lock *flock, lock->flags = flags; if (lock->start == lock->end) lock->zero_len = 1; + INIT_LIST_HEAD(&lock->clist); + INIT_LIST_HEAD(&lock->flist); INIT_LIST_HEAD(&lock->llist); - INIT_LIST_HEAD(&lock->glist); list_add_tail(&lock->llist, lock_list); return lock; @@ -6553,7 +6554,8 @@ int smb2_lock(struct ksmbd_work *work) int cmd = 0; int err = 0, i; u64 lock_start, lock_length; - struct ksmbd_lock *smb_lock = NULL, *cmp_lock, *tmp; + struct ksmbd_lock *smb_lock = NULL, *cmp_lock, *tmp, *tmp2; + struct ksmbd_conn *conn; int nolock = 0; LIST_HEAD(lock_list); LIST_HEAD(rollback_list); @@ -6662,72 +6664,89 @@ int smb2_lock(struct ksmbd_work *work) if (!(smb_lock->flags & SMB2_LOCKFLAG_UNLOCK) && !(smb_lock->flags & SMB2_LOCKFLAG_FAIL_IMMEDIATELY)) - goto no_check_gl; + goto no_check_cl; nolock = 1; - /* check locks in global list */ - list_for_each_entry(cmp_lock, &global_lock_list, glist) { - if (file_inode(cmp_lock->fl->fl_file) != - file_inode(smb_lock->fl->fl_file)) - continue; + /* check locks in connection list */ + read_lock(&conn_list_lock); + list_for_each_entry(conn, &conn_list, conns_list) { + spin_lock(&conn->llist_lock); + list_for_each_entry_safe(cmp_lock, tmp2, &conn->lock_list, clist) { + if (file_inode(cmp_lock->fl->fl_file) != + file_inode(smb_lock->fl->fl_file)) + continue; - if (smb_lock->fl->fl_type == F_UNLCK) { - if (cmp_lock->fl->fl_file == smb_lock->fl->fl_file && - cmp_lock->start == smb_lock->start && - cmp_lock->end == smb_lock->end && - !lock_defer_pending(cmp_lock->fl)) { - nolock = 0; - locks_free_lock(cmp_lock->fl); - list_del(&cmp_lock->glist); - kfree(cmp_lock); - break; + if (smb_lock->fl->fl_type == F_UNLCK) { + if (cmp_lock->fl->fl_file == smb_lock->fl->fl_file && + cmp_lock->start == smb_lock->start && + cmp_lock->end == smb_lock->end && + !lock_defer_pending(cmp_lock->fl)) { + nolock = 0; + list_del(&cmp_lock->flist); + list_del(&cmp_lock->clist); + spin_unlock(&conn->llist_lock); + read_unlock(&conn_list_lock); + + locks_free_lock(cmp_lock->fl); + kfree(cmp_lock); + goto out_check_cl; + } + continue; } - continue; - } - if (cmp_lock->fl->fl_file == smb_lock->fl->fl_file) { - if (smb_lock->flags & SMB2_LOCKFLAG_SHARED) - continue; - } else { - if (cmp_lock->flags & SMB2_LOCKFLAG_SHARED) - continue; - } + if (cmp_lock->fl->fl_file == smb_lock->fl->fl_file) { + if (smb_lock->flags & SMB2_LOCKFLAG_SHARED) + continue; + } else { + if (cmp_lock->flags & SMB2_LOCKFLAG_SHARED) + continue; + } - /* check zero byte lock range */ - if (cmp_lock->zero_len && !smb_lock->zero_len && - cmp_lock->start > smb_lock->start && - cmp_lock->start < smb_lock->end) { - pr_err("previous lock conflict with zero byte lock range\n"); - rsp->hdr.Status = STATUS_LOCK_NOT_GRANTED; - goto out; - } + /* check zero byte lock range */ + if (cmp_lock->zero_len && !smb_lock->zero_len && + cmp_lock->start > smb_lock->start && + cmp_lock->start < smb_lock->end) { + spin_unlock(&conn->llist_lock); + read_unlock(&conn_list_lock); + pr_err("previous lock conflict with zero byte lock range\n"); + rsp->hdr.Status = STATUS_LOCK_NOT_GRANTED; + goto out; + } - if (smb_lock->zero_len && !cmp_lock->zero_len && - smb_lock->start > cmp_lock->start && - smb_lock->start < cmp_lock->end) { - pr_err("current lock conflict with zero byte lock range\n"); - rsp->hdr.Status = STATUS_LOCK_NOT_GRANTED; - goto out; - } + if (smb_lock->zero_len && !cmp_lock->zero_len && + smb_lock->start > cmp_lock->start && + smb_lock->start < cmp_lock->end) { + spin_unlock(&conn->llist_lock); + read_unlock(&conn_list_lock); + pr_err("current lock conflict with zero byte lock range\n"); + rsp->hdr.Status = STATUS_LOCK_NOT_GRANTED; + goto out; + } - if (((cmp_lock->start <= smb_lock->start && - cmp_lock->end > smb_lock->start) || - (cmp_lock->start < smb_lock->end && cmp_lock->end >= smb_lock->end)) && - !cmp_lock->zero_len && !smb_lock->zero_len) { - pr_err("Not allow lock operation on exclusive lock range\n"); - rsp->hdr.Status = - STATUS_LOCK_NOT_GRANTED; - goto out; + if (((cmp_lock->start <= smb_lock->start && + cmp_lock->end > smb_lock->start) || + (cmp_lock->start < smb_lock->end && + cmp_lock->end >= smb_lock->end)) && + !cmp_lock->zero_len && !smb_lock->zero_len) { + spin_unlock(&conn->llist_lock); + read_unlock(&conn_list_lock); + pr_err("Not allow lock operation on exclusive lock range\n"); + rsp->hdr.Status = + STATUS_LOCK_NOT_GRANTED; + goto out; + } } + spin_unlock(&conn->llist_lock); } - + read_unlock(&conn_list_lock); +out_check_cl: if (smb_lock->fl->fl_type == F_UNLCK && nolock) { pr_err("Try to unlock nolocked range\n"); rsp->hdr.Status = STATUS_RANGE_NOT_LOCKED; goto out; } -no_check_gl: +no_check_cl: if (smb_lock->zero_len) { err = 0; goto skip; @@ -6753,8 +6772,10 @@ int smb2_lock(struct ksmbd_work *work) ksmbd_debug(SMB, "would have to wait for getting lock\n"); - list_add_tail(&smb_lock->glist, - &global_lock_list); + spin_lock(&work->conn->llist_lock); + list_add_tail(&smb_lock->clist, + &work->conn->lock_list); + spin_unlock(&work->conn->llist_lock); list_add(&smb_lock->llist, &rollback_list); argv = kmalloc(sizeof(void *), GFP_KERNEL); @@ -6782,7 +6803,9 @@ int smb2_lock(struct ksmbd_work *work) if (work->state != KSMBD_WORK_ACTIVE) { list_del(&smb_lock->llist); - list_del(&smb_lock->glist); + spin_lock(&work->conn->llist_lock); + list_del(&smb_lock->clist); + spin_unlock(&work->conn->llist_lock); locks_free_lock(flock); if (work->state == KSMBD_WORK_CANCELLED) { @@ -6806,14 +6829,21 @@ int smb2_lock(struct ksmbd_work *work) } list_del(&smb_lock->llist); - list_del(&smb_lock->glist); + spin_lock(&work->conn->llist_lock); + list_del(&smb_lock->clist); + spin_unlock(&work->conn->llist_lock); + spin_lock(&fp->f_lock); list_del(&work->fp_entry); spin_unlock(&fp->f_lock); goto retry; } else if (!err) { - list_add_tail(&smb_lock->glist, - &global_lock_list); + spin_lock(&work->conn->llist_lock); + list_add_tail(&smb_lock->clist, + &work->conn->lock_list); + list_add_tail(&smb_lock->flist, + &fp->lock_list); + spin_unlock(&work->conn->llist_lock); list_add(&smb_lock->llist, &rollback_list); ksmbd_debug(SMB, "successful in taking lock\n"); } else { @@ -6852,8 +6882,14 @@ int smb2_lock(struct ksmbd_work *work) err = vfs_lock_file(filp, 0, rlock, NULL); if (err) pr_err("rollback unlock fail : %d\n", err); + list_del(&smb_lock->llist); - list_del(&smb_lock->glist); + spin_lock(&work->conn->llist_lock); + if (!list_empty(&smb_lock->flist)) + list_del(&smb_lock->flist); + list_del(&smb_lock->clist); + spin_unlock(&work->conn->llist_lock); + locks_free_lock(smb_lock->fl); locks_free_lock(rlock); kfree(smb_lock); diff --git a/fs/ksmbd/smb_common.c b/fs/ksmbd/smb_common.c index 38026d9bb7043..24c6bb476f6ef 100644 --- a/fs/ksmbd/smb_common.c +++ b/fs/ksmbd/smb_common.c @@ -23,8 +23,6 @@ static const char basechars[43] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ_-!@#$%"; #define mangle(V) ((char)(basechars[(V) % MANGLE_BASE])) #define KSMBD_MIN_SUPPORTED_HEADER_SIZE (sizeof(struct smb2_hdr)) -LIST_HEAD(global_lock_list); - struct smb_protocol { int index; char *name; diff --git a/fs/ksmbd/smb_common.h b/fs/ksmbd/smb_common.h index 6ab28aa330247..b8c350725905a 100644 --- a/fs/ksmbd/smb_common.h +++ b/fs/ksmbd/smb_common.h @@ -48,8 +48,6 @@ #define CIFS_DEFAULT_IOSIZE (64 * 1024) #define MAX_CIFS_SMALL_BUFFER_SIZE 448 /* big enough for most */ -extern struct list_head global_lock_list; - /* RFC 1002 session packet types */ #define RFC1002_SESSION_MESSAGE 0x00 #define RFC1002_SESSION_REQUEST 0x81 diff --git a/fs/ksmbd/vfs_cache.c b/fs/ksmbd/vfs_cache.c index c54c605637a06..92d8c61ffd2a5 100644 --- a/fs/ksmbd/vfs_cache.c +++ b/fs/ksmbd/vfs_cache.c @@ -302,6 +302,7 @@ static void __ksmbd_remove_fd(struct ksmbd_file_table *ft, struct ksmbd_file *fp static void __ksmbd_close_fd(struct ksmbd_file_table *ft, struct ksmbd_file *fp) { struct file *filp; + struct ksmbd_lock *smb_lock, *tmp_lock; fd_limit_close(); __ksmbd_remove_durable_fd(fp); @@ -313,6 +314,20 @@ static void __ksmbd_close_fd(struct ksmbd_file_table *ft, struct ksmbd_file *fp) __ksmbd_inode_close(fp); if (!IS_ERR_OR_NULL(filp)) fput(filp); + + /* because the reference count of fp is 0, it is guaranteed that + * there are not accesses to fp->lock_list. + */ + list_for_each_entry_safe(smb_lock, tmp_lock, &fp->lock_list, flist) { + spin_lock(&fp->conn->llist_lock); + list_del(&smb_lock->clist); + spin_unlock(&fp->conn->llist_lock); + + list_del(&smb_lock->flist); + locks_free_lock(smb_lock->fl); + kfree(smb_lock); + } + kfree(fp->filename); if (ksmbd_stream_fd(fp)) kfree(fp->stream.name); @@ -549,6 +564,7 @@ struct ksmbd_file *ksmbd_open_fd(struct ksmbd_work *work, struct file *filp) INIT_LIST_HEAD(&fp->blocked_works); INIT_LIST_HEAD(&fp->node); + INIT_LIST_HEAD(&fp->lock_list); spin_lock_init(&fp->f_lock); atomic_set(&fp->refcount, 1); diff --git a/fs/ksmbd/vfs_cache.h b/fs/ksmbd/vfs_cache.h index 70e987293564e..70dfe6a99f13f 100644 --- a/fs/ksmbd/vfs_cache.h +++ b/fs/ksmbd/vfs_cache.h @@ -30,7 +30,8 @@ struct ksmbd_session; struct ksmbd_lock { struct file_lock *fl; - struct list_head glist; + struct list_head clist; + struct list_head flist; struct list_head llist; unsigned int flags; int cmd; @@ -91,6 +92,7 @@ struct ksmbd_file { struct stream stream; struct list_head node; struct list_head blocked_works; + struct list_head lock_list; int durable_timeout; From 45a64e8b08493b768fa029a5508cec8cf2b89f2d Mon Sep 17 00:00:00 2001 From: Hyunchul Lee Date: Sat, 10 Jul 2021 09:34:20 +0900 Subject: [PATCH 10/12] ksmbd: uninterruptible wait for a file being unlocked the wait can be canceled by SMB2_CANCEL, SMB2_CLOSE, SMB2_LOGOFF, disconnection or shutdown, we don't have to use wait_event_interruptible. And this remove the warning from Coverity: CID 1502834 (#1 of 1): Unused value (UNUSED_VALUE) returned_value: Assigning value from ksmbd_vfs_posix_lock_wait(flock) to err here, but that stored value is overwritten before it can be used. Signed-off-by: Hyunchul Lee Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smb2pdu.c | 2 +- fs/ksmbd/vfs.c | 4 ++-- fs/ksmbd/vfs.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 99e2368ae672f..f73721c3b0e9b 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -6799,7 +6799,7 @@ int smb2_lock(struct ksmbd_work *work) smb2_send_interim_resp(work, STATUS_PENDING); - err = ksmbd_vfs_posix_lock_wait(flock); + ksmbd_vfs_posix_lock_wait(flock); if (work->state != KSMBD_WORK_ACTIVE) { list_del(&smb_lock->llist); diff --git a/fs/ksmbd/vfs.c b/fs/ksmbd/vfs.c index 38677c20d048c..88e947f69f474 100644 --- a/fs/ksmbd/vfs.c +++ b/fs/ksmbd/vfs.c @@ -1784,9 +1784,9 @@ int ksmbd_vfs_copy_file_ranges(struct ksmbd_work *work, return 0; } -int ksmbd_vfs_posix_lock_wait(struct file_lock *flock) +void ksmbd_vfs_posix_lock_wait(struct file_lock *flock) { - return wait_event_interruptible(flock->fl_wait, !flock->fl_blocker); + wait_event(flock->fl_wait, !flock->fl_blocker); } int ksmbd_vfs_posix_lock_wait_timeout(struct file_lock *flock, long timeout) diff --git a/fs/ksmbd/vfs.h b/fs/ksmbd/vfs.h index b255f90acf8f6..cb0cba0d5d076 100644 --- a/fs/ksmbd/vfs.h +++ b/fs/ksmbd/vfs.h @@ -168,7 +168,7 @@ int ksmbd_vfs_fill_dentry_attrs(struct ksmbd_work *work, struct user_namespace *user_ns, struct dentry *dentry, struct ksmbd_kstat *ksmbd_kstat); -int ksmbd_vfs_posix_lock_wait(struct file_lock *flock); +void ksmbd_vfs_posix_lock_wait(struct file_lock *flock); int ksmbd_vfs_posix_lock_wait_timeout(struct file_lock *flock, long timeout); void ksmbd_vfs_posix_lock_unblock(struct file_lock *flock); int ksmbd_vfs_remove_acl_xattrs(struct user_namespace *user_ns, From ce154c32af3c60727171ff28ae97bcceda63b1c6 Mon Sep 17 00:00:00 2001 From: Hyunchul Lee Date: Fri, 9 Jul 2021 17:06:33 +0900 Subject: [PATCH 11/12] ksmbd: make smb2_find_context_vals return NULL if not found instead of -ENOENT, make smb2_find_context_vals return NULL if the given context cannot be found. Reported-by: Dan Carpenter Signed-off-by: Hyunchul Lee Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/oplock.c | 2 +- fs/ksmbd/smb2pdu.c | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/fs/ksmbd/oplock.c b/fs/ksmbd/oplock.c index 71063568dfee3..8e53815eedc68 100644 --- a/fs/ksmbd/oplock.c +++ b/fs/ksmbd/oplock.c @@ -1472,7 +1472,7 @@ struct create_context *smb2_find_context_vals(void *open_req, const char *tag) next = le32_to_cpu(cc->Next); } while (next != 0); - return ERR_PTR(-ENOENT); + return NULL; } /** diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index f73721c3b0e9b..af33d4f95d440 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -2131,7 +2131,7 @@ static inline int check_context_err(void *ctx, char *str) int err; err = PTR_ERR(ctx); - ksmbd_debug(SMB, "find context %s err %d\n", str, err); + ksmbd_debug(SMB, "find context %s err %d\n", str, err ? err : -ENOENT); if (err == -EINVAL) { pr_err("bad name length\n"); @@ -2525,7 +2525,7 @@ int smb2_open(struct ksmbd_work *work) if (req->CreateContextsOffset) { /* Parse non-durable handle create contexts */ context = smb2_find_context_vals(req, SMB2_CREATE_EA_BUFFER); - if (IS_ERR(context)) { + if (IS_ERR_OR_NULL(context)) { rc = check_context_err(context, SMB2_CREATE_EA_BUFFER); if (rc < 0) goto err_out1; @@ -2540,7 +2540,7 @@ int smb2_open(struct ksmbd_work *work) context = smb2_find_context_vals(req, SMB2_CREATE_QUERY_MAXIMAL_ACCESS_REQUEST); - if (IS_ERR(context)) { + if (IS_ERR_OR_NULL(context)) { rc = check_context_err(context, SMB2_CREATE_QUERY_MAXIMAL_ACCESS_REQUEST); if (rc < 0) @@ -2553,7 +2553,7 @@ int smb2_open(struct ksmbd_work *work) context = smb2_find_context_vals(req, SMB2_CREATE_TIMEWARP_REQUEST); - if (IS_ERR(context)) { + if (IS_ERR_OR_NULL(context)) { rc = check_context_err(context, SMB2_CREATE_TIMEWARP_REQUEST); if (rc < 0) @@ -2567,7 +2567,7 @@ int smb2_open(struct ksmbd_work *work) if (tcon->posix_extensions) { context = smb2_find_context_vals(req, SMB2_CREATE_TAG_POSIX); - if (IS_ERR(context)) { + if (IS_ERR_OR_NULL(context)) { rc = check_context_err(context, SMB2_CREATE_TAG_POSIX); if (rc < 0) @@ -2970,7 +2970,7 @@ int smb2_open(struct ksmbd_work *work) az_req = (struct create_alloc_size_req *)smb2_find_context_vals(req, SMB2_CREATE_ALLOCATION_SIZE); - if (IS_ERR(az_req)) { + if (IS_ERR_OR_NULL(az_req)) { rc = check_context_err(az_req, SMB2_CREATE_ALLOCATION_SIZE); if (rc < 0) @@ -2992,7 +2992,7 @@ int smb2_open(struct ksmbd_work *work) } context = smb2_find_context_vals(req, SMB2_CREATE_QUERY_ON_DISK_ID); - if (IS_ERR(context)) { + if (IS_ERR_OR_NULL(context)) { rc = check_context_err(context, SMB2_CREATE_QUERY_ON_DISK_ID); if (rc < 0) goto err_out; From 21dd1fd6d718ac59841c3ee3d0b1d82508ef24dc Mon Sep 17 00:00:00 2001 From: Hyunchul Lee Date: Fri, 9 Jul 2021 17:06:34 +0900 Subject: [PATCH 12/12] ksmbd: handle error cases first in smb2_create_sd_buffers For code cleanup, handle error cases first in smb2_create_sd_buffers(). Reported-by: Dan Carpenter Signed-off-by: Hyunchul Lee Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smb2pdu.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index af33d4f95d440..2e266a9d39353 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -2319,25 +2319,23 @@ static int smb2_create_sd_buffer(struct ksmbd_work *work, struct path *path) { struct create_context *context; - int rc = -ENOENT; + struct create_sd_buf_req *sd_buf; if (!req->CreateContextsOffset) - return rc; + return -ENOENT; /* Parse SD BUFFER create contexts */ context = smb2_find_context_vals(req, SMB2_CREATE_SD_BUFFER); - if (context && !IS_ERR(context)) { - struct create_sd_buf_req *sd_buf; - - ksmbd_debug(SMB, - "Set ACLs using SMB2_CREATE_SD_BUFFER context\n"); - sd_buf = (struct create_sd_buf_req *)context; - rc = set_info_sec(work->conn, work->tcon, - path, &sd_buf->ntsd, - le32_to_cpu(sd_buf->ccontext.DataLength), true); - } + if (!context) + return -ENOENT; + else if (IS_ERR(context)) + return PTR_ERR(context); - return rc; + ksmbd_debug(SMB, + "Set ACLs using SMB2_CREATE_SD_BUFFER context\n"); + sd_buf = (struct create_sd_buf_req *)context; + return set_info_sec(work->conn, work->tcon, path, &sd_buf->ntsd, + le32_to_cpu(sd_buf->ccontext.DataLength), true); } static void ksmbd_acls_fattr(struct smb_fattr *fattr, struct inode *inode)