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


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