[v6,09/14] ksmbd: check PDU len is at least header plus body size in ksmbd_smb2_check_message()

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(+)

2021-10-02 22:12 GMT+09:00, 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;
The point I'm talking about is an issue caused by you moving the
header buffer size here first( and you remove header buffer size check
in ksmbd_conn_handler_loop().)
When I apply patch till 09/14 number, check_user_session() and
get_ksmbd_tcon() is still has problem that access header without
header buffer size check.

And there still are functions to access header without header buffer
size check. Please check allocate_rsp_buf(), is_transform_hdr() and
init_rsp_hdr() in __handle_ksmbd_work().

> +
>  	if (hdr->StructureSize != SMB2_HEADER_STRUCTURE_SIZE) {
>  		ksmbd_debug(SMB, "Illegal structure size %u\n",
>  			    le16_to_cpu(hdr->StructureSize));
> --
> 2.31.1
>
>

Am 03.10.21 um 03:11 schrieb Namjae Jeon:
> 2021-10-02 22:12 GMT+09:00, 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;
> The point I'm talking about is an issue caused by you moving the
> header buffer size here first( and you remove header buffer size check
> in ksmbd_conn_handler_loop().)
> When I apply patch till 09/14 number, check_user_session() and
> get_ksmbd_tcon() is still has problem that access header without
> header buffer size check.
> 
> And there still are functions to access header without header buffer
> size check. Please check allocate_rsp_buf(), is_transform_hdr() and
> init_rsp_hdr() in __handle_ksmbd_work().

oh, that's a problem, missed that, sorry. Looks like we have to keep the 
validation at this layer. :(

-slow