All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hyunchul Lee <hyc.lee@gmail.com>
To: Namjae Jeon <linkinjeon@kernel.org>
Cc: linux-cifs <linux-cifs@vger.kernel.org>,
	"Ronnie Sahlberg" <lsahlber@redhat.com>,
	"Tom Talpey" <tom@talpey.com>,
	"Ronnie Sahlberg" <ronniesahlberg@gmail.com>,
	"Ralph Böhme" <slow@samba.org>,
	"Steve French" <smfrench@gmail.com>,
	"Sergey Senozhatsky" <senozhatsky@chromium.org>
Subject: Re: [PATCH 3/7] ksmbd: remove RFC1002 check in smb2 request
Date: Sat, 25 Sep 2021 17:18:57 +0900	[thread overview]
Message-ID: <CANFS6bYm+=zQhh1VcZ=PvWuuQ35Pafk7O+dAPsb8yFD9quh8OA@mail.gmail.com> (raw)
In-Reply-To: <20210924021254.27096-4-linkinjeon@kernel.org>

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

2021년 9월 24일 (금) 오전 11:13, Namjae Jeon <linkinjeon@kernel.org>님이 작성:
>
> From: Ronnie Sahlberg <lsahlber@redhat.com>
>
> In smb_common.c you have this function :   ksmbd_smb_request() which
> is called from connection.c once you have read the initial 4 bytes for
> the next length+smb2 blob.
>
> It checks the first byte of this 4 byte preamble for valid values,
> i.e. a NETBIOSoverTCP SESSION_MESSAGE or a SESSION_KEEP_ALIVE.
>
> We don't need to check this for ksmbd since it only implements SMB2
> over TCP port 445.
> The netbios stuff was only used in very old servers when SMB ran over
> TCP port 139.
> Now that we run over TCP port 445, this is actually not a NB header anymore
> and you can just treat it as a 4 byte length field that must be less
> than 16Mbyte. and remove the references to the RFC1002 constants that no
> longer applies.
>
> 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: Hyunchul Lee <hyc.lee@gmail.com>
> Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
> ---
>  fs/ksmbd/smb_common.c | 15 +--------------
>  fs/ksmbd/smb_common.h |  8 --------
>  2 files changed, 1 insertion(+), 22 deletions(-)
>
> diff --git a/fs/ksmbd/smb_common.c b/fs/ksmbd/smb_common.c
> index 40f4fafa2e11..5901b2884c60 100644
> --- a/fs/ksmbd/smb_common.c
> +++ b/fs/ksmbd/smb_common.c
> @@ -155,20 +155,7 @@ int ksmbd_verify_smb_message(struct ksmbd_work *work)
>   */
>  bool ksmbd_smb_request(struct ksmbd_conn *conn)
>  {
> -       int type = *(char *)conn->request_buf;
> -
> -       switch (type) {
> -       case RFC1002_SESSION_MESSAGE:
> -               /* Regular SMB request */
> -               return true;
> -       case RFC1002_SESSION_KEEP_ALIVE:
> -               ksmbd_debug(SMB, "RFC 1002 session keep alive\n");
> -               break;
> -       default:
> -               ksmbd_debug(SMB, "RFC 1002 unknown request type 0x%x\n", type);
> -       }
> -
> -       return false;
> +       return conn->request_buf[0] == 0;
>  }
>
>  static bool supported_protocol(int idx)
> diff --git a/fs/ksmbd/smb_common.h b/fs/ksmbd/smb_common.h
> index 0a6af447cc45..994abede27e9 100644
> --- a/fs/ksmbd/smb_common.h
> +++ b/fs/ksmbd/smb_common.h
> @@ -48,14 +48,6 @@
>  #define CIFS_DEFAULT_IOSIZE    (64 * 1024)
>  #define MAX_CIFS_SMALL_BUFFER_SIZE 448 /* big enough for most */
>
> -/* RFC 1002 session packet types */
> -#define RFC1002_SESSION_MESSAGE                        0x00
> -#define RFC1002_SESSION_REQUEST                        0x81
> -#define RFC1002_POSITIVE_SESSION_RESPONSE      0x82
> -#define RFC1002_NEGATIVE_SESSION_RESPONSE      0x83
> -#define RFC1002_RETARGET_SESSION_RESPONSE      0x84
> -#define RFC1002_SESSION_KEEP_ALIVE             0x85
> -
>  /* Responses when opening a file. */
>  #define F_SUPERSEDED   0
>  #define F_OPENED       1
> --
> 2.25.1
>


-- 
Thanks,
Hyunchul

  reply	other threads:[~2021-09-25  8:25 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-24  2:12 [PATCH 0/7] a bunch of patches that have not yet been reviewed Namjae Jeon
2021-09-24  2:12 ` [PATCH 1/7] ksmbd: add validation in smb2_ioctl Namjae Jeon
2021-09-25 10:16   ` Hyunchul Lee
2021-09-25 10:44     ` Namjae Jeon
2021-09-24  2:12 ` [PATCH 2/7] ksmbd: add request buffer validation in smb2_set_info Namjae Jeon
2021-09-25  8:13   ` Hyunchul Lee
2021-09-25  9:19     ` Namjae Jeon
2021-09-24  2:12 ` [PATCH 3/7] ksmbd: remove RFC1002 check in smb2 request Namjae Jeon
2021-09-25  8:18   ` Hyunchul Lee [this message]
2021-09-24  2:12 ` [PATCH 4/7] ksmbd: check strictly data area in ksmbd_smb2_check_message() Namjae Jeon
2021-09-25 10:27   ` Hyunchul Lee
2021-09-25 10:46     ` Namjae Jeon
2021-09-24  2:12 ` [PATCH 5/7] ksmbd: add the check to vaildate if stream protocol length exceeds maximum value Namjae Jeon
2021-09-25  8:41   ` Hyunchul Lee
2021-09-25  9:24     ` Namjae Jeon
2021-09-24  2:12 ` [PATCH 6/7] ksmbd: fix invalid request buffer access in compound Namjae Jeon
2021-09-25  9:41   ` Hyunchul Lee
2021-09-24  2:12 ` [PATCH 7/7] ksmbd: add validation in smb2 negotiate Namjae Jeon
2021-09-24  4:58   ` Namjae Jeon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CANFS6bYm+=zQhh1VcZ=PvWuuQ35Pafk7O+dAPsb8yFD9quh8OA@mail.gmail.com' \
    --to=hyc.lee@gmail.com \
    --cc=linkinjeon@kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=lsahlber@redhat.com \
    --cc=ronniesahlberg@gmail.com \
    --cc=senozhatsky@chromium.org \
    --cc=slow@samba.org \
    --cc=smfrench@gmail.com \
    --cc=tom@talpey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.