On Wed, May 17, 2023 at 06:58:20PM +0000, HexRabbit wrote:
From: Kuan-Ting Chen <h3xrabbit@xxxxxxxxx>
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@xxxxxxxxx>
---
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.
