* [PATCH v3] ksmbd: fix invalid request buffer access in compound request
@ 2021-09-22 12:01 Namjae Jeon
2021-09-22 14:23 ` Ralph Boehme
0 siblings, 1 reply; 3+ messages in thread
From: Namjae Jeon @ 2021-09-22 12:01 UTC (permalink / raw)
To: linux-cifs
Cc: Namjae Jeon, Ronnie Sahlberg, Ralph Böhme, Steve French,
Ronnie Sahlberg
Ronnie reported invalid request buffer access in chained command when
inserting garbage value to NextCommand of compound request.
This patch add validation check to avoid this issue.
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Ralph Böhme <slow@samba.org>
Cc: Steve French <smfrench@gmail.com>
Reported-by: Ronnie Sahlberg <lsahlber@redhat.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
v2:
- fix integer overflow from work->next_smb2_rcv_hdr_off.
v3:
- check next command offset and at least header size of next pdu at
the same time.
fs/ksmbd/smb2pdu.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index 4f11eb85bb6b..3d250e2539e6 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -466,6 +466,13 @@ bool is_chained_smb2_message(struct ksmbd_work *work)
hdr = ksmbd_req_buf_next(work);
if (le32_to_cpu(hdr->NextCommand) > 0) {
+ if ((u64)work->next_smb2_rcv_hdr_off + le32_to_cpu(hdr->NextCommand) + 64 >
+ get_rfc1002_len(work->request_buf)) {
+ pr_err("next command(%u) offset exceeds smb msg size\n",
+ le32_to_cpu(hdr->NextCommand));
+ return false;
+ }
+
ksmbd_debug(SMB, "got SMB2 chained command\n");
init_chained_smb2_rsp(work);
return true;
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v3] ksmbd: fix invalid request buffer access in compound request
2021-09-22 12:01 [PATCH v3] ksmbd: fix invalid request buffer access in compound request Namjae Jeon
@ 2021-09-22 14:23 ` Ralph Boehme
2021-09-22 23:15 ` Namjae Jeon
0 siblings, 1 reply; 3+ messages in thread
From: Ralph Boehme @ 2021-09-22 14:23 UTC (permalink / raw)
To: Namjae Jeon, linux-cifs; +Cc: Ronnie Sahlberg, Steve French, Ronnie Sahlberg
[-- Attachment #1.1: Type: text/plain, Size: 1392 bytes --]
Am 22.09.21 um 14:01 schrieb Namjae Jeon:
> Ronnie reported invalid request buffer access in chained command when
> inserting garbage value to NextCommand of compound request.
> This patch add validation check to avoid this issue.
>
> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> Cc: Ralph Böhme <slow@samba.org>
> Cc: Steve French <smfrench@gmail.com>
> Reported-by: Ronnie Sahlberg <lsahlber@redhat.com>
> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
> ---
> v2:
> - fix integer overflow from work->next_smb2_rcv_hdr_off.
> v3:
> - check next command offset and at least header size of next pdu at
> the same time.
> fs/ksmbd/smb2pdu.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
> index 4f11eb85bb6b..3d250e2539e6 100644
> --- a/fs/ksmbd/smb2pdu.c
> +++ b/fs/ksmbd/smb2pdu.c
> @@ -466,6 +466,13 @@ bool is_chained_smb2_message(struct ksmbd_work *work)
>
> hdr = ksmbd_req_buf_next(work);
> if (le32_to_cpu(hdr->NextCommand) > 0) {
> + if ((u64)work->next_smb2_rcv_hdr_off + le32_to_cpu(hdr->NextCommand) + 64 >
> + get_rfc1002_len(work->request_buf)) {
is this safe from overflows on 32 bit arch?
Thanks!
-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] 3+ messages in thread* Re: [PATCH v3] ksmbd: fix invalid request buffer access in compound request
2021-09-22 14:23 ` Ralph Boehme
@ 2021-09-22 23:15 ` Namjae Jeon
0 siblings, 0 replies; 3+ messages in thread
From: Namjae Jeon @ 2021-09-22 23:15 UTC (permalink / raw)
To: Ralph Boehme; +Cc: linux-cifs, Ronnie Sahlberg, Steve French, Ronnie Sahlberg
2021-09-22 23:23 GMT+09:00, Ralph Boehme <slow@samba.org>:
> Am 22.09.21 um 14:01 schrieb Namjae Jeon:
>> Ronnie reported invalid request buffer access in chained command when
>> inserting garbage value to NextCommand of compound request.
>> This patch add validation check to avoid this issue.
>>
>> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
>> Cc: Ralph Böhme <slow@samba.org>
>> Cc: Steve French <smfrench@gmail.com>
>> Reported-by: Ronnie Sahlberg <lsahlber@redhat.com>
>> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
>> ---
>> v2:
>> - fix integer overflow from work->next_smb2_rcv_hdr_off.
>> v3:
>> - check next command offset and at least header size of next pdu at
>> the same time.
>> fs/ksmbd/smb2pdu.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
>> index 4f11eb85bb6b..3d250e2539e6 100644
>> --- a/fs/ksmbd/smb2pdu.c
>> +++ b/fs/ksmbd/smb2pdu.c
>> @@ -466,6 +466,13 @@ bool is_chained_smb2_message(struct ksmbd_work
>> *work)
>>
>> hdr = ksmbd_req_buf_next(work);
>> if (le32_to_cpu(hdr->NextCommand) > 0) {
>> + if ((u64)work->next_smb2_rcv_hdr_off + le32_to_cpu(hdr->NextCommand) +
>> 64 >
>> + get_rfc1002_len(work->request_buf)) {
>
> is this safe from overflows on 32 bit arch?
Okay, will fix it on next version.
Thanks for your review!
>
> Thanks!
> -slow
>
> --
> Ralph Boehme, Samba Team https://samba.org/
> SerNet Samba Team Lead https://sernet.de/en/team-samba
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-09-22 23:15 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-22 12:01 [PATCH v3] ksmbd: fix invalid request buffer access in compound request Namjae Jeon
2021-09-22 14:23 ` Ralph Boehme
2021-09-22 23:15 ` Namjae Jeon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.