2021-09-30 22:33 GMT+09:00, Ralph Boehme <slow@xxxxxxxxx>:
> Am 30.09.21 um 15:17 schrieb Namjae Jeon:
>> 2021-09-30 21:53 GMT+09:00, Ralph Boehme <slow@xxxxxxxxx>:
>>> yes, this was next on my list, sorry forgot to mention this. Afaict in
>>> the current code the session and tcon checks are only done once on the
>>> first SMB2 PDU for a series of compound non-related PDUs, while for
>>> non-related PDUs the calls to check_user_session() and
>>> smb2_get_ksmbd_tcon() should be probably be done inside
>>> __process_request(), or eventually just inside
>>> ksmbd_smb2_check_message().
>> check_user_session and get_ksmbd_tcon should not be moved inside
>> __process_request()
>> because session id and tree id of related pdu is 0xFFFFFFFFFFFFFFFF
>> and 0xFFFFFFFF.
>
> of course, but that must just be handled by those functions. I'm
> currently working on tentative fix for this.
1. You need to check header size of related pdu of compound request is
already checked in the is_chained_smb2_message function.
is_chained_smb2_message()
...
if (next_cmd > 0) {
if ((u64)work->next_smb2_rcv_hdr_off + next_cmd +
__SMB2_HEADER_STRUCTURE_SIZE >
get_rfc1002_len(work->request_buf)) {
pr_err("next command(%u) offset exceeds smb msg size\n",
next_cmd);
return false;
}
2. session id and tree id only needs to be checked in the header of
the first pdu regardless of compound and single one.
So I don't know what would be better if I moved it.
Thanks!
>
>>
>>>
>>>> is_chained_smb2_message is
>>>> checking next command header size.
>>>>>
>>>>> You also dropped the fix for the possible invalid read in
>>>>> ksmbd_verify_smb_message() of the protocol_id field.
>>>> Where ? You are saying your patch in github ? If it is right, I didn't
>>>> drop.
>>>
>>> this one:
>>>
>>> <https://github.com/smfrench/smb3-kernel/commit/ffc410f1d19a0f06a952c7f28e9bca4fa4bd4a74>
>> Ah.. You pushed this patch to ksmbd-for-next-pending ?
>> Sorry for that, my mistake, I will check branch before applying my patch.
>
> Yeah, the whole patchset is in the branch ksmbd-for-next-pending which
> is actually the one you correctly used for the comments on github. :)
Ah.. okay.
I will carefully check it next time.
Thanks!
>
> Cheers!
> -slow
>
> --
> Ralph Boehme, Samba Team https://samba.org/
> SerNet Samba Team Lead https://sernet.de/en/team-samba
>
