diff mbox series

ksmbd: fix multiple out-of-bounds read during context decoding

Message ID 20230517185820.1264368-1-h3xrabbit@gmail.com (mailing list archive)
State New, archived
Headers show
Series ksmbd: fix multiple out-of-bounds read during context decoding | expand

Commit Message

Hex Rabbit May 17, 2023, 6:58 p.m. UTC 
From: Kuan-Ting Chen <h3xrabbit@gmail.com>

Ensure the context's length is valid (excluding VLAs) before casting the
pointer to the corresponding structure pointer type, also removed
redundant check on `len_of_ctxts`.

Signed-off-by: Kuan-Ting Chen <h3xrabbit@gmail.com>
---
 fs/ksmbd/smb2pdu.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

Comments

Jeremy Allison May 17, 2023, 7:02 p.m. UTC | #1 
On Wed, May 17, 2023 at 06:58:20PM +0000, HexRabbit wrote:
>From: Kuan-Ting Chen <h3xrabbit@gmail.com>
>
>Ensure the context's length is valid (excluding VLAs) before casting the
>pointer to the corresponding structure pointer type, also removed
>redundant check on `len_of_ctxts`.
>
>Signed-off-by: Kuan-Ting Chen <h3xrabbit@gmail.com>
>---
> fs/ksmbd/smb2pdu.c | 23 +++++++++++++++++------
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
>diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
>index 972176bff..83b877254 100644
>--- a/fs/ksmbd/smb2pdu.c
>+++ b/fs/ksmbd/smb2pdu.c
>@@ -969,18 +969,16 @@ static __le32 deassemble_neg_contexts(struct ksmbd_conn *conn,
> 	len_of_ctxts = len_of_smb - offset;
>
> 	while (i++ < neg_ctxt_cnt) {
>-		int clen;
>-
>-		/* check that offset is not beyond end of SMB */
>-		if (len_of_ctxts == 0)
>-			break;
>+		int clen, ctxt_len;

Just a drive-by comment here (haven't had time to look at the
underlying code).

Should lengths in protocol parsing *ever* be defined at 'int' ?

IMHO no, never. That's a disaster waiting to happen as int
overflow driven by the peer can often cause integer wrap to
negative, leaving all the nice "not greater than packet length"
to fail horribly. We excised all 'int' length representations
from the Samba parser a long time ago.
Sergey Senozhatsky May 18, 2023, 12:37 a.m. UTC | #2 
On (23/05/18 03:28), Hex Rabbit wrote:
>
>    Maybe it might be worth addressing these issues in the upcoming patches.
>

I guess that would be nice to see
Jeremy Allison May 18, 2023, 12:46 a.m. UTC | #3 
On Thu, May 18, 2023 at 09:37:48AM +0900, Sergey Senozhatsky wrote:
>On (23/05/18 03:28), Hex Rabbit wrote:
>>
>>    Maybe it might be worth addressing these issues in the upcoming patches.
>>
>
>I guess that would be nice to see

More than nice. Essential IMHO.
Namjae Jeon May 18, 2023, 5:45 a.m. UTC | #4 
2023-05-18 3:58 GMT+09:00, HexRabbit <h3xrabbit@gmail.com>:
> From: Kuan-Ting Chen <h3xrabbit@gmail.com>
>
> Ensure the context's length is valid (excluding VLAs) before casting the
> pointer to the corresponding structure pointer type, also removed
> redundant check on `len_of_ctxts`.
>
> Signed-off-by: Kuan-Ting Chen <h3xrabbit@gmail.com>
> ---
>  fs/ksmbd/smb2pdu.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
> index 972176bff..83b877254 100644
> --- a/fs/ksmbd/smb2pdu.c
> +++ b/fs/ksmbd/smb2pdu.c
> @@ -969,18 +969,16 @@ static __le32 deassemble_neg_contexts(struct
> ksmbd_conn *conn,
>  	len_of_ctxts = len_of_smb - offset;
>
>  	while (i++ < neg_ctxt_cnt) {
> -		int clen;
> -
> -		/* check that offset is not beyond end of SMB */
> -		if (len_of_ctxts == 0)
> -			break;
> +		int clen, ctxt_len;
>
>  		if (len_of_ctxts < sizeof(struct smb2_neg_context))
>  			break;
>
>  		pctx = (struct smb2_neg_context *)((char *)pctx + offset);
>  		clen = le16_to_cpu(pctx->DataLength);
> -		if (clen + sizeof(struct smb2_neg_context) > len_of_ctxts)
> +		ctxt_len = clen + sizeof(struct smb2_neg_context);
> +
> +		if (ctxt_len > len_of_ctxts)
>  			break;
>
>  		if (pctx->ContextType == SMB2_PREAUTH_INTEGRITY_CAPABILITIES) {
> @@ -989,6 +987,9 @@ static __le32 deassemble_neg_contexts(struct ksmbd_conn
> *conn,
>  			if (conn->preauth_info->Preauth_HashId)
>  				break;
>
> +			if (ctxt_len < sizeof(struct smb2_preauth_neg_context))
> +				break;
> +
>  			status = decode_preauth_ctxt(conn,
>  						     (struct smb2_preauth_neg_context *)pctx,
>  						     len_of_ctxts);
> @@ -1000,6 +1001,9 @@ static __le32 deassemble_neg_contexts(struct
> ksmbd_conn *conn,
>  			if (conn->cipher_type)
>  				break;
>
> +			if (ctxt_len < sizeof(struct smb2_encryption_neg_context))
You need to consider Ciphers flex-array size to validate ctxt_len. we
can get its size using CipherCount in smb2_encryption_neg_context.
> +				break;
> +
>  			decode_encrypt_ctxt(conn,
>  					    (struct smb2_encryption_neg_context *)pctx,
>  					    len_of_ctxts);
> @@ -1009,6 +1013,9 @@ static __le32 deassemble_neg_contexts(struct
> ksmbd_conn *conn,
>  			if (conn->compress_algorithm)
>  				break;
>
> +			if (ctxt_len < sizeof(struct smb2_compression_capabilities_context))
Ditto.
> +				break;
> +
>  			decode_compress_ctxt(conn,
>  					     (struct smb2_compression_capabilities_context *)pctx);
>  		} else if (pctx->ContextType == SMB2_NETNAME_NEGOTIATE_CONTEXT_ID) {
> @@ -1021,6 +1028,10 @@ static __le32 deassemble_neg_contexts(struct
> ksmbd_conn *conn,
>  		} else if (pctx->ContextType == SMB2_SIGNING_CAPABILITIES) {
>  			ksmbd_debug(SMB,
>  				    "deassemble SMB2_SIGNING_CAPABILITIES context\n");
> +
> +			if (ctxt_len < sizeof(struct smb2_signing_capabilities))
Ditto.

Thanks.
> +				break;
> +
>  			decode_sign_cap_ctxt(conn,
>  					     (struct smb2_signing_capabilities *)pctx,
>  					     len_of_ctxts);
> --
> 2.25.1
>
>
Hex Rabbit May 18, 2023, 6:30 a.m. UTC | #5 
> You need to consider Ciphers flex-array size to validate ctxt_len. we
> can get its size using CipherCount in smb2_encryption_neg_context.

I'm not checking the flex-array size since both `decode_sign_cap_ctxt()`
and `decode_encrypt_ctxt()` have done it, or should I move it out?

```
if (sizeof(struct smb2_encryption_neg_context) + cphs_size >
   len_of_ctxts) {
    pr_err("Invalid cipher count(%d)\n", cph_cnt);
    return;
}
```

```
if (sizeof(struct smb2_signing_capabilities) + sign_alos_size >
   len_of_ctxts) {
    pr_err("Invalid signing algorithm count(%d)\n", sign_algo_cnt);
    return;
}
```
Namjae Jeon May 18, 2023, 6:36 a.m. UTC | #6 
2023-05-18 15:30 GMT+09:00, Hex Rabbit <h3xrabbit@gmail.com>:
>> You need to consider Ciphers flex-array size to validate ctxt_len. we
>> can get its size using CipherCount in smb2_encryption_neg_context.
>
> I'm not checking the flex-array size since both `decode_sign_cap_ctxt()`
> and `decode_encrypt_ctxt()` have done it, or should I move it out?
Yes, We can move it out. Thanks.
>
> ```
> if (sizeof(struct smb2_encryption_neg_context) + cphs_size >
>    len_of_ctxts) {
>     pr_err("Invalid cipher count(%d)\n", cph_cnt);
>     return;
> }
> ```
>
> ```
> if (sizeof(struct smb2_signing_capabilities) + sign_alos_size >
>    len_of_ctxts) {
>     pr_err("Invalid signing algorithm count(%d)\n", sign_algo_cnt);
>     return;
> }
> ```
>
Hex Rabbit May 18, 2023, 8:11 a.m. UTC | #7 
I have decided to leave the modifications within the function that handles the
corresponding context. The reason for this is that I believe consolidating the
checks together can improve readability, also, moving them out would require
us to read the size of the flex-sized array again in the corresponding function.

What do you think?

below is the modified patch:

diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index 972176bff..0285c3f9e 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -849,13 +849,13 @@ static void assemble_neg_contexts(struct ksmbd_conn *conn,

 static __le32 decode_preauth_ctxt(struct ksmbd_conn *conn,
   struct smb2_preauth_neg_context *pneg_ctxt,
-  int len_of_ctxts)
+  int ctxt_len)
 {
  /*
  * sizeof(smb2_preauth_neg_context) assumes SMB311_SALT_SIZE Salt,
  * which may not be present. Only check for used HashAlgorithms[1].
  */
- if (len_of_ctxts < MIN_PREAUTH_CTXT_DATA_LEN)
+ if (ctxt_len < MIN_PREAUTH_CTXT_DATA_LEN)
  return STATUS_INVALID_PARAMETER;

  if (pneg_ctxt->HashAlgorithms != SMB2_PREAUTH_INTEGRITY_SHA512)
@@ -867,15 +867,23 @@ static __le32 decode_preauth_ctxt(struct ksmbd_conn *conn,

 static void decode_encrypt_ctxt(struct ksmbd_conn *conn,
  struct smb2_encryption_neg_context *pneg_ctxt,
- int len_of_ctxts)
+ int ctxt_len)
 {
- int cph_cnt = le16_to_cpu(pneg_ctxt->CipherCount);
- int i, cphs_size = cph_cnt * sizeof(__le16);
+ int cph_cnt;
+ int i, cphs_size;
+
+ if (sizeof(struct smb2_encryption_neg_context) > ctxt_len) {
+ pr_err("Invalid SMB2_ENCRYPTION_CAPABILITIES context size\n");
+ return;
+ }

  conn->cipher_type = 0;

+ cph_cnt = le16_to_cpu(pneg_ctxt->CipherCount);
+ cphs_size = cph_cnt * sizeof(__le16);
+
  if (sizeof(struct smb2_encryption_neg_context) + cphs_size >
-    len_of_ctxts) {
+    ctxt_len) {
  pr_err("Invalid cipher count(%d)\n", cph_cnt);
  return;
  }
@@ -923,15 +931,22 @@ static void decode_compress_ctxt(struct ksmbd_conn *conn,

 static void decode_sign_cap_ctxt(struct ksmbd_conn *conn,
  struct smb2_signing_capabilities *pneg_ctxt,
- int len_of_ctxts)
+ int ctxt_len)
 {
- int sign_algo_cnt = le16_to_cpu(pneg_ctxt->SigningAlgorithmCount);
- int i, sign_alos_size = sign_algo_cnt * sizeof(__le16);
+ int sign_algo_cnt;
+ int i, sign_alos_size;
+
+ if (sizeof(struct smb2_signing_capabilities) > ctxt_len) {
+ pr_err("Invalid SMB2_SIGNING_CAPABILITIES context length\n");
+ return;
+ }

  conn->signing_negotiated = false;
+ sign_algo_cnt = le16_to_cpu(pneg_ctxt->SigningAlgorithmCount);
+ sign_alos_size = sign_algo_cnt * sizeof(__le16);

  if (sizeof(struct smb2_signing_capabilities) + sign_alos_size >
-    len_of_ctxts) {
+    ctxt_len) {
  pr_err("Invalid signing algorithm count(%d)\n", sign_algo_cnt);
  return;
  }
@@ -969,18 +984,16 @@ static __le32 deassemble_neg_contexts(struct
ksmbd_conn *conn,
  len_of_ctxts = len_of_smb - offset;

  while (i++ < neg_ctxt_cnt) {
- int clen;
-
- /* check that offset is not beyond end of SMB */
- if (len_of_ctxts == 0)
- break;
+ int clen, ctxt_len;

  if (len_of_ctxts < sizeof(struct smb2_neg_context))
  break;

  pctx = (struct smb2_neg_context *)((char *)pctx + offset);
  clen = le16_to_cpu(pctx->DataLength);
- if (clen + sizeof(struct smb2_neg_context) > len_of_ctxts)
+ ctxt_len = clen + sizeof(struct smb2_neg_context);
+
+ if (ctxt_len > len_of_ctxts)
  break;

  if (pctx->ContextType == SMB2_PREAUTH_INTEGRITY_CAPABILITIES) {
@@ -991,7 +1004,7 @@ static __le32 deassemble_neg_contexts(struct
ksmbd_conn *conn,

  status = decode_preauth_ctxt(conn,
      (struct smb2_preauth_neg_context *)pctx,
-     len_of_ctxts);
+     ctxt_len);
  if (status != STATUS_SUCCESS)
  break;
  } else if (pctx->ContextType == SMB2_ENCRYPTION_CAPABILITIES) {
@@ -1002,7 +1015,7 @@ static __le32 deassemble_neg_contexts(struct
ksmbd_conn *conn,

  decode_encrypt_ctxt(conn,
     (struct smb2_encryption_neg_context *)pctx,
-    len_of_ctxts);
+    ctxt_len);
  } else if (pctx->ContextType == SMB2_COMPRESSION_CAPABILITIES) {
  ksmbd_debug(SMB,
     "deassemble SMB2_COMPRESSION_CAPABILITIES context\n");
@@ -1021,9 +1034,10 @@ static __le32 deassemble_neg_contexts(struct
ksmbd_conn *conn,
  } else if (pctx->ContextType == SMB2_SIGNING_CAPABILITIES) {
  ksmbd_debug(SMB,
     "deassemble SMB2_SIGNING_CAPABILITIES context\n");
+
  decode_sign_cap_ctxt(conn,
      (struct smb2_signing_capabilities *)pctx,
-     len_of_ctxts);
+     ctxt_len);
  }

  /* offsets must be 8 byte aligned */
---

Namjae Jeon <linkinjeon@kernel.org> 於 2023年5月18日 週四 下午2:36寫道：
>
> 2023-05-18 15:30 GMT+09:00, Hex Rabbit <h3xrabbit@gmail.com>:
> >> You need to consider Ciphers flex-array size to validate ctxt_len. we
> >> can get its size using CipherCount in smb2_encryption_neg_context.
> >
> > I'm not checking the flex-array size since both `decode_sign_cap_ctxt()`
> > and `decode_encrypt_ctxt()` have done it, or should I move it out?
> Yes, We can move it out. Thanks.
> >
> > ```
> > if (sizeof(struct smb2_encryption_neg_context) + cphs_size >
> >    len_of_ctxts) {
> >     pr_err("Invalid cipher count(%d)\n", cph_cnt);
> >     return;
> > }
> > ```
> >
> > ```
> > if (sizeof(struct smb2_signing_capabilities) + sign_alos_size >
> >    len_of_ctxts) {
> >     pr_err("Invalid signing algorithm count(%d)\n", sign_algo_cnt);
> >     return;
> > }
> > ```
> >
Namjae Jeon May 18, 2023, 2:10 p.m. UTC | #8 
2023-05-18 17:11 GMT+09:00, Hex Rabbit <h3xrabbit@gmail.com>:
> I have decided to leave the modifications within the function that handles
> the
> corresponding context. The reason for this is that I believe consolidating
> the
> checks together can improve readability, also, moving them out would
> require
> us to read the size of the flex-sized array again in the corresponding
> function.
>
> What do you think?
Looks okay. Please send the patch to test to the list.

Thanks.
>
> below is the modified patch:
>
> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
> index 972176bff..0285c3f9e 100644
> --- a/fs/ksmbd/smb2pdu.c
> +++ b/fs/ksmbd/smb2pdu.c
> @@ -849,13 +849,13 @@ static void assemble_neg_contexts(struct ksmbd_conn
> *conn,
>
>  static __le32 decode_preauth_ctxt(struct ksmbd_conn *conn,
>    struct smb2_preauth_neg_context *pneg_ctxt,
> -  int len_of_ctxts)
> +  int ctxt_len)
>  {
>   /*
>   * sizeof(smb2_preauth_neg_context) assumes SMB311_SALT_SIZE Salt,
>   * which may not be present. Only check for used HashAlgorithms[1].
>   */
> - if (len_of_ctxts < MIN_PREAUTH_CTXT_DATA_LEN)
> + if (ctxt_len < MIN_PREAUTH_CTXT_DATA_LEN)
>   return STATUS_INVALID_PARAMETER;
>
>   if (pneg_ctxt->HashAlgorithms != SMB2_PREAUTH_INTEGRITY_SHA512)
> @@ -867,15 +867,23 @@ static __le32 decode_preauth_ctxt(struct ksmbd_conn
> *conn,
>
>  static void decode_encrypt_ctxt(struct ksmbd_conn *conn,
>   struct smb2_encryption_neg_context *pneg_ctxt,
> - int len_of_ctxts)
> + int ctxt_len)
>  {
> - int cph_cnt = le16_to_cpu(pneg_ctxt->CipherCount);
> - int i, cphs_size = cph_cnt * sizeof(__le16);
> + int cph_cnt;
> + int i, cphs_size;
> +
> + if (sizeof(struct smb2_encryption_neg_context) > ctxt_len) {
> + pr_err("Invalid SMB2_ENCRYPTION_CAPABILITIES context size\n");
> + return;
> + }
>
>   conn->cipher_type = 0;
>
> + cph_cnt = le16_to_cpu(pneg_ctxt->CipherCount);
> + cphs_size = cph_cnt * sizeof(__le16);
> +
>   if (sizeof(struct smb2_encryption_neg_context) + cphs_size >
> -    len_of_ctxts) {
> +    ctxt_len) {
>   pr_err("Invalid cipher count(%d)\n", cph_cnt);
>   return;
>   }
> @@ -923,15 +931,22 @@ static void decode_compress_ctxt(struct ksmbd_conn
> *conn,
>
>  static void decode_sign_cap_ctxt(struct ksmbd_conn *conn,
>   struct smb2_signing_capabilities *pneg_ctxt,
> - int len_of_ctxts)
> + int ctxt_len)
>  {
> - int sign_algo_cnt = le16_to_cpu(pneg_ctxt->SigningAlgorithmCount);
> - int i, sign_alos_size = sign_algo_cnt * sizeof(__le16);
> + int sign_algo_cnt;
> + int i, sign_alos_size;
> +
> + if (sizeof(struct smb2_signing_capabilities) > ctxt_len) {
> + pr_err("Invalid SMB2_SIGNING_CAPABILITIES context length\n");
> + return;
> + }
>
>   conn->signing_negotiated = false;
> + sign_algo_cnt = le16_to_cpu(pneg_ctxt->SigningAlgorithmCount);
> + sign_alos_size = sign_algo_cnt * sizeof(__le16);
>
>   if (sizeof(struct smb2_signing_capabilities) + sign_alos_size >
> -    len_of_ctxts) {
> +    ctxt_len) {
>   pr_err("Invalid signing algorithm count(%d)\n", sign_algo_cnt);
>   return;
>   }
> @@ -969,18 +984,16 @@ static __le32 deassemble_neg_contexts(struct
> ksmbd_conn *conn,
>   len_of_ctxts = len_of_smb - offset;
>
>   while (i++ < neg_ctxt_cnt) {
> - int clen;
> -
> - /* check that offset is not beyond end of SMB */
> - if (len_of_ctxts == 0)
> - break;
> + int clen, ctxt_len;
>
>   if (len_of_ctxts < sizeof(struct smb2_neg_context))
>   break;
>
>   pctx = (struct smb2_neg_context *)((char *)pctx + offset);
>   clen = le16_to_cpu(pctx->DataLength);
> - if (clen + sizeof(struct smb2_neg_context) > len_of_ctxts)
> + ctxt_len = clen + sizeof(struct smb2_neg_context);
> +
> + if (ctxt_len > len_of_ctxts)
>   break;
>
>   if (pctx->ContextType == SMB2_PREAUTH_INTEGRITY_CAPABILITIES) {
> @@ -991,7 +1004,7 @@ static __le32 deassemble_neg_contexts(struct
> ksmbd_conn *conn,
>
>   status = decode_preauth_ctxt(conn,
>       (struct smb2_preauth_neg_context *)pctx,
> -     len_of_ctxts);
> +     ctxt_len);
>   if (status != STATUS_SUCCESS)
>   break;
>   } else if (pctx->ContextType == SMB2_ENCRYPTION_CAPABILITIES) {
> @@ -1002,7 +1015,7 @@ static __le32 deassemble_neg_contexts(struct
> ksmbd_conn *conn,
>
>   decode_encrypt_ctxt(conn,
>      (struct smb2_encryption_neg_context *)pctx,
> -    len_of_ctxts);
> +    ctxt_len);
>   } else if (pctx->ContextType == SMB2_COMPRESSION_CAPABILITIES) {
>   ksmbd_debug(SMB,
>      "deassemble SMB2_COMPRESSION_CAPABILITIES context\n");
> @@ -1021,9 +1034,10 @@ static __le32 deassemble_neg_contexts(struct
> ksmbd_conn *conn,
>   } else if (pctx->ContextType == SMB2_SIGNING_CAPABILITIES) {
>   ksmbd_debug(SMB,
>      "deassemble SMB2_SIGNING_CAPABILITIES context\n");
> +
>   decode_sign_cap_ctxt(conn,
>       (struct smb2_signing_capabilities *)pctx,
> -     len_of_ctxts);
> +     ctxt_len);
>   }
>
>   /* offsets must be 8 byte aligned */
> ---
>
> Namjae Jeon <linkinjeon@kernel.org> 於 2023年5月18日 週四 下午2:36寫道：
>>
>> 2023-05-18 15:30 GMT+09:00, Hex Rabbit <h3xrabbit@gmail.com>:
>> >> You need to consider Ciphers flex-array size to validate ctxt_len. we
>> >> can get its size using CipherCount in smb2_encryption_neg_context.
>> >
>> > I'm not checking the flex-array size since both
>> > `decode_sign_cap_ctxt()`
>> > and `decode_encrypt_ctxt()` have done it, or should I move it out?
>> Yes, We can move it out. Thanks.
>> >
>> > ```
>> > if (sizeof(struct smb2_encryption_neg_context) + cphs_size >
>> >    len_of_ctxts) {
>> >     pr_err("Invalid cipher count(%d)\n", cph_cnt);
>> >     return;
>> > }
>> > ```
>> >
>> > ```
>> > if (sizeof(struct smb2_signing_capabilities) + sign_alos_size >
>> >    len_of_ctxts) {
>> >     pr_err("Invalid signing algorithm count(%d)\n", sign_algo_cnt);
>> >     return;
>> > }
>> > ```
>> >
>
diff mbox series

Patch

diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index 972176bff..83b877254 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -969,18 +969,16 @@  static __le32 deassemble_neg_contexts(struct ksmbd_conn *conn,
 	len_of_ctxts = len_of_smb - offset;
 
 	while (i++ < neg_ctxt_cnt) {
-		int clen;
-
-		/* check that offset is not beyond end of SMB */
-		if (len_of_ctxts == 0)
-			break;
+		int clen, ctxt_len;
 
 		if (len_of_ctxts < sizeof(struct smb2_neg_context))
 			break;
 
 		pctx = (struct smb2_neg_context *)((char *)pctx + offset);
 		clen = le16_to_cpu(pctx->DataLength);
-		if (clen + sizeof(struct smb2_neg_context) > len_of_ctxts)
+		ctxt_len = clen + sizeof(struct smb2_neg_context);
+
+		if (ctxt_len > len_of_ctxts)
 			break;
 
 		if (pctx->ContextType == SMB2_PREAUTH_INTEGRITY_CAPABILITIES) {
@@ -989,6 +987,9 @@  static __le32 deassemble_neg_contexts(struct ksmbd_conn *conn,
 			if (conn->preauth_info->Preauth_HashId)
 				break;
 
+			if (ctxt_len < sizeof(struct smb2_preauth_neg_context))
+				break;
+
 			status = decode_preauth_ctxt(conn,
 						     (struct smb2_preauth_neg_context *)pctx,
 						     len_of_ctxts);
@@ -1000,6 +1001,9 @@  static __le32 deassemble_neg_contexts(struct ksmbd_conn *conn,
 			if (conn->cipher_type)
 				break;
 
+			if (ctxt_len < sizeof(struct smb2_encryption_neg_context))
+				break;
+
 			decode_encrypt_ctxt(conn,
 					    (struct smb2_encryption_neg_context *)pctx,
 					    len_of_ctxts);
@@ -1009,6 +1013,9 @@  static __le32 deassemble_neg_contexts(struct ksmbd_conn *conn,
 			if (conn->compress_algorithm)
 				break;
 
+			if (ctxt_len < sizeof(struct smb2_compression_capabilities_context))
+				break;
+
 			decode_compress_ctxt(conn,
 					     (struct smb2_compression_capabilities_context *)pctx);
 		} else if (pctx->ContextType == SMB2_NETNAME_NEGOTIATE_CONTEXT_ID) {
@@ -1021,6 +1028,10 @@  static __le32 deassemble_neg_contexts(struct ksmbd_conn *conn,
 		} else if (pctx->ContextType == SMB2_SIGNING_CAPABILITIES) {
 			ksmbd_debug(SMB,
 				    "deassemble SMB2_SIGNING_CAPABILITIES context\n");
+
+			if (ctxt_len < sizeof(struct smb2_signing_capabilities))
+				break;
+
 			decode_sign_cap_ctxt(conn,
 					     (struct smb2_signing_capabilities *)pctx,
 					     len_of_ctxts);