All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steve French <smfrench@gmail.com>
To: Namjae Jeon <linkinjeon@kernel.org>
Cc: 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>,
	"Sergey Senozhatsky" <senozhatsky@chromium.org>,
	"Hyunchul Lee" <hyc.lee@gmail.com>
Subject: Re: [PATCH v2 1/7] ksmbd: remove RFC1002 check in smb2 request
Date: Sat, 25 Sep 2021 22:21:21 -0500	[thread overview]
Message-ID: <CAH2r5msWH=mk_H2ajsKuN8N5ufWPt1zKhnEpbwRu52WxqJG+QA@mail.gmail.com> (raw)
In-Reply-To: <20210925131625.29617-2-linkinjeon@kernel.org>

Also reviewed by me, and merged into cifsd-for-next

On Sat, Sep 25, 2021 at 8:17 AM Namjae Jeon <linkinjeon@kernel.org> wrote:
>
> 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: Sergey Senozhatsky <senozhatsky@chromium.org>
> Acked-by: Hyunchul Lee <hyc.lee@gmail.com>
> 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 43d3123d8b62..1da67217698d 100644
> --- a/fs/ksmbd/smb_common.c
> +++ b/fs/ksmbd/smb_common.c
> @@ -149,20 +149,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 57c667c1be06..d7df19c97c4c 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,

Steve

  reply	other threads:[~2021-09-26  3:21 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-25 13:16 [PATCH v2 0/7] ksmbd: a bunch of patches Namjae Jeon
2021-09-25 13:16 ` [PATCH v2 1/7] ksmbd: remove RFC1002 check in smb2 request Namjae Jeon
2021-09-26  3:21   ` Steve French [this message]
2021-09-25 13:16 ` [PATCH v2 2/7] ksmbd: add the check to vaildate if stream protocol length exceeds maximum value Namjae Jeon
2021-09-25 13:16 ` [PATCH v2 3/7] ksmbd: add validation in smb2_ioctl Namjae Jeon
2021-09-26  3:26   ` Namjae Jeon
2021-09-25 13:16 ` [PATCH v2 4/7] ksmbd: add request buffer validation in smb2_set_info Namjae Jeon
2021-09-25 13:16 ` [PATCH v2 5/7] ksmbd: check strictly data area in ksmbd_smb2_check_message() Namjae Jeon
2021-09-25 13:16 ` [PATCH v2 6/7] ksmbd: fix invalid request buffer access in compound Namjae Jeon
2021-09-25 13:16 ` [PATCH v2 7/7] ksmbd: add validation in smb2 negotiate 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='CAH2r5msWH=mk_H2ajsKuN8N5ufWPt1zKhnEpbwRu52WxqJG+QA@mail.gmail.com' \
    --to=smfrench@gmail.com \
    --cc=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=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.