linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/4] ksmbd: add request buffer validation in smb2_set_info
@ 2021-09-18  9:45 Namjae Jeon
  2021-09-18  9:45 ` [PATCH 2/4] ksmbd: add validation in smb2_ioctl Namjae Jeon
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Namjae Jeon @ 2021-09-18  9:45 UTC (permalink / raw)
  To: linux-cifs; +Cc: Namjae Jeon, Ronnie Sahlberg, Ralph Böhme, Steve French

Add buffer validation in smb2_set_info.

Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Ralph Böhme <slow@samba.org>
Cc: Steve French <smfrench@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 fs/ksmbd/smb2pdu.c | 113 +++++++++++++++++++++++++++++++++++----------
 fs/ksmbd/smb2pdu.h |   9 ++++
 2 files changed, 97 insertions(+), 25 deletions(-)

diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index 46e0275a77a8..7763f69e1ae8 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -2107,17 +2107,23 @@ static noinline int create_smb2_pipe(struct ksmbd_work *work)
  * smb2_set_ea() - handler for setting extended attributes using set
  *		info command
  * @eabuf:	set info command buffer
+ * @buf_len:	set info command buffer length
  * @path:	dentry path for get ea
  *
  * Return:	0 on success, otherwise error
  */
-static int smb2_set_ea(struct smb2_ea_info *eabuf, struct path *path)
+static int smb2_set_ea(struct smb2_ea_info *eabuf, unsigned int buf_len,
+		       struct path *path)
 {
 	struct user_namespace *user_ns = mnt_user_ns(path->mnt);
 	char *attr_name = NULL, *value;
 	int rc = 0;
 	int next = 0;
 
+	if (buf_len < sizeof(struct smb2_ea_info) + eabuf->EaNameLength +
+			le16_to_cpu(eabuf->EaValueLength))
+		return -EINVAL;
+
 	attr_name = kmalloc(XATTR_NAME_MAX + 1, GFP_KERNEL);
 	if (!attr_name)
 		return -ENOMEM;
@@ -2181,7 +2187,13 @@ static int smb2_set_ea(struct smb2_ea_info *eabuf, struct path *path)
 
 next:
 		next = le32_to_cpu(eabuf->NextEntryOffset);
+		if (next == 0 || buf_len < next)
+			break;
+		buf_len -= next;
 		eabuf = (struct smb2_ea_info *)((char *)eabuf + next);
+		if (next < eabuf->EaNameLength + le16_to_cpu(eabuf->EaValueLength))
+			break;
+
 	} while (next != 0);
 
 	kfree(attr_name);
@@ -2790,7 +2802,9 @@ int smb2_open(struct ksmbd_work *work)
 		created = true;
 		user_ns = mnt_user_ns(path.mnt);
 		if (ea_buf) {
-			rc = smb2_set_ea(&ea_buf->ea, &path);
+			rc = smb2_set_ea(&ea_buf->ea,
+					 le32_to_cpu(ea_buf->ccontext.DataLength),
+					 &path);
 			if (rc == -EOPNOTSUPP)
 				rc = 0;
 			else if (rc)
@@ -5375,7 +5389,7 @@ static int smb2_rename(struct ksmbd_work *work,
 static int smb2_create_link(struct ksmbd_work *work,
 			    struct ksmbd_share_config *share,
 			    struct smb2_file_link_info *file_info,
-			    struct file *filp,
+			    int buf_len, struct file *filp,
 			    struct nls_table *local_nls)
 {
 	char *link_name = NULL, *target_name = NULL, *pathname = NULL;
@@ -5383,6 +5397,10 @@ static int smb2_create_link(struct ksmbd_work *work,
 	bool file_present = true;
 	int rc;
 
+	if (buf_len < sizeof(struct smb2_file_link_info) +
+			le32_to_cpu(file_info->FileNameLength))
+		return -EINVAL;
+
 	ksmbd_debug(SMB, "setting FILE_LINK_INFORMATION\n");
 	pathname = kmalloc(PATH_MAX, GFP_KERNEL);
 	if (!pathname)
@@ -5442,7 +5460,7 @@ static int smb2_create_link(struct ksmbd_work *work,
 static int set_file_basic_info(struct ksmbd_file *fp, char *buf,
 			       struct ksmbd_share_config *share)
 {
-	struct smb2_file_all_info *file_info;
+	struct smb2_file_basic_info *file_info;
 	struct iattr attrs;
 	struct timespec64 ctime;
 	struct file *filp;
@@ -5453,7 +5471,7 @@ static int set_file_basic_info(struct ksmbd_file *fp, char *buf,
 	if (!(fp->daccess & FILE_WRITE_ATTRIBUTES_LE))
 		return -EACCES;
 
-	file_info = (struct smb2_file_all_info *)buf;
+	file_info = (struct smb2_file_basic_info *)buf;
 	attrs.ia_valid = 0;
 	filp = fp->filp;
 	inode = file_inode(filp);
@@ -5619,7 +5637,8 @@ 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 smb2_file_rename_info *rename_info,
+			   int buf_len)
 {
 	struct user_namespace *user_ns;
 	struct ksmbd_file *parent_fp;
@@ -5632,6 +5651,10 @@ static int set_rename_info(struct ksmbd_work *work, struct ksmbd_file *fp,
 		return -EACCES;
 	}
 
+	if (buf_len < sizeof(struct smb2_file_rename_info) +
+			le32_to_cpu(rename_info->FileNameLength))
+		return -EINVAL;
+
 	user_ns = file_mnt_user_ns(fp->filp);
 	if (ksmbd_stream_fd(fp))
 		goto next;
@@ -5654,8 +5677,7 @@ static int set_rename_info(struct ksmbd_work *work, struct ksmbd_file *fp,
 		}
 	}
 next:
-	return smb2_rename(work, fp, user_ns,
-			   (struct smb2_file_rename_info *)buf,
+	return smb2_rename(work, fp, user_ns, rename_info,
 			   work->sess->conn->local_nls);
 }
 
@@ -5741,40 +5763,71 @@ static int set_file_mode_info(struct ksmbd_file *fp, char *buf)
  * TODO: need to implement an error handling for STATUS_INFO_LENGTH_MISMATCH
  */
 static int smb2_set_info_file(struct ksmbd_work *work, struct ksmbd_file *fp,
-			      int info_class, char *buf,
+			      struct smb2_set_info_req *req,
 			      struct ksmbd_share_config *share)
 {
-	switch (info_class) {
+	int buf_len = le32_to_cpu(req->BufferLength);
+
+	switch (req->FileInfoClass) {
 	case FILE_BASIC_INFORMATION:
-		return set_file_basic_info(fp, buf, share);
+	{
+		if (buf_len < sizeof(struct smb2_file_basic_info))
+			return -EINVAL;
 
+		return set_file_basic_info(fp, req->Buffer, share);
+	}
 	case FILE_ALLOCATION_INFORMATION:
-		return set_file_allocation_info(work, fp, buf);
+	{
+		if (buf_len < sizeof(struct smb2_file_alloc_info))
+			return -EINVAL;
 
+		return set_file_allocation_info(work, fp, req->Buffer);
+	}
 	case FILE_END_OF_FILE_INFORMATION:
-		return set_end_of_file_info(work, fp, buf);
+	{
+		if (buf_len < sizeof(struct smb2_file_eof_info))
+			return -EINVAL;
 
+		return set_end_of_file_info(work, fp, req->Buffer);
+	}
 	case FILE_RENAME_INFORMATION:
+	{
 		if (!test_tree_conn_flag(work->tcon, KSMBD_TREE_CONN_FLAG_WRITABLE)) {
 			ksmbd_debug(SMB,
 				    "User does not have write permission\n");
 			return -EACCES;
 		}
-		return set_rename_info(work, fp, buf);
 
+		if (buf_len < sizeof(struct smb2_file_rename_info))
+			return -EINVAL;
+
+		return set_rename_info(work, fp,
+				       (struct smb2_file_rename_info *)req->Buffer,
+				       buf_len);
+	}
 	case FILE_LINK_INFORMATION:
+	{
+		if (buf_len < sizeof(struct smb2_file_link_info))
+			return -EINVAL;
+
 		return smb2_create_link(work, work->tcon->share_conf,
-					(struct smb2_file_link_info *)buf, fp->filp,
+					(struct smb2_file_link_info *)req->Buffer,
+					buf_len, fp->filp,
 					work->sess->conn->local_nls);
-
+	}
 	case FILE_DISPOSITION_INFORMATION:
+	{
 		if (!test_tree_conn_flag(work->tcon, KSMBD_TREE_CONN_FLAG_WRITABLE)) {
 			ksmbd_debug(SMB,
 				    "User does not have write permission\n");
 			return -EACCES;
 		}
-		return set_file_disposition_info(fp, buf);
 
+		if (buf_len < sizeof(struct smb2_file_disposition_info))
+			return -EINVAL;
+
+		return set_file_disposition_info(fp, req->Buffer);
+	}
 	case FILE_FULL_EA_INFORMATION:
 	{
 		if (!(fp->daccess & FILE_WRITE_EA_LE)) {
@@ -5783,18 +5836,29 @@ static int smb2_set_info_file(struct ksmbd_work *work, struct ksmbd_file *fp,
 			return -EACCES;
 		}
 
-		return smb2_set_ea((struct smb2_ea_info *)buf,
-				   &fp->filp->f_path);
-	}
+		if (buf_len < sizeof(struct smb2_ea_info))
+			return -EINVAL;
 
+		return smb2_set_ea((struct smb2_ea_info *)req->Buffer,
+				   buf_len, &fp->filp->f_path);
+	}
 	case FILE_POSITION_INFORMATION:
-		return set_file_position_info(fp, buf);
+	{
+		if (buf_len < sizeof(struct smb2_file_pos_info))
+			return -EINVAL;
 
+		return set_file_position_info(fp, req->Buffer);
+	}
 	case FILE_MODE_INFORMATION:
-		return set_file_mode_info(fp, buf);
+	{
+		if (buf_len < sizeof(struct smb2_file_mode_info))
+			return -EINVAL;
+
+		return set_file_mode_info(fp, req->Buffer);
+	}
 	}
 
-	pr_err("Unimplemented Fileinfoclass :%d\n", info_class);
+	pr_err("Unimplemented Fileinfoclass :%d\n", req->FileInfoClass);
 	return -EOPNOTSUPP;
 }
 
@@ -5855,8 +5919,7 @@ int smb2_set_info(struct ksmbd_work *work)
 	switch (req->InfoType) {
 	case SMB2_O_INFO_FILE:
 		ksmbd_debug(SMB, "GOT SMB2_O_INFO_FILE\n");
-		rc = smb2_set_info_file(work, fp, req->FileInfoClass,
-					req->Buffer, work->tcon->share_conf);
+		rc = smb2_set_info_file(work, fp, req, work->tcon->share_conf);
 		break;
 	case SMB2_O_INFO_SECURITY:
 		ksmbd_debug(SMB, "GOT SMB2_O_INFO_SECURITY\n");
diff --git a/fs/ksmbd/smb2pdu.h b/fs/ksmbd/smb2pdu.h
index bcec845b03f3..261825d06391 100644
--- a/fs/ksmbd/smb2pdu.h
+++ b/fs/ksmbd/smb2pdu.h
@@ -1464,6 +1464,15 @@ struct smb2_file_all_info { /* data block encoding of response to level 18 */
 	char   FileName[1];
 } __packed; /* level 18 Query */
 
+struct smb2_file_basic_info { /* data block encoding of response to level 18 */
+	__le64 CreationTime;	/* Beginning of FILE_BASIC_INFO equivalent */
+	__le64 LastAccessTime;
+	__le64 LastWriteTime;
+	__le64 ChangeTime;
+	__le32 Attributes;
+	__u32  Pad1;		/* End of FILE_BASIC_INFO_INFO equivalent */
+} __packed;
+
 struct smb2_file_alt_name_info {
 	__le32 FileNameLength;
 	char FileName[0];
-- 
2.25.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/4] ksmbd: add validation in smb2_ioctl
  2021-09-18  9:45 [PATCH 1/4] ksmbd: add request buffer validation in smb2_set_info Namjae Jeon
@ 2021-09-18  9:45 ` Namjae Jeon
  2021-09-18 18:51   ` kernel test robot
  2021-09-18  9:45 ` [PATCH 3/4] ksmbd: add validatioin for FILE_FULL_EA_INFORMATION of smb2_get_info Namjae Jeon
  2021-09-18  9:45 ` [PATCH 4/4] ksmbd: add buffer validation for SMB2_CREATE_CONTEXT Namjae Jeon
  2 siblings, 1 reply; 9+ messages in thread
From: Namjae Jeon @ 2021-09-18  9:45 UTC (permalink / raw)
  To: linux-cifs; +Cc: Namjae Jeon, Ronnie Sahlberg, Ralph Böhme, Steve French

Add validation for request/response buffer size check in smb2_ioctl.

Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Ralph Böhme <slow@samba.org>
Cc: Steve French <smfrench@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 fs/ksmbd/smb2pdu.c | 54 +++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 49 insertions(+), 5 deletions(-)

diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index 7763f69e1ae8..e589e8cc389f 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -7038,6 +7038,8 @@ static int fsctl_copychunk(struct ksmbd_work *work, struct smb2_ioctl_req *req,
 
 	chunks = (struct srv_copychunk *)&ci_req->Chunks[0];
 	chunk_count = le32_to_cpu(ci_req->ChunkCount);
+	if (chunk_count == 0)
+		goto out;
 	total_size_written = 0;
 
 	/* verify the SRV_COPYCHUNK_COPY packet */
@@ -7142,7 +7144,8 @@ static __be32 idev_ipv4_address(struct in_device *idev)
 
 static int fsctl_query_iface_info_ioctl(struct ksmbd_conn *conn,
 					struct smb2_ioctl_req *req,
-					struct smb2_ioctl_rsp *rsp)
+					struct smb2_ioctl_rsp *rsp,
+					int out_buf_len)
 {
 	struct network_interface_info_ioctl_rsp *nii_rsp = NULL;
 	int nbytes = 0;
@@ -7225,6 +7228,8 @@ static int fsctl_query_iface_info_ioctl(struct ksmbd_conn *conn,
 			sockaddr_storage->addr6.ScopeId = 0;
 		}
 
+		if (out_buf_len < sizeof(struct network_interface_info_ioctl_rsp))
+			break;
 		nbytes += sizeof(struct network_interface_info_ioctl_rsp);
 	}
 	rtnl_unlock();
@@ -7245,11 +7250,16 @@ static int fsctl_query_iface_info_ioctl(struct ksmbd_conn *conn,
 
 static int fsctl_validate_negotiate_info(struct ksmbd_conn *conn,
 					 struct validate_negotiate_info_req *neg_req,
-					 struct validate_negotiate_info_rsp *neg_rsp)
+					 struct validate_negotiate_info_rsp *neg_rsp,
+					 int in_buf_len)
 {
 	int ret = 0;
 	int dialect;
 
+	if (in_buf_len < sizeof(struct validate_negotiate_info_req) +
+			le16_to_cpu(neg_req->DialectCount) * sizeof(__le16))
+		return -EINVAL;
+
 	dialect = ksmbd_lookup_dialect_by_id(neg_req->Dialects,
 					     neg_req->DialectCount);
 	if (dialect == BAD_PROT_ID || dialect != conn->dialect) {
@@ -7425,7 +7435,7 @@ int smb2_ioctl(struct ksmbd_work *work)
 	struct smb2_ioctl_req *req;
 	struct smb2_ioctl_rsp *rsp, *rsp_org;
 	int cnt_code, nbytes = 0;
-	int out_buf_len;
+	int out_buf_len, in_buf_len;
 	u64 id = KSMBD_NO_FID;
 	struct ksmbd_conn *conn = work->conn;
 	int ret = 0;
@@ -7455,6 +7465,7 @@ int smb2_ioctl(struct ksmbd_work *work)
 	cnt_code = le32_to_cpu(req->CntCode);
 	out_buf_len = le32_to_cpu(req->MaxOutputResponse);
 	out_buf_len = min(KSMBD_IPC_MAX_PAYLOAD, out_buf_len);
+	in_buf_len = le32_to_cpu(req->InputCount);
 
 	switch (cnt_code) {
 	case FSCTL_DFS_GET_REFERRALS:
@@ -7490,9 +7501,16 @@ int smb2_ioctl(struct ksmbd_work *work)
 			goto out;
 		}
 
+		if (in_buf_len < sizeof(struct validate_negotiate_info_req))
+			return -EINVAL;
+
+		if (out_buf_len < sizeof(struct validate_negotiate_info_rsp))
+			return -EINVAL;
+
 		ret = fsctl_validate_negotiate_info(conn,
 			(struct validate_negotiate_info_req *)&req->Buffer[0],
-			(struct validate_negotiate_info_rsp *)&rsp->Buffer[0]);
+			(struct validate_negotiate_info_rsp *)&rsp->Buffer[0],
+			in_buf_len);
 		if (ret < 0)
 			goto out;
 
@@ -7501,7 +7519,8 @@ int smb2_ioctl(struct ksmbd_work *work)
 		rsp->VolatileFileId = cpu_to_le64(SMB2_NO_FID);
 		break;
 	case FSCTL_QUERY_NETWORK_INTERFACE_INFO:
-		nbytes = fsctl_query_iface_info_ioctl(conn, req, rsp);
+		nbytes = fsctl_query_iface_info_ioctl(conn, req, rsp,
+						      out_buf_len);
 		if (nbytes < 0)
 			goto out;
 		break;
@@ -7528,6 +7547,11 @@ int smb2_ioctl(struct ksmbd_work *work)
 			goto out;
 		}
 
+		if (in_buf_len < sizeof(struct copychunk_ioctl_req)) {
+			ret = -EINVAL;
+			goto out;
+		}
+
 		if (out_buf_len < sizeof(struct copychunk_ioctl_rsp)) {
 			ret = -EINVAL;
 			goto out;
@@ -7537,6 +7561,11 @@ int smb2_ioctl(struct ksmbd_work *work)
 		fsctl_copychunk(work, req, rsp);
 		break;
 	case FSCTL_SET_SPARSE:
+		if (in_buf_len < sizeof(struct file_sparse)) {
+			ret = -EINVAL;
+			goto out;
+		}
+
 		ret = fsctl_set_sparse(work, id,
 				       (struct file_sparse *)&req->Buffer[0]);
 		if (ret < 0)
@@ -7555,6 +7584,11 @@ int smb2_ioctl(struct ksmbd_work *work)
 			goto out;
 		}
 
+		if (in_buf_len < sizeof(struct file_zero_data_information)) {
+			ret = -EINVAL;
+			goto out;
+		}
+
 		zero_data =
 			(struct file_zero_data_information *)&req->Buffer[0];
 
@@ -7574,6 +7608,11 @@ int smb2_ioctl(struct ksmbd_work *work)
 		break;
 	}
 	case FSCTL_QUERY_ALLOCATED_RANGES:
+		if (in_buf_len < sizeof(struct file_allocated_range_buffer)) {
+			ret = -EINVAL;
+			goto out;
+		}
+
 		ret = fsctl_query_allocated_ranges(work, id,
 			(struct file_allocated_range_buffer *)&req->Buffer[0],
 			(struct file_allocated_range_buffer *)&rsp->Buffer[0],
@@ -7614,6 +7653,11 @@ int smb2_ioctl(struct ksmbd_work *work)
 		struct duplicate_extents_to_file *dup_ext;
 		loff_t src_off, dst_off, length, cloned;
 
+		if (in_buf_len < sizeof(struct duplicate_extents_to_file)) {
+			ret = -EINVAL;
+			goto out;
+		}
+
 		dup_ext = (struct duplicate_extents_to_file *)&req->Buffer[0];
 
 		fp_in = ksmbd_lookup_fd_slow(work, dup_ext->VolatileFileHandle,
-- 
2.25.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/4] ksmbd: add validatioin for FILE_FULL_EA_INFORMATION of smb2_get_info
  2021-09-18  9:45 [PATCH 1/4] ksmbd: add request buffer validation in smb2_set_info Namjae Jeon
  2021-09-18  9:45 ` [PATCH 2/4] ksmbd: add validation in smb2_ioctl Namjae Jeon
@ 2021-09-18  9:45 ` Namjae Jeon
  2021-09-18 15:55   ` Steve French
  2021-09-18  9:45 ` [PATCH 4/4] ksmbd: add buffer validation for SMB2_CREATE_CONTEXT Namjae Jeon
  2 siblings, 1 reply; 9+ messages in thread
From: Namjae Jeon @ 2021-09-18  9:45 UTC (permalink / raw)
  To: linux-cifs; +Cc: Namjae Jeon, Ronnie Sahlberg, Ralph Böhme, Steve French

Add validation to check whether req->InputBufferLength is smaller than
smb2_ea_info_req structure size.

Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Ralph Böhme <slow@samba.org>
Cc: Steve French <smfrench@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 fs/ksmbd/smb2pdu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index e589e8cc389f..e92af212583e 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -4059,6 +4059,10 @@ static int smb2_get_ea(struct ksmbd_work *work, struct ksmbd_file *fp,
 	path = &fp->filp->f_path;
 	/* single EA entry is requested with given user.* name */
 	if (req->InputBufferLength) {
+		if (le32_to_cpu(req->InputBufferLength) <
+		    sizeof(struct smb2_ea_info_req))
+			return -EINVAL;
+
 		ea_req = (struct smb2_ea_info_req *)req->Buffer;
 	} else {
 		/* need to send all EAs, if no specific EA is requested*/
-- 
2.25.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4/4] ksmbd: add buffer validation for SMB2_CREATE_CONTEXT
  2021-09-18  9:45 [PATCH 1/4] ksmbd: add request buffer validation in smb2_set_info Namjae Jeon
  2021-09-18  9:45 ` [PATCH 2/4] ksmbd: add validation in smb2_ioctl Namjae Jeon
  2021-09-18  9:45 ` [PATCH 3/4] ksmbd: add validatioin for FILE_FULL_EA_INFORMATION of smb2_get_info Namjae Jeon
@ 2021-09-18  9:45 ` Namjae Jeon
  2 siblings, 0 replies; 9+ messages in thread
