[5.15.y PATCH 0/4] ksmbd: ZDI Vulnerability patches for 5.15.y

[5.15.y PATCH 0/4] ksmbd: ZDI Vulnerability patches for 5.15.y

[Namjae Jeon]

These are ZDI Vulnerability patches that was not applied in linux 5.15
stable kernel.

Namjae Jeon (4):
  ksmbd: use ksmbd_req_buf_next() in ksmbd_smb2_check_message()
  ksmbd: validate command payload size
  ksmbd: fix out-of-bound read in smb2_write
  ksmbd: validate session id and tree id in the compound request

 fs/ksmbd/server.c   | 33 ++++++++++++++++++++-------------
 fs/ksmbd/smb2misc.c | 38 ++++++++++++++++++++------------------
 fs/ksmbd/smb2pdu.c  | 44 +++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 79 insertions(+), 36 deletions(-)

-- 
2.25.1

[5.15.y PATCH 1/4] ksmbd: use ksmbd_req_buf_next() in ksmbd_smb2_check_message()

[Namjae Jeon]

From: Ralph Boehme <slow@samba.org>

commit b83b27909e74d27796de19c802fbc3b65ab4ba9a upstream.

Use ksmbd_req_buf_next() in ksmbd_smb2_check_message().

Cc: Tom Talpey <tom@talpey.com>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Steve French <smfrench@gmail.com>
Cc: Hyunchul Lee <hyc.lee@gmail.com>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Ralph Boehme <slow@samba.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/ksmbd/smb2misc.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/fs/ksmbd/smb2misc.c b/fs/ksmbd/smb2misc.c
index 33a927df64f1..abc18af14f04 100644
--- a/fs/ksmbd/smb2misc.c
+++ b/fs/ksmbd/smb2misc.c
@@ -347,16 +347,11 @@ static int smb2_validate_credit_charge(struct ksmbd_conn *conn,
 
 int ksmbd_smb2_check_message(struct ksmbd_work *work)
 {
-	struct smb2_pdu *pdu = work->request_buf;
+	struct smb2_pdu *pdu = ksmbd_req_buf_next(work);
 	struct smb2_hdr *hdr = &pdu->hdr;
 	int command;
 	__u32 clc_len;  /* calculated length */
-	__u32 len = get_rfc1002_len(pdu);
-
-	if (work->next_smb2_rcv_hdr_off) {
-		pdu = ksmbd_req_buf_next(work);
-		hdr = &pdu->hdr;
-	}
+	__u32 len = get_rfc1002_len(work->request_buf);
 
 	if (le32_to_cpu(hdr->NextCommand) > 0)
 		len = le32_to_cpu(hdr->NextCommand);
-- 
2.25.1

[5.15.y PATCH 2/4] ksmbd: validate command payload size

[Namjae Jeon]

commit 2b9b8f3b68edb3d67d79962f02e26dbb5ae3808d upstream.

->StructureSize2 indicates command payload size. ksmbd should validate
this size with rfc1002 length before accessing it.
This patch remove unneeded check and add the validation for this.

[    8.912583] BUG: KASAN: slab-out-of-bounds in ksmbd_smb2_check_message+0x12a/0xc50
[    8.913051] Read of size 2 at addr ffff88800ac7d92c by task kworker/0:0/7
...
[    8.914967] Call Trace:
[    8.915126]  <TASK>
[    8.915267]  dump_stack_lvl+0x33/0x50
[    8.915506]  print_report+0xcc/0x620
[    8.916558]  kasan_report+0xae/0xe0
[    8.917080]  kasan_check_range+0x35/0x1b0
[    8.917334]  ksmbd_smb2_check_message+0x12a/0xc50
[    8.917935]  ksmbd_verify_smb_message+0xae/0xd0
[    8.918223]  handle_ksmbd_work+0x192/0x820
[    8.918478]  process_one_work+0x419/0x760
[    8.918727]  worker_thread+0x2a2/0x6f0
[    8.919222]  kthread+0x187/0x1d0
[    8.919723]  ret_from_fork+0x1f/0x30
[    8.919954]  </TASK>

Cc: stable@vger.kernel.org
Reported-by: Chih-Yen Chang <cc85nod@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/ksmbd/smb2misc.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/fs/ksmbd/smb2misc.c b/fs/ksmbd/smb2misc.c
index abc18af14f04..ad805b37b81d 100644
--- a/fs/ksmbd/smb2misc.c
+++ b/fs/ksmbd/smb2misc.c
@@ -352,6 +352,7 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work)
 	int command;
 	__u32 clc_len;  /* calculated length */
 	__u32 len = get_rfc1002_len(work->request_buf);
+	__u32 req_struct_size;
 
 	if (le32_to_cpu(hdr->NextCommand) > 0)
 		len = le32_to_cpu(hdr->NextCommand);
@@ -374,17 +375,9 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work)
 	}
 
 	if (smb2_req_struct_sizes[command] != pdu->StructureSize2) {
-		if (command != SMB2_OPLOCK_BREAK_HE &&
-		    (hdr->Status == 0 || pdu->StructureSize2 != SMB2_ERROR_STRUCTURE_SIZE2_LE)) {
-			/* error packets have 9 byte structure size */
-			ksmbd_debug(SMB,
-				    "Illegal request size %u for command %d\n",
-				    le16_to_cpu(pdu->StructureSize2), command);
-			return 1;
-		} else if (command == SMB2_OPLOCK_BREAK_HE &&
-			   hdr->Status == 0 &&
-			   le16_to_cpu(pdu->StructureSize2) != OP_BREAK_STRUCT_SIZE_20 &&
-			   le16_to_cpu(pdu->StructureSize2) != OP_BREAK_STRUCT_SIZE_21) {
+		if (command == SMB2_OPLOCK_BREAK_HE &&
+		    le16_to_cpu(pdu->StructureSize2) != OP_BREAK_STRUCT_SIZE_20 &&
+		    le16_to_cpu(pdu->StructureSize2) != OP_BREAK_STRUCT_SIZE_21) {
 			/* special case for SMB2.1 lease break message */
 			ksmbd_debug(SMB,
 				    "Illegal request size %d for oplock break\n",
@@ -393,6 +386,14 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work)
 		}
 	}
 
+	req_struct_size = le16_to_cpu(pdu->StructureSize2) +
+		__SMB2_HEADER_STRUCTURE_SIZE;
+	if (command == SMB2_LOCK_HE)
+		req_struct_size -= sizeof(struct smb2_lock_element);
+
+	if (req_struct_size > len + 1)
+		return 1;
+
 	if (smb2_calc_size(hdr, &clc_len))
 		return 1;
 
-- 
2.25.1

[5.15.y PATCH 3/4] ksmbd: fix out-of-bound read in smb2_write

[Namjae Jeon]

commit 5fe7f7b78290638806211046a99f031ff26164e1 upstream.

ksmbd_smb2_check_message doesn't validate hdr->NextCommand. If
->NextCommand is bigger than Offset + Length of smb2 write, It will
allow oversized smb2 write length. It will cause OOB read in smb2_write.

Cc: stable@vger.kernel.org
Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-21164
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/ksmbd/smb2misc.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/fs/ksmbd/smb2misc.c b/fs/ksmbd/smb2misc.c
index ad805b37b81d..c24674fc1904 100644
--- a/fs/ksmbd/smb2misc.c
+++ b/fs/ksmbd/smb2misc.c
@@ -352,10 +352,16 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work)
 	int command;
 	__u32 clc_len;  /* calculated length */
 	__u32 len = get_rfc1002_len(work->request_buf);
