Re: [PATCH v3] ksmbd: fix invalid request buffer access in compound request

[Namjae Jeon <linkinjeon@kernel.org>]

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
>
>