[PATCH v2] ksmbd: use buf_data_size instead of recalculation in smb3_decrypt_req()

[PATCH v2] ksmbd: use buf_data_size instead of recalculation in smb3_decrypt_req()

[Namjae Jeon]

Tom suggested to use buf_data_size that is already calculated, to verify
these offsets.

Cc: Tom Talpey <tom@talpey.com>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Ralph Böhme <slow@samba.org>
Cc: Steve French <smfrench@gmail.com>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
Cc: Hyunchul Lee <hyc.lee@gmail.com>
Suggested-by: Tom Talpey <tom@talpey.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 v2:
   - change data type of buf_data_size to signed to validate
     smb2_transfrom_hdr size.
 fs/ksmbd/smb2pdu.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index b06361313889..bb030e4366ad 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -8452,20 +8452,18 @@ int smb3_decrypt_req(struct ksmbd_work *work)
 	struct smb2_hdr *hdr;
 	unsigned int pdu_length = get_rfc1002_len(buf);
 	struct kvec iov[2];
-	unsigned int buf_data_size = pdu_length + 4 -
+	int buf_data_size = pdu_length + 4 -
 		sizeof(struct smb2_transform_hdr);
 	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)) {
 		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

Re: [PATCH v2] ksmbd: use buf_data_size instead of recalculation in smb3_decrypt_req()

[Hyunchul Lee]

Looks good to me.
Acked-by: Hyunchul Lee <hyc.lee@gmail.com>

2021년 10월 5일 (화) 오전 10:20, Namjae Jeon <linkinjeon@kernel.org>님이 작성:

>
> Tom suggested to use buf_data_size that is already calculated, to verify
> these offsets.
>
> Cc: Tom Talpey <tom@talpey.com>
> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> Cc: Ralph Böhme <slow@samba.org>
> Cc: Steve French <smfrench@gmail.com>
> Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
> Cc: Hyunchul Lee <hyc.lee@gmail.com>
> Suggested-by: Tom Talpey <tom@talpey.com>
> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
> ---
>  v2:
>    - change data type of buf_data_size to signed to validate
>      smb2_transfrom_hdr size.
>  fs/ksmbd/smb2pdu.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
> index b06361313889..bb030e4366ad 100644
> --- a/fs/ksmbd/smb2pdu.c
> +++ b/fs/ksmbd/smb2pdu.c
> @@ -8452,20 +8452,18 @@ int smb3_decrypt_req(struct ksmbd_work *work)
>         struct smb2_hdr *hdr;
>         unsigned int pdu_length = get_rfc1002_len(buf);
>         struct kvec iov[2];
> -       unsigned int buf_data_size = pdu_length + 4 -
> +       int buf_data_size = pdu_length + 4 -
>                 sizeof(struct smb2_transform_hdr);
>         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)) {
>                 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