-	__u32 req_struct_size;
+	__u32 req_struct_size, next_cmd = le32_to_cpu(hdr->NextCommand);
 
-	if (le32_to_cpu(hdr->NextCommand) > 0)
-		len = le32_to_cpu(hdr->NextCommand);
+	if ((u64)work->next_smb2_rcv_hdr_off + next_cmd > len) {
+		pr_err("next command(%u) offset exceeds smb msg size\n",
+				next_cmd);
+		return 1;
+	}
+
+	if (next_cmd > 0)
+		len = next_cmd;
 	else if (work->next_smb2_rcv_hdr_off)
 		len -= work->next_smb2_rcv_hdr_off;
 
-- 
2.25.1

[5.15.y PATCH 4/4] ksmbd: validate session id and tree id in the compound request

[Namjae Jeon]

commit 5005bcb4219156f1bf7587b185080ec1da08518e upstream.

This patch validate session id and tree id in compound request.
If first operation in the compound is SMB2 ECHO request, ksmbd bypass
session and tree validation. So work->sess and work->tcon could be NULL.
If secound request in the compound access work->sess or tcon, It cause
NULL pointer dereferecing error.

Cc: stable@vger.kernel.org
Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-21165
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/ksmbd/server.c  | 33 ++++++++++++++++++++-------------
 fs/ksmbd/smb2pdu.c | 44 +++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 59 insertions(+), 18 deletions(-)

