Re: [PATCH v5 16/20] ksmbd: check PDU len is at least header plus body size in ksmbd_smb2_check_message()

[Namjae Jeon <linkinjeon@kernel.org>]

2021-10-02 21:49 GMT+09:00, Ralph Boehme <slow@samba.org>:
> Am 02.10.21 um 14:45 schrieb Hyunchul Lee:
>> Hi Ralph,
>>
>> 2021년 10월 1일 (금) 오후 9:25, Ralph Boehme <slow@samba.org>님이 작성:
>>>
>>> Note: we already have the same check in is_chained_smb2_message(), but
>>> there it
>>> only applies to compound requests, so we have to repeat the check here to
>>> cover
>>> both cases.
>>>
>>> Cc: Namjae Jeon <linkinjeon@kernel.org>
>>> Cc: Tom Talpey <tom@talpey.com>
>>> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
>>> Cc: Steve French <smfrench@gmail.com>
>>> Cc: Hyunchul Lee <hyc.lee@gmail.com>
>>> Signed-off-by: Ralph Boehme <slow@samba.org>
>>> ---
>>>   fs/ksmbd/smb2misc.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/fs/ksmbd/smb2misc.c b/fs/ksmbd/smb2misc.c
>>> index 7ed266eb6c5e..541b39b7a84b 100644
>>> --- a/fs/ksmbd/smb2misc.c
>>> +++ b/fs/ksmbd/smb2misc.c
>>> @@ -338,6 +338,9 @@ int ksmbd_smb2_check_message(struct ksmbd_work
>>> *work)
>>>          if (check_smb2_hdr(hdr))
>>>                  return 1;
>>>
>>> +       if (len < sizeof(struct smb2_pdu) - 4)
>>> +               return 1;
>>> +
>>
>> Do we need this check before accessing any fields of smb2_hdr in
>> ksmbd_verify_smb_message()?
>
> well, my idea was to have the core PDU size checking logic in
> ksmbd_smb2_check_message() and ksmbd_verify_smb_message() merely
> switches between SMB1/SMB2.
Hyunchul  pointed out that this header buffer check should be moved to
the above check_smb2_hdr(). I think that it should be move to the
above ksmbd_smb2_cur_pdu_buflen().

>
> -slow
>
> --
> Ralph Boehme, Samba Team                 https://samba.org/
> SerNet Samba Team Lead      https://sernet.de/en/team-samba
>