diff options
| author | Namjae Jeon <linkinjeon@kernel.org> | 2023-05-27 18:23:09 +0300 |
|---|---|---|
| committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2023-06-14 12:15:33 +0300 |
| commit | bf12d7fb63b365fb766655cedcb5d5f292b0c35e (patch) | |
| tree | aed8a903dd9f78a717ae7a77bfd4428219d312f6 | |
| parent | fb322da83ce538bdce0f65029d1f0008ac70cda8 (diff) | |
| download | linux-bf12d7fb63b365fb766655cedcb5d5f292b0c35e.tar.xz | |
ksmbd: fix out-of-bound read in deassemble_neg_contexts()
commit f1a411873c85b642f13b01f21b534c2bab81fc1b upstream.
The check in the beginning is
`clen + sizeof(struct smb2_neg_context) <= len_of_ctxts`,
but in the end of loop, `len_of_ctxts` will subtract
`((clen + 7) & ~0x7) + sizeof(struct smb2_neg_context)`, which causes
integer underflow when clen does the 8 alignment. We should use
`(clen + 7) & ~0x7` in the check to avoid underflow from happening.
Then there are some variables that need to be declared unsigned
instead of signed.
[ 11.671070] BUG: KASAN: slab-out-of-bounds in smb2_handle_negotiate+0x799/0x1610
[ 11.671533] Read of size 2 at addr ffff888005e86cf2 by task kworker/0:0/7
...
[ 11.673383] Call Trace:
[ 11.673541] <TASK>
[ 11.673679] dump_stack_lvl+0x33/0x50
[ 11.673913] print_report+0xcc/0x620
[ 11.674671] kasan_report+0xae/0xe0
[ 11.675171] kasan_check_range+0x35/0x1b0
[ 11.675412] smb2_handle_negotiate+0x799/0x1610
[ 11.676217] ksmbd_smb_negotiate_common+0x526/0x770
[ 11.676795] handle_ksmbd_work+0x274/0x810
...
Cc: stable@vger.kernel.org
Signed-off-by: Chih-Yen Chang <cc85nod@gmail.com>
Tested-by: Chih-Yen Chang <cc85nod@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
| -rw-r--r-- | fs/ksmbd/smb2pdu.c | 13 |
1 files changed, 6 insertions, 7 deletions
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 01887570efe8..56f661e5ef62 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -979,13 +979,13 @@ static void decode_sign_cap_ctxt(struct ksmbd_conn *conn, static __le32 deassemble_neg_contexts(struct ksmbd_conn *conn, struct smb2_negotiate_req *req, - int len_of_smb) + unsigned int len_of_smb) { /* +4 is to account for the RFC1001 len field */ struct smb2_neg_context *pctx = (struct smb2_neg_context *)req; int i = 0, len_of_ctxts; - int offset = le32_to_cpu(req->NegotiateContextOffset); - int neg_ctxt_cnt = le16_to_cpu(req->NegotiateContextCount); + unsigned int offset = le32_to_cpu(req->NegotiateContextOffset); + unsigned int neg_ctxt_cnt = le16_to_cpu(req->NegotiateContextCount); __le32 status = STATUS_INVALID_PARAMETER; ksmbd_debug(SMB, "decoding %d negotiate contexts\n", neg_ctxt_cnt); @@ -999,7 +999,7 @@ static __le32 deassemble_neg_contexts(struct ksmbd_conn *conn, while (i++ < neg_ctxt_cnt) { int clen, ctxt_len; - if (len_of_ctxts < sizeof(struct smb2_neg_context)) + if (len_of_ctxts < (int)sizeof(struct smb2_neg_context)) break; pctx = (struct smb2_neg_context *)((char *)pctx + offset); @@ -1054,9 +1054,8 @@ static __le32 deassemble_neg_contexts(struct ksmbd_conn *conn, } /* offsets must be 8 byte aligned */ - clen = (clen + 7) & ~0x7; - offset = clen + sizeof(struct smb2_neg_context); - len_of_ctxts -= clen + sizeof(struct smb2_neg_context); + offset = (ctxt_len + 7) & ~0x7; + len_of_ctxts -= offset; } return status; } |