diff --git a/fs/ksmbd/server.c b/fs/ksmbd/server.c
index 1c5e7e023058..eb45d56b3577 100644
--- a/fs/ksmbd/server.c
+++ b/fs/ksmbd/server.c
@@ -184,24 +184,31 @@ static void __handle_ksmbd_work(struct ksmbd_work *work,
 		goto send;
 	}
 
-	if (conn->ops->check_user_session) {
-		rc = conn->ops->check_user_session(work);
-		if (rc < 0) {
-			command = conn->ops->get_cmd_val(work);
-			conn->ops->set_rsp_status(work,
-					STATUS_USER_SESSION_DELETED);
-			goto send;
-		} else if (rc > 0) {
-			rc = conn->ops->get_ksmbd_tcon(work);
+	do {
+		if (conn->ops->check_user_session) {
+			rc = conn->ops->check_user_session(work);
 			if (rc < 0) {
-				conn->ops->set_rsp_status(work,
-					STATUS_NETWORK_NAME_DELETED);
+				if (rc == -EINVAL)
+					conn->ops->set_rsp_status(work,
+						STATUS_INVALID_PARAMETER);
+				else
+					conn->ops->set_rsp_status(work,
+						STATUS_USER_SESSION_DELETED);
 				goto send;
+			} else if (rc > 0) {
+				rc = conn->ops->get_ksmbd_tcon(work);
+				if (rc < 0) {
+					if (rc == -EINVAL)
+						conn->ops->set_rsp_status(work,
+							STATUS_INVALID_PARAMETER);
+					else
+						conn->ops->set_rsp_status(work,
+							STATUS_NETWORK_NAME_DELETED);
+					goto send;
+				}
 			}
 		}
-	}
 
-	do {
 		rc = __process_request(work, conn, &command);
 		if (rc == SERVER_HANDLER_ABORT)
 			break;
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index 266430a2a0e0..9f9d07caa57e 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -97,7 +97,6 @@ int smb2_get_ksmbd_tcon(struct ksmbd_work *work)
 	struct smb2_hdr *req_hdr = work->request_buf;
 	int tree_id;
 
-	work->tcon = NULL;
 	if (work->conn->ops->get_cmd_val(work) == SMB2_TREE_CONNECT_HE ||
 	    work->conn->ops->get_cmd_val(work) ==  SMB2_CANCEL_HE ||
 	    work->conn->ops->get_cmd_val(work) ==  SMB2_LOGOFF_HE) {
@@ -111,10 +110,28 @@ int smb2_get_ksmbd_tcon(struct ksmbd_work *work)
 	}
 
 	tree_id = le32_to_cpu(req_hdr->Id.SyncId.TreeId);
+
+	/*
+	 * If request is not the first in Compound request,
+	 * Just validate tree id in header with work->tcon->id.
+	 */
+	if (work->next_smb2_rcv_hdr_off) {
+		if (!work->tcon) {
+			pr_err("The first operation in the compound does not have tcon\n");
+			return -EINVAL;
+		}
+		if (work->tcon->id != tree_id) {
+			pr_err("tree id(%u) is different with id(%u) in first operation\n",
+					tree_id, work->tcon->id);
+			return -EINVAL;
+		}
+		return 1;
+	}
+
 	work->tcon = ksmbd_tree_conn_lookup(work->sess, tree_id);
 	if (!work->tcon) {
 		pr_err("Invalid tid %d\n", tree_id);
-		return -EINVAL;
+		return -ENOENT;
 	}
 
 	return 1;
@@ -569,7 +586,6 @@ int smb2_check_user_session(struct ksmbd_work *work)
 	unsigned int cmd = conn->ops->get_cmd_val(work);
 	unsigned long long sess_id;
 
-	work->sess = NULL;
 	/*
 	 * SMB2_ECHO, SMB2_NEGOTIATE, SMB2_SESSION_SETUP command do not
 	 * require a session id, so no need to validate user session's for
@@ -580,15 +596,33 @@ int smb2_check_user_session(struct ksmbd_work *work)
 		return 0;
 
 	if (!ksmbd_conn_good(work))
-		return -EINVAL;
+		return -EIO;
 
 	sess_id = le64_to_cpu(req_hdr->SessionId);
+
+	/*
+	 * If request is not the first in Compound request,
+	 * Just validate session id in header with work->sess->id.
+	 */
+	if (work->next_smb2_rcv_hdr_off) {
+		if (!work->sess) {
+			pr_err("The first operation in the compound does not have sess\n");
+			return -EINVAL;
+		}
+		if (work->sess->id != sess_id) {
+			pr_err("session id(%llu) is different with the first operation(%lld)\n",
+					sess_id, work->sess->id);
+			return -EINVAL;
+		}
+		return 1;
+	}
+
 	/* Check for validity of user session */
 	work->sess = ksmbd_session_lookup_all(conn, sess_id);
 	if (work->sess)
 		return 1;
 	ksmbd_debug(SMB, "Invalid user session, Uid %llu\n", sess_id);
-	return -EINVAL;
+	return -ENOENT;
 }
 
 static void destroy_previous_session(struct ksmbd_conn *conn,
-- 
2.25.1

[5.15.y PATCH 0/4] ksmbd: ZDI Vulnerability patches for 5.15.y

[Namjae Jeon]

These are ZDI Vulnerability patches that was not applied in linux 5.15
stable kernel.

Namjae Jeon (4):
  ksmbd: use ksmbd_req_buf_next() in ksmbd_smb2_check_message()
  ksmbd: validate command payload size
  ksmbd: fix out-of-bound read in smb2_write
  ksmbd: validate session id and tree id in the compound request

 fs/ksmbd/server.c   | 33 ++++++++++++++++++++-------------
 fs/ksmbd/smb2misc.c | 38 ++++++++++++++++++++------------------
 fs/ksmbd/smb2pdu.c  | 44 +++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 79 insertions(+), 36 deletions(-)

-- 
2.25.1

Re: [5.15.y PATCH 0/4] ksmbd: ZDI Vulnerability patches for 5.15.y

[Namjae Jeon]

2023-07-20 22:23 GMT+09:00, Namjae Jeon <linkinjeon@kernel.org>:
> These are ZDI Vulnerability patches that was not applied in linux 5.15
> stable kernel.
Note that same patch sent twice...
>
> Namjae Jeon (4):
>   ksmbd: use ksmbd_req_buf_next() in ksmbd_smb2_check_message()
>   ksmbd: validate command payload size
>   ksmbd: fix out-of-bound read in smb2_write
>   ksmbd: validate session id and tree id in the compound request
>
>  fs/ksmbd/server.c   | 33 ++++++++++++++++++++-------------
>  fs/ksmbd/smb2misc.c | 38 ++++++++++++++++++++------------------
>  fs/ksmbd/smb2pdu.c  | 44 +++++++++++++++++++++++++++++++++++++++-----
>  3 files changed, 79 insertions(+), 36 deletions(-)
>
> --
> 2.25.1
>
>

[5.15.y PATCH 1/4] ksmbd: use ksmbd_req_buf_next() in ksmbd_smb2_check_message()

[Namjae Jeon]

From: Ralph Boehme <slow@samba.org>

commit b83b27909e74d27796de19c802fbc3b65ab4ba9a upstream.

Use ksmbd_req_buf_next() in ksmbd_smb2_check_message().

Cc: Tom Talpey <tom@talpey.com>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Steve French <smfrench@gmail.com>
Cc: Hyunchul Lee <hyc.lee@gmail.com>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Ralph Boehme <slow@samba.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/ksmbd/smb2misc.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/fs/ksmbd/smb2misc.c b/fs/ksmbd/smb2misc.c
index 33a927df64f1..abc18af14f04 100644
--- a/fs/ksmbd/smb2misc.c
+++ b/fs/ksmbd/smb2misc.c
@@ -347,16 +347,11 @@ static int smb2_validate_credit_charge(struct ksmbd_conn *conn,
 
 int ksmbd_smb2_check_message(struct ksmbd_work *work)
 {
-	struct smb2_pdu *pdu = work->request_buf;
+	struct smb2_pdu *pdu = ksmbd_req_buf_next(work);
 	struct smb2_hdr *hdr = &pdu->hdr;
 	int command;
 	__u32 clc_len;  /* calculated length */
-	__u32 len = get_rfc1002_len(pdu);
-
-	if (work->next_smb2_rcv_hdr_off) {
-		pdu = ksmbd_req_buf_next(work);
-		hdr = &pdu->hdr;
-	}
+	__u32 len = get_rfc1002_len(work->request_buf);
 
 	if (le32_to_cpu(hdr->NextCommand) > 0)
 		len = le32_to_cpu(hdr->NextCommand);
-- 
2.25.1

Re: [5.15.y PATCH 1/4] ksmbd: use ksmbd_req_buf_next() in ksmbd_smb2_check_message()

[Namjae Jeon]

2023-07-20 22:23 GMT+09:00, Namjae Jeon <linkinjeon@kernel.org>:
> From: Ralph Boehme <slow@samba.org>
>
> commit b83b27909e74d27796de19c802fbc3b65ab4ba9a upstream.
>
> Use ksmbd_req_buf_next() in ksmbd_smb2_check_message().
Note that same patch sent twice...
>
> Cc: Tom Talpey <tom@talpey.com>
> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> Cc: Steve French <smfrench@gmail.com>
> Cc: Hyunchul Lee <hyc.lee@gmail.com>
> Acked-by: Namjae Jeon <linkinjeon@kernel.org>
> Signed-off-by: Ralph Boehme <slow@samba.org>
> Signed-off-by: Steve French <stfrench@microsoft.com>
> ---
>  fs/ksmbd/smb2misc.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/fs/ksmbd/smb2misc.c b/fs/ksmbd/smb2misc.c
> index 33a927df64f1..abc18af14f04 100644
> --- a/fs/ksmbd/smb2misc.c
> +++ b/fs/ksmbd/smb2misc.c
> @@ -347,16 +347,11 @@ static int smb2_validate_credit_charge(struct
> ksmbd_conn *conn,
>
>  int ksmbd_smb2_check_message(struct ksmbd_work *work)
>  {
> -	struct smb2_pdu *pdu = work->request_buf;
> +	struct smb2_pdu *pdu = ksmbd_req_buf_next(work);
>  	struct smb2_hdr *hdr = &pdu->hdr;
>  	int command;
>  	__u32 clc_len;  /* calculated length */
> -	__u32 len = get_rfc1002_len(pdu);
> -
> -	if (work->next_smb2_rcv_hdr_off) {
> -		pdu = ksmbd_req_buf_next(work);
> -		hdr = &pdu->hdr;
> -	}
> +	__u32 len = get_rfc1002_len(work->request_buf);
>
>  	if (le32_to_cpu(hdr->NextCommand) > 0)
>  		len = le32_to_cpu(hdr->NextCommand);
> --
> 2.25.1
>
>

Re: [5.15.y PATCH 0/4] ksmbd: ZDI Vulnerability patches for 5.15.y

[Greg KH]

On Thu, Jul 20, 2023 at 10:23:27PM +0900, Namjae Jeon wrote:
> These are ZDI Vulnerability patches that was not applied in linux 5.15
> stable kernel.
> 
> Namjae Jeon (4):
>   ksmbd: use ksmbd_req_buf_next() in ksmbd_smb2_check_message()
>   ksmbd: validate command payload size
>   ksmbd: fix out-of-bound read in smb2_write
>   ksmbd: validate session id and tree id in the compound request
> 
>  fs/ksmbd/server.c   | 33 ++++++++++++++++++++-------------
>  fs/ksmbd/smb2misc.c | 38 ++++++++++++++++++++------------------
>  fs/ksmbd/smb2pdu.c  | 44 +++++++++++++++++++++++++++++++++++++++-----
>  3 files changed, 79 insertions(+), 36 deletions(-)
> 
> -- 
> 2.25.1
> 

All now queued up now, thanks!

greg k-h

Re: [5.15.y PATCH 0/4] ksmbd: ZDI Vulnerability patches for 5.15.y

[Namjae Jeon]

2023-07-21 2:54 GMT+09:00, Greg KH <gregkh@linuxfoundation.org>:
> On Thu, Jul 20, 2023 at 10:23:27PM +0900, Namjae Jeon wrote:
>> These are ZDI Vulnerability patches that was not applied in linux 5.15
>> stable kernel.
>>
>> Namjae Jeon (4):
>>   ksmbd: use ksmbd_req_buf_next() in ksmbd_smb2_check_message()
>>   ksmbd: validate command payload size
>>   ksmbd: fix out-of-bound read in smb2_write
>>   ksmbd: validate session id and tree id in the compound request
>>
>>  fs/ksmbd/server.c   | 33 ++++++++++++++++++++-------------
>>  fs/ksmbd/smb2misc.c | 38 ++++++++++++++++++++------------------
>>  fs/ksmbd/smb2pdu.c  | 44 +++++++++++++++++++++++++++++++++++++++-----
>>  3 files changed, 79 insertions(+), 36 deletions(-)
>>
>> --
>> 2.25.1
>>
>
> All now queued up now, thanks!
Thank you!
>
> greg k-h
>