From: Namjae Jeon @ 2021-09-18  9:45 UTC (permalink / raw)
  To: linux-cifs
  Cc: Hyunchul Lee, Ronnie Sahlberg, Ralph Böhme, Steve French,
	Namjae Jeon

From: Hyunchul Lee <hyc.lee@gmail.com>

Add buffer validation for SMB2_CREATE_CONTEXT.

Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Ralph Böhme <slow@samba.org>
Cc: Steve French <smfrench@gmail.com>
Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 fs/ksmbd/oplock.c  | 35 +++++++++++++++++++++++++----------
 fs/ksmbd/smb2pdu.c | 25 ++++++++++++++++++++++++-
 fs/ksmbd/smbacl.c  |  9 ++++++++-
 3 files changed, 57 insertions(+), 12 deletions(-)

diff --git a/fs/ksmbd/oplock.c b/fs/ksmbd/oplock.c
index 16b6236d1bd2..3fd2713f2282 100644
--- a/fs/ksmbd/oplock.c
+++ b/fs/ksmbd/oplock.c
@@ -1451,26 +1451,41 @@ struct lease_ctx_info *parse_lease_state(void *open_req)
  */
 struct create_context *smb2_find_context_vals(void *open_req, const char *tag)
 {
-	char *data_offset;
+	struct smb2_create_req *req = (struct smb2_create_req *)open_req;
 	struct create_context *cc;
-	unsigned int next = 0;
+	char *data_offset, *data_end;
 	char *name;
-	struct smb2_create_req *req = (struct smb2_create_req *)open_req;
+	unsigned int next = 0;
+	unsigned int name_off, name_len, value_off, value_len;
 
 	data_offset = (char *)req + 4 + le32_to_cpu(req->CreateContextsOffset);
+	data_end = data_offset + le32_to_cpu(req->CreateContextsLength);
 	cc = (struct create_context *)data_offset;
 	do {
-		int val;
-
 		cc = (struct create_context *)((char *)cc + next);
-		name = le16_to_cpu(cc->NameOffset) + (char *)cc;
-		val = le16_to_cpu(cc->NameLength);
-		if (val < 4)
+		if ((char *)cc + offsetof(struct create_context, Buffer) >
+		    data_end)
 			return ERR_PTR(-EINVAL);
 
-		if (memcmp(name, tag, val) == 0)
-			return cc;
 		next = le32_to_cpu(cc->Next);
+		name_off = le16_to_cpu(cc->NameOffset);
+		name_len = le16_to_cpu(cc->NameLength);
+		value_off = le16_to_cpu(cc->DataOffset);
+		value_len = le32_to_cpu(cc->DataLength);
+
+		if ((char *)cc + name_off + name_len > data_end ||
+		    (value_len && (char *)cc + value_off + value_len > data_end))
+			return ERR_PTR(-EINVAL);
+		else if (next && (next < name_off + name_len ||
+			 (value_len && next < value_off + value_len)))
+			return ERR_PTR(-EINVAL);
+
+		name = (char *)cc + name_off;
+		if (name_len < 4)
+			return ERR_PTR(-EINVAL);
+
+		if (memcmp(name, tag, name_len) == 0)
+			return cc;
 	} while (next != 0);
 
 	return NULL;
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index e92af212583e..49a1ca75f427 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -2393,6 +2393,10 @@ static int smb2_create_sd_buffer(struct ksmbd_work *work,
 	ksmbd_debug(SMB,
 		    "Set ACLs using SMB2_CREATE_SD_BUFFER context\n");
 	sd_buf = (struct create_sd_buf_req *)context;
+	if (le16_to_cpu(context->DataOffset) +
+	    le32_to_cpu(context->DataLength) <
+	    sizeof(struct create_sd_buf_req))
+		return -EINVAL;
 	return set_info_sec(work->conn, work->tcon, path, &sd_buf->ntsd,
 			    le32_to_cpu(sd_buf->ccontext.DataLength), true);
 }
@@ -2593,6 +2597,12 @@ int smb2_open(struct ksmbd_work *work)
 			goto err_out1;
 		} else if (context) {
 			ea_buf = (struct create_ea_buf_req *)context;
+			if (le16_to_cpu(context->DataOffset) +
+			    le32_to_cpu(context->DataLength) <
+			    sizeof(struct create_ea_buf_req)) {
+				rc = -EINVAL;
+				goto err_out1;
+			}
 			if (req->CreateOptions & FILE_NO_EA_KNOWLEDGE_LE) {
 				rsp->hdr.Status = STATUS_ACCESS_DENIED;
 				rc = -EACCES;
@@ -2631,6 +2641,12 @@ int smb2_open(struct ksmbd_work *work)
 			} else if (context) {
 				struct create_posix *posix =
 					(struct create_posix *)context;
+				if (le16_to_cpu(context->DataOffset) +
+				    le32_to_cpu(context->DataLength) <
+				    sizeof(struct create_posix)) {
+					rc = -EINVAL;
+					goto err_out1;
+				}
 				ksmbd_debug(SMB, "get posix context\n");
 
 				posix_mode = le32_to_cpu(posix->Mode);
@@ -3037,9 +3053,16 @@ int smb2_open(struct ksmbd_work *work)
 			rc = PTR_ERR(az_req);
 			goto err_out;
 		} else if (az_req) {
-			loff_t alloc_size = le64_to_cpu(az_req->AllocationSize);
+			loff_t alloc_size;
 			int err;
 
+			if (le16_to_cpu(az_req->ccontext.DataOffset) +
+			    le32_to_cpu(az_req->ccontext.DataLength) <
+			    sizeof(struct create_alloc_size_req)) {
+				rc = -EINVAL;
+				goto err_out;
+			}
+			alloc_size = le64_to_cpu(az_req->AllocationSize);
 			ksmbd_debug(SMB,
 				    "request smb2 create allocate size : %llu\n",
 				    alloc_size);
diff --git a/fs/ksmbd/smbacl.c b/fs/ksmbd/smbacl.c
index 0a95cdec8c80..f67567e1e178 100644
--- a/fs/ksmbd/smbacl.c
+++ b/fs/ksmbd/smbacl.c
@@ -392,7 +392,7 @@ static void parse_dacl(struct user_namespace *user_ns,
 		return;
 
 	/* validate that we do not go past end of acl */
-	if (end_of_acl <= (char *)pdacl ||
+	if (end_of_acl < (char *)pdacl + sizeof(struct smb_acl) ||
 	    end_of_acl < (char *)pdacl + le16_to_cpu(pdacl->size)) {
 		pr_err("ACL too small to parse DACL\n");
 		return;
@@ -434,6 +434,10 @@ static void parse_dacl(struct user_namespace *user_ns,
 		ppace[i] = (struct smb_ace *)(acl_base + acl_size);
 		acl_base = (char *)ppace[i];
 		acl_size = le16_to_cpu(ppace[i]->size);
+
+		if (acl_base + acl_size > end_of_acl)
+			break;
+
 		ppace[i]->access_req =
 			smb_map_generic_desired_access(ppace[i]->access_req);
 
@@ -807,6 +811,9 @@ int parse_sec_desc(struct user_namespace *user_ns, struct smb_ntsd *pntsd,
 	if (!pntsd)
 		return -EIO;
 
+	if (acl_len < sizeof(struct smb_ntsd))
+		return -EINVAL;
+
 	owner_sid_ptr = (struct smb_sid *)((char *)pntsd +
 			le32_to_cpu(pntsd->osidoffset));
 	group_sid_ptr = (struct smb_sid *)((char *)pntsd +
-- 
2.25.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/4] ksmbd: add validatioin for FILE_FULL_EA_INFORMATION of smb2_get_info
  2021-09-18  9:45 ` [PATCH 3/4] ksmbd: add validatioin for FILE_FULL_EA_INFORMATION of smb2_get_info Namjae Jeon
@ 2021-09-18 15:55   ` Steve French
  2021-09-18 17:57     ` Steve French
  2021-09-18 18:10     ` Ralph Boehme
  0 siblings, 2 replies; 9+ messages in thread
From: Steve French @ 2021-09-18 15:55 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: CIFS, Ronnie Sahlberg, Ralph Böhme

Merged into cifsd-for-next (smbd-for-next) after fixing typo in title.
The other three look promising but want to look in more detail at
those unless others have review feedback on those - those patches
include some potentially very important checks.

On Sat, Sep 18, 2021 at 4:45 AM Namjae Jeon <linkinjeon@kernel.org> wrote:
>
> Add validation to check whether req->InputBufferLength is smaller than
> smb2_ea_info_req structure size.
>
> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> Cc: Ralph Böhme <slow@samba.org>
> Cc: Steve French <smfrench@gmail.com>
> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
> ---
>  fs/ksmbd/smb2pdu.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
> index e589e8cc389f..e92af212583e 100644
> --- a/fs/ksmbd/smb2pdu.c
> +++ b/fs/ksmbd/smb2pdu.c
> @@ -4059,6 +4059,10 @@ static int smb2_get_ea(struct ksmbd_work *work, struct ksmbd_file *fp,
>         path = &fp->filp->f_path;
>         /* single EA entry is requested with given user.* name */
>         if (req->InputBufferLength) {
> +               if (le32_to_cpu(req->InputBufferLength) <
> +                   sizeof(struct smb2_ea_info_req))
> +                       return -EINVAL;
> +
>                 ea_req = (struct smb2_ea_info_req *)req->Buffer;
>         } else {
>                 /* need to send all EAs, if no specific EA is requested*/
> --
> 2.25.1
>


-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/4] ksmbd: add validatioin for FILE_FULL_EA_INFORMATION of smb2_get_info
  2021-09-18 15:55   ` Steve French
@ 2021-09-18 17:57     ` Steve French
  2021-09-18 18:10     ` Ralph Boehme
  1 sibling, 0 replies; 9+ messages in thread
From: Steve French @ 2021-09-18 17:57 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: CIFS, Ronnie Sahlberg, Ralph Böhme

Regression tests with the three in linux-next passed ...
http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/8/builds/67

On Sat, Sep 18, 2021 at 10:55 AM Steve French <smfrench@gmail.com> wrote:
>
> Merged into cifsd-for-next (smbd-for-next) after fixing typo in title.
> The other three look promising but want to look in more detail at
> those unless others have review feedback on those - those patches
> include some potentially very important checks.
>
> On Sat, Sep 18, 2021 at 4:45 AM Namjae Jeon <linkinjeon@kernel.org> wrote:
> >
> > Add validation to check whether req->InputBufferLength is smaller than
> > smb2_ea_info_req structure size.
> >
> > Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> > Cc: Ralph Böhme <slow@samba.org>
> > Cc: Steve French <smfrench@gmail.com>
> > Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
> > ---
> >  fs/ksmbd/smb2pdu.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
> > index e589e8cc389f..e92af212583e 100644
> > --- a/fs/ksmbd/smb2pdu.c
> > +++ b/fs/ksmbd/smb2pdu.c
> > @@ -4059,6 +4059,10 @@ static int smb2_get_ea(struct ksmbd_work *work, struct ksmbd_file *fp,
> >         path = &fp->filp->f_path;
> >         /* single EA entry is requested with given user.* name */
> >         if (req->InputBufferLength) {
> > +               if (le32_to_cpu(req->InputBufferLength) <
> > +                   sizeof(struct smb2_ea_info_req))
> > +                       return -EINVAL;
> > +
> >                 ea_req = (struct smb2_ea_info_req *)req->Buffer;
> >         } else {
> >                 /* need to send all EAs, if no specific EA is requested*/
> > --
> > 2.25.1
> >
>
>
> --
> Thanks,
>
> Steve



-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/4] ksmbd: add validatioin for FILE_FULL_EA_INFORMATION of smb2_get_info
  2021-09-18 15:55   ` Steve French
  2021-09-18 17:57     ` Steve French
@ 2021-09-18 18:10     ` Ralph Boehme
  1 sibling, 0 replies; 9+ messages in thread
From: Ralph Boehme @ 2021-09-18 18:10 UTC (permalink / raw)
  To: Steve French, Namjae Jeon; +Cc: CIFS, Ronnie Sahlberg


[-- Attachment #1.1: Type: text/plain, Size: 523 bytes --]

Am 18.09.21 um 17:55 schrieb Steve French:
> Merged into cifsd-for-next (smbd-for-next) after fixing typo in title.
> The other three look promising but want to look in more detail at
> those unless others have review feedback on those - those patches
> include some potentially very important checks.

I'm carefully looking at all four, it just takes a bit of time.

Cheers!
-slow

-- 
Ralph Boehme, Samba Team                 https://samba.org/
SerNet Samba Team Lead      https://sernet.de/en/team-samba


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/4] ksmbd: add validation in smb2_ioctl
  2021-09-18  9:45 ` [PATCH 2/4] ksmbd: add validation in smb2_ioctl Namjae Jeon
@ 2021-09-18 18:51   ` kernel test robot
  2021-09-18 21:43     ` Namjae Jeon
  0 siblings, 1 reply; 9+ messages in thread
From: kernel test robot @ 2021-09-18 18:51 UTC (permalink / raw)
  To: Namjae Jeon, linux-cifs
  Cc: llvm, kbuild-all, Namjae Jeon, Ronnie Sahlberg, Ralph Böhme,
	Steve French

[-- Attachment #1: Type: text/plain, Size: 6667 bytes --]

Hi Namjae,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.15-rc1 next-20210917]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Namjae-Jeon/ksmbd-add-request-buffer-validation-in-smb2_set_info/20210918-174717
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 4357f03d6611753936e4d52fc251b54a6afb1b54
config: hexagon-randconfig-r022-20210918 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project c8b3d7d6d6de37af68b2f379d0e37304f78e115f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/57e7ede2bf2d38cb0f368f2fc54d646168b3d119
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Namjae-Jeon/ksmbd-add-request-buffer-validation-in-smb2_set_info/20210918-174717
        git checkout 57e7ede2bf2d38cb0f368f2fc54d646168b3d119
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=hexagon 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> fs/ksmbd/smb2pdu.c:7037:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (chunk_count == 0)
               ^~~~~~~~~~~~~~~~
   fs/ksmbd/smb2pdu.c:7120:9: note: uninitialized use occurs here
           return ret;
                  ^~~
   fs/ksmbd/smb2pdu.c:7037:2: note: remove the 'if' if its condition is always false
           if (chunk_count == 0)
           ^~~~~~~~~~~~~~~~~~~~~
   fs/ksmbd/smb2pdu.c:7020:9: note: initialize the variable 'ret' to silence this warning
           int ret, cnt_code;
                  ^
                   = 0
   1 warning generated.


vim +7037 fs/ksmbd/smb2pdu.c

  7009	
  7010	static int fsctl_copychunk(struct ksmbd_work *work, struct smb2_ioctl_req *req,
  7011				   struct smb2_ioctl_rsp *rsp)
  7012	{
  7013		struct copychunk_ioctl_req *ci_req;
  7014		struct copychunk_ioctl_rsp *ci_rsp;
  7015		struct ksmbd_file *src_fp = NULL, *dst_fp = NULL;
  7016		struct srv_copychunk *chunks;
  7017		unsigned int i, chunk_count, chunk_count_written = 0;
  7018		unsigned int chunk_size_written = 0;
  7019		loff_t total_size_written = 0;
  7020		int ret, cnt_code;
  7021	
  7022		cnt_code = le32_to_cpu(req->CntCode);
  7023		ci_req = (struct copychunk_ioctl_req *)&req->Buffer[0];
  7024		ci_rsp = (struct copychunk_ioctl_rsp *)&rsp->Buffer[0];
  7025	
  7026		rsp->VolatileFileId = req->VolatileFileId;
  7027		rsp->PersistentFileId = req->PersistentFileId;
  7028		ci_rsp->ChunksWritten =
  7029			cpu_to_le32(ksmbd_server_side_copy_max_chunk_count());
  7030		ci_rsp->ChunkBytesWritten =
  7031			cpu_to_le32(ksmbd_server_side_copy_max_chunk_size());
  7032		ci_rsp->TotalBytesWritten =
  7033			cpu_to_le32(ksmbd_server_side_copy_max_total_size());
  7034	
  7035		chunks = (struct srv_copychunk *)&ci_req->Chunks[0];
  7036		chunk_count = le32_to_cpu(ci_req->ChunkCount);
> 7037		if (chunk_count == 0)
  7038			goto out;
  7039		total_size_written = 0;
  7040	
  7041		/* verify the SRV_COPYCHUNK_COPY packet */
  7042		if (chunk_count > ksmbd_server_side_copy_max_chunk_count() ||
  7043		    le32_to_cpu(req->InputCount) <
  7044		     offsetof(struct copychunk_ioctl_req, Chunks) +
  7045		     chunk_count * sizeof(struct srv_copychunk)) {
  7046			rsp->hdr.Status = STATUS_INVALID_PARAMETER;
  7047			return -EINVAL;
  7048		}
  7049	
  7050		for (i = 0; i < chunk_count; i++) {
  7051			if (le32_to_cpu(chunks[i].Length) == 0 ||
  7052			    le32_to_cpu(chunks[i].Length) > ksmbd_server_side_copy_max_chunk_size())
  7053				break;
  7054			total_size_written += le32_to_cpu(chunks[i].Length);
  7055		}
  7056	
  7057		if (i < chunk_count ||
  7058		    total_size_written > ksmbd_server_side_copy_max_total_size()) {
  7059			rsp->hdr.Status = STATUS_INVALID_PARAMETER;
  7060			return -EINVAL;
  7061		}
  7062	
  7063		src_fp = ksmbd_lookup_foreign_fd(work,
  7064						 le64_to_cpu(ci_req->ResumeKey[0]));
  7065		dst_fp = ksmbd_lookup_fd_slow(work,
  7066					      le64_to_cpu(req->VolatileFileId),
  7067					      le64_to_cpu(req->PersistentFileId));
  7068		ret = -EINVAL;
  7069		if (!src_fp ||
  7070		    src_fp->persistent_id != le64_to_cpu(ci_req->ResumeKey[1])) {
  7071			rsp->hdr.Status = STATUS_OBJECT_NAME_NOT_FOUND;
  7072			goto out;
  7073		}
  7074	
  7075		if (!dst_fp) {
  7076			rsp->hdr.Status = STATUS_FILE_CLOSED;
  7077			goto out;
  7078		}
  7079	
  7080		/*
  7081		 * FILE_READ_DATA should only be included in
  7082		 * the FSCTL_COPYCHUNK case
  7083		 */
  7084		if (cnt_code == FSCTL_COPYCHUNK &&
  7085		    !(dst_fp->daccess & (FILE_READ_DATA_LE | FILE_GENERIC_READ_LE))) {
  7086			rsp->hdr.Status = STATUS_ACCESS_DENIED;
  7087			goto out;
  7088		}
  7089	
  7090		ret = ksmbd_vfs_copy_file_ranges(work, src_fp, dst_fp,
  7091						 chunks, chunk_count,
  7092						 &chunk_count_written,
  7093						 &chunk_size_written,
  7094						 &total_size_written);
  7095		if (ret < 0) {
  7096			if (ret == -EACCES)
  7097				rsp->hdr.Status = STATUS_ACCESS_DENIED;
  7098			if (ret == -EAGAIN)
  7099				rsp->hdr.Status = STATUS_FILE_LOCK_CONFLICT;
  7100			else if (ret == -EBADF)
  7101				rsp->hdr.Status = STATUS_INVALID_HANDLE;
  7102			else if (ret == -EFBIG || ret == -ENOSPC)
  7103				rsp->hdr.Status = STATUS_DISK_FULL;
  7104			else if (ret == -EINVAL)
  7105				rsp->hdr.Status = STATUS_INVALID_PARAMETER;
  7106			else if (ret == -EISDIR)
  7107				rsp->hdr.Status = STATUS_FILE_IS_A_DIRECTORY;
  7108			else if (ret == -E2BIG)
  7109				rsp->hdr.Status = STATUS_INVALID_VIEW_SIZE;
  7110			else
  7111				rsp->hdr.Status = STATUS_UNEXPECTED_IO_ERROR;
  7112		}
  7113	
  7114		ci_rsp->ChunksWritten = cpu_to_le32(chunk_count_written);
  7115		ci_rsp->ChunkBytesWritten = cpu_to_le32(chunk_size_written);
  7116		ci_rsp->TotalBytesWritten = cpu_to_le32(total_size_written);
  7117	out:
  7118		ksmbd_fd_put(work, src_fp);
  7119		ksmbd_fd_put(work, dst_fp);
  7120		return ret;
  7121	}
  7122	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25522 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/4] ksmbd: add validation in smb2_ioctl
  2021-09-18 18:51   ` kernel test robot
@ 2021-09-18 21:43     ` Namjae Jeon
  0 siblings, 0 replies; 9+ messages in thread
From: Namjae Jeon @ 2021-09-18 21:43 UTC (permalink / raw)
  To: kernel test robot
  Cc: linux-cifs, llvm, kbuild-all, Ronnie Sahlberg, Ralph Böhme,
	Steve French

2021-09-19 3:51 GMT+09:00, kernel test robot <lkp@intel.com>:
> Hi Namjae,
Hi,

I will fix it, Thanks for your report!
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on linus/master]
> [also build test WARNING on v5.15-rc1 next-20210917]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url:
> https://github.com/0day-ci/linux/commits/Namjae-Jeon/ksmbd-add-request-buffer-validation-in-smb2_set_info/20210918-174717
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> 4357f03d6611753936e4d52fc251b54a6afb1b54
> config: hexagon-randconfig-r022-20210918 (attached as .config)
> compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project
> c8b3d7d6d6de37af68b2f379d0e37304f78e115f)
> reproduce (this is a W=1 build):
>         wget
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
> ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         #
> https://github.com/0day-ci/linux/commit/57e7ede2bf2d38cb0f368f2fc54d646168b3d119
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review
> Namjae-Jeon/ksmbd-add-request-buffer-validation-in-smb2_set_info/20210918-174717
>         git checkout 57e7ede2bf2d38cb0f368f2fc54d646168b3d119
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1
> ARCH=hexagon
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
>>> fs/ksmbd/smb2pdu.c:7037:6: warning: variable 'ret' is used uninitialized
>>> whenever 'if' condition is true [-Wsometimes-uninitialized]
>            if (chunk_count == 0)
>                ^~~~~~~~~~~~~~~~
>    fs/ksmbd/smb2pdu.c:7120:9: note: uninitialized use occurs here
>            return ret;
>                   ^~~
>    fs/ksmbd/smb2pdu.c:7037:2: note: remove the 'if' if its condition is
> always false
>            if (chunk_count == 0)
>            ^~~~~~~~~~~~~~~~~~~~~
>    fs/ksmbd/smb2pdu.c:7020:9: note: initialize the variable 'ret' to silence
> this warning
>            int ret, cnt_code;
>                   ^
>                    = 0
>    1 warning generated.
>
>
> vim +7037 fs/ksmbd/smb2pdu.c
>
>   7009	
>   7010	static int fsctl_copychunk(struct ksmbd_work *work, struct
> smb2_ioctl_req *req,
>   7011				   struct smb2_ioctl_rsp *rsp)
>   7012	{
>   7013		struct copychunk_ioctl_req *ci_req;
>   7014		struct copychunk_ioctl_rsp *ci_rsp;
>   7015		struct ksmbd_file *src_fp = NULL, *dst_fp = NULL;
>   7016		struct srv_copychunk *chunks;
>   7017		unsigned int i, chunk_count, chunk_count_written = 0;
>   7018		unsigned int chunk_size_written = 0;
>   7019		loff_t total_size_written = 0;
>   7020		int ret, cnt_code;
>   7021	
>   7022		cnt_code = le32_to_cpu(req->CntCode);
>   7023		ci_req = (struct copychunk_ioctl_req *)&req->Buffer[0];
>   7024		ci_rsp = (struct copychunk_ioctl_rsp *)&rsp->Buffer[0];
>   7025	
>   7026		rsp->VolatileFileId = req->VolatileFileId;
>   7027		rsp->PersistentFileId = req->PersistentFileId;
>   7028		ci_rsp->ChunksWritten =
>   7029			cpu_to_le32(ksmbd_server_side_copy_max_chunk_count());
>   7030		ci_rsp->ChunkBytesWritten =
>   7031			cpu_to_le32(ksmbd_server_side_copy_max_chunk_size());
>   7032		ci_rsp->TotalBytesWritten =
>   7033			cpu_to_le32(ksmbd_server_side_copy_max_total_size());
>   7034	
>   7035		chunks = (struct srv_copychunk *)&ci_req->Chunks[0];
>   7036		chunk_count = le32_to_cpu(ci_req->ChunkCount);
>> 7037		if (chunk_count == 0)
>   7038			goto out;
>   7039		total_size_written = 0;
>   7040	
>   7041		/* verify the SRV_COPYCHUNK_COPY packet */
>   7042		if (chunk_count > ksmbd_server_side_copy_max_chunk_count() ||
>   7043		    le32_to_cpu(req->InputCount) <
>   7044		     offsetof(struct copychunk_ioctl_req, Chunks) +
>   7045		     chunk_count * sizeof(struct srv_copychunk)) {
>   7046			rsp->hdr.Status = STATUS_INVALID_PARAMETER;
>   7047			return -EINVAL;
>   7048		}
>   7049	
>   7050		for (i = 0; i < chunk_count; i++) {
>   7051			if (le32_to_cpu(chunks[i].Length) == 0 ||
>   7052			    le32_to_cpu(chunks[i].Length) >
> ksmbd_server_side_copy_max_chunk_size())
>   7053				break;
>   7054			total_size_written += le32_to_cpu(chunks[i].Length);
>   7055		}
>   7056	
>   7057		if (i < chunk_count ||
>   7058		    total_size_written > ksmbd_server_side_copy_max_total_size()) {
>   7059			rsp->hdr.Status = STATUS_INVALID_PARAMETER;
>   7060			return -EINVAL;
>   7061		}
>   7062	
>   7063		src_fp = ksmbd_lookup_foreign_fd(work,
>   7064						 le64_to_cpu(ci_req->ResumeKey[0]));
>   7065		dst_fp = ksmbd_lookup_fd_slow(work,
>   7066					      le64_to_cpu(req->VolatileFileId),
>   7067					      le64_to_cpu(req->PersistentFileId));
>   7068		ret = -EINVAL;
>   7069		if (!src_fp ||
>   7070		    src_fp->persistent_id != le64_to_cpu(ci_req->ResumeKey[1])) {
>   7071			rsp->hdr.Status = STATUS_OBJECT_NAME_NOT_FOUND;
>   7072			goto out;
>   7073		}
>   7074	
>   7075		if (!dst_fp) {
>   7076			rsp->hdr.Status = STATUS_FILE_CLOSED;
>   7077			goto out;
>   7078		}
>   7079	
>   7080		/*
>   7081		 * FILE_READ_DATA should only be included in
>   7082		 * the FSCTL_COPYCHUNK case
>   7083		 */
>   7084		if (cnt_code == FSCTL_COPYCHUNK &&
>   7085		    !(dst_fp->daccess & (FILE_READ_DATA_LE | FILE_GENERIC_READ_LE)))
> {
>   7086			rsp->hdr.Status = STATUS_ACCESS_DENIED;
>   7087			goto out;
>   7088		}
>   7089	
>   7090		ret = ksmbd_vfs_copy_file_ranges(work, src_fp, dst_fp,
>   7091						 chunks, chunk_count,
>   7092						 &chunk_count_written,
>   7093						 &chunk_size_written,
>   7094						 &total_size_written);
>   7095		if (ret < 0) {
>   7096			if (ret == -EACCES)
>   7097				rsp->hdr.Status = STATUS_ACCESS_DENIED;
>   7098			if (ret == -EAGAIN)
>   7099				rsp->hdr.Status = STATUS_FILE_LOCK_CONFLICT;
>   7100			else if (ret == -EBADF)
>   7101				rsp->hdr.Status = STATUS_INVALID_HANDLE;
>   7102			else if (ret == -EFBIG || ret == -ENOSPC)
>   7103				rsp->hdr.Status = STATUS_DISK_FULL;
>   7104			else if (ret == -EINVAL)
>   7105				rsp->hdr.Status = STATUS_INVALID_PARAMETER;
>   7106			else if (ret == -EISDIR)
>   7107				rsp->hdr.Status = STATUS_FILE_IS_A_DIRECTORY;
>   7108			else if (ret == -E2BIG)
>   7109				rsp->hdr.Status = STATUS_INVALID_VIEW_SIZE;
>   7110			else
>   7111				rsp->hdr.Status = STATUS_UNEXPECTED_IO_ERROR;
>   7112		}
>   7113	
>   7114		ci_rsp->ChunksWritten = cpu_to_le32(chunk_count_written);
>   7115		ci_rsp->ChunkBytesWritten = cpu_to_le32(chunk_size_written);
>   7116		ci_rsp->TotalBytesWritten = cpu_to_le32(total_size_written);
>   7117	out:
>   7118		ksmbd_fd_put(work, src_fp);
>   7119		ksmbd_fd_put(work, dst_fp);
>   7120		return ret;
>   7121	}
>   7122	
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-09-18 21:43 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-18  9:45 [PATCH 1/4] ksmbd: add request buffer validation in smb2_set_info Namjae Jeon
2021-09-18  9:45 ` [PATCH 2/4] ksmbd: add validation in smb2_ioctl Namjae Jeon
2021-09-18 18:51   ` kernel test robot
2021-09-18 21:43     ` Namjae Jeon
2021-09-18  9:45 ` [PATCH 3/4] ksmbd: add validatioin for FILE_FULL_EA_INFORMATION of smb2_get_info Namjae Jeon
2021-09-18 15:55   ` Steve French
2021-09-18 17:57     ` Steve French
2021-09-18 18:10     ` Ralph Boehme
2021-09-18  9:45 ` [PATCH 4/4] ksmbd: add buffer validation for SMB2_CREATE_CONTEXT Namjae Jeon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).