Re: [PATCH 1/3] ksmbd: use buf_data_size instead of recalculation in smb3_decrypt_req()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2021-10-04 20:14 GMT+09:00, Hyunchul Lee <hyc.lee@xxxxxxxxx>:
> 2021년 10월 4일 (월) 오후 5:58, Namjae Jeon <linkinjeon@xxxxxxxxxx>님이 작성:
>
>>
>> 2021-10-04 17:38 GMT+09:00, Hyunchul Lee <hyc.lee@xxxxxxxxx>:
>> > 2021년 10월 3일 (일) 오후 1:31, Namjae Jeon <linkinjeon@xxxxxxxxxx>님이 작성:
>> >>
>> >> Tom suggested to use buf_data_size that is already calculated, to
>> >> verify
>> >> these offsets.
>> >>
>> >> Cc: Tom Talpey <tom@xxxxxxxxxx>
>> >> Cc: Ronnie Sahlberg <ronniesahlberg@xxxxxxxxx>
>> >> Cc: Ralph Böhme <slow@xxxxxxxxx>
>> >> Cc: Steve French <smfrench@xxxxxxxxx>
>> >> Cc: Sergey Senozhatsky <senozhatsky@xxxxxxxxxxxx>
>> >> Cc: Hyunchul Lee <hyc.lee@xxxxxxxxx>
>> >> Suggested-by: Tom Talpey <tom@xxxxxxxxxx>
>> >> Signed-off-by: Namjae Jeon <linkinjeon@xxxxxxxxxx>
>> >> ---
>> >>  fs/ksmbd/smb2pdu.c | 6 ++----
>> >>  1 file changed, 2 insertions(+), 4 deletions(-)
>> >>
>> >> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
>> >> index b06361313889..4d1be224dd8e 100644
>> >> --- a/fs/ksmbd/smb2pdu.c
>> >> +++ b/fs/ksmbd/smb2pdu.c
>> >> @@ -8457,15 +8457,13 @@ int smb3_decrypt_req(struct ksmbd_work *work)
>> >>         struct smb2_transform_hdr *tr_hdr = (struct smb2_transform_hdr
>> >> *)buf;
>> >>         int rc = 0;
>> >>
>> >> -       if (pdu_length + 4 <
>> >> -           sizeof(struct smb2_transform_hdr) + sizeof(struct
>> >> smb2_hdr))
>> >> {
>> >> +       if (buf_data_size < sizeof(struct smb2_hdr)) {
>> >
>> > Could integer overflow occur when buf_data_size is initialized?
>> > buf_data_size is initialized with "pdu_length + 4 -
>> > sizeof(struct smb2_transform_hdr)".
>> overflow does not occur. See the comments below.
>> >
>
> Ah, I am worried that pdu_length + 4 is less than
> sizeof(struct smb2_transform_hdr). And I can't find the check
> that pdu size is enough before this function is called.
Got it, I will change data type of buf_data_size to signed on next version.

Thanks!
>
>
>> > There was the check that the pdu size is greater than at least
>> > __SMB2_HEADER_STRUCTURE_SIZE at ksmbd_conn_handler_loop(),
>> > But I can't find this check in the latest patch set.
>> Please check "ksmbd: add the check to vaildate if stream protocol
>> length exceeds maximum value". pdu_length will never exceed
>> MAX_STREAM_PROT_LEN(0x00FFFFFF).
>>
>> Thanks!
>> >
>> >
>> >>                 pr_err("Transform message is too small (%u)\n",
>> >>                        pdu_length);
>> >>                 return -ECONNABORTED;
>> >>         }
>> >>
>> >> -       if (pdu_length + 4 <
>> >> -           le32_to_cpu(tr_hdr->OriginalMessageSize) + sizeof(struct
>> >> smb2_transform_hdr)) {
>> >> +       if (buf_data_size < le32_to_cpu(tr_hdr->OriginalMessageSize))
>> >> {
>> >>                 pr_err("Transform message is broken\n");
>> >>                 return -ECONNABORTED;
>> >>         }
>> >> --
>> >> 2.25.1
>> >>
>> >
>> >
>> > --
>> > Thanks,
>> > Hyunchul
>> >
>
>
>
> --
> Thanks,
> Hyunchul
>




[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux