All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steve French <smfrench@gmail.com>
To: ronnie sahlberg <ronniesahlberg@gmail.com>
Cc: "Namjae Jeon" <linkinjeon@kernel.org>,
	linux-cifs <linux-cifs@vger.kernel.org>,
	"Ralph Böhme" <slow@samba.org>,
	"Ronnie Sahlberg" <lsahlber@redhat.com>
Subject: Re: [PATCH] ksmbd: check protocol id in ksmbd_verify_smb_message()
Date: Wed, 22 Sep 2021 17:24:12 -0500	[thread overview]
Message-ID: <CAH2r5mvaDzXeuooUq+_GkR_F5EeSPrt-yUGjPsfR=MVbv6joMw@mail.gmail.com> (raw)
In-Reply-To: <CAN05THQVgu33LmFx5u3xm7MjdJZMYe81-bJEvAVJLPrMkjYYZg@mail.gmail.com>

added the R-Bs and merged into cifsd-for-next  (current content is
below, although looks like we could update the "buffer invalidation in
smb2_set_info" patch)

e6201b4a0bac (HEAD -> cifsd-for-next, origin/cifsd-for-next) ksmbd:
add request buffer validation in smb2_set_info
743d886affeb ksmbd: remove follow symlinks support
3bee78ad0062 ksmbd: fix invalid request buffer access in compound request
18a015bccf9e ksmbd: check protocol id in ksmbd_verify_smb_message()
9f6323311c70 ksmbd: add default data stream name in FILE_STREAM_INFORMATION
e44fd5081c50 ksmbd: log that server is experimental at module load

On Wed, Sep 22, 2021 at 4:33 PM ronnie sahlberg
<ronniesahlberg@gmail.com> wrote:
>
> reviewed by me
>
> On Wed, Sep 22, 2021 at 10:01 PM Namjae Jeon <linkinjeon@kernel.org> wrote:
> >
> > When second smb2 pdu has invalid protocol id, ksmbd doesn't detect it
> > and allow to process smb2 request. This patch add the check it in
> > ksmbd_verify_smb_message() and don't use protocol id of smb2 request as
> > protocol id of response.
> >
> > Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> > Cc: Ralph Böhme <slow@samba.org>
> > Cc: Steve French <smfrench@gmail.com>
> > Reported-by: Ronnie Sahlberg <lsahlber@redhat.com>
> > Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
> > ---
> >  fs/ksmbd/smb2pdu.c    |  2 +-
> >  fs/ksmbd/smb_common.c | 13 +++++++++----
> >  fs/ksmbd/smb_common.h |  1 +
> >  3 files changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
> > index 3d250e2539e6..3be1493cb18d 100644
> > --- a/fs/ksmbd/smb2pdu.c
> > +++ b/fs/ksmbd/smb2pdu.c
> > @@ -433,7 +433,7 @@ static void init_chained_smb2_rsp(struct ksmbd_work *work)
> >                 work->compound_pfid = KSMBD_NO_FID;
> >         }
> >         memset((char *)rsp_hdr + 4, 0, sizeof(struct smb2_hdr) + 2);
> > -       rsp_hdr->ProtocolId = rcv_hdr->ProtocolId;
> > +       rsp_hdr->ProtocolId = SMB2_PROTO_NUMBER;
> >         rsp_hdr->StructureSize = SMB2_HEADER_STRUCTURE_SIZE;
> >         rsp_hdr->Command = rcv_hdr->Command;
> >
> > diff --git a/fs/ksmbd/smb_common.c b/fs/ksmbd/smb_common.c
> > index da17b21ac685..ace8a1b02c81 100644
> > --- a/fs/ksmbd/smb_common.c
> > +++ b/fs/ksmbd/smb_common.c
> > @@ -129,16 +129,22 @@ int ksmbd_lookup_protocol_idx(char *str)
> >   *
> >   * check for valid smb signature and packet direction(request/response)
> >   *
> > - * Return:      0 on success, otherwise 1
> > + * Return:      0 on success, otherwise -EINVAL
> >   */
> >  int ksmbd_verify_smb_message(struct ksmbd_work *work)
> >  {
> > -       struct smb2_hdr *smb2_hdr = work->request_buf;
> > +       struct smb2_hdr *smb2_hdr = work->request_buf + work->next_smb2_rcv_hdr_off;
> > +       struct smb_hdr *hdr;
> >
> >         if (smb2_hdr->ProtocolId == SMB2_PROTO_NUMBER)
> >                 return ksmbd_smb2_check_message(work);
> >
> > -       return 0;
> > +       hdr = work->request_buf;
> > +       if (*(__le32 *)hdr->Protocol == SMB1_PROTO_NUMBER &&
> > +           hdr->Command == SMB_COM_NEGOTIATE)
> > +               return 0;
> > +
> > +       return -EINVAL;
> >  }
> >
> >  /**
> > @@ -270,7 +276,6 @@ static int ksmbd_negotiate_smb_dialect(void *buf)
> >         return BAD_PROT_ID;
> >  }
> >
> > -#define SMB_COM_NEGOTIATE      0x72
> >  int ksmbd_init_smb_server(struct ksmbd_work *work)
> >  {
> >         struct ksmbd_conn *conn = work->conn;
> > diff --git a/fs/ksmbd/smb_common.h b/fs/ksmbd/smb_common.h
> > index d7df19c97c4c..994abede27e9 100644
> > --- a/fs/ksmbd/smb_common.h
> > +++ b/fs/ksmbd/smb_common.h
> > @@ -202,6 +202,7 @@
> >                 FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES)
> >
> >  #define SMB1_PROTO_NUMBER              cpu_to_le32(0x424d53ff)
> > +#define SMB_COM_NEGOTIATE              0x72
> >
> >  #define SMB1_CLIENT_GUID_SIZE          (16)
> >  struct smb_hdr {
> > --
> > 2.25.1
> >



-- 
Thanks,

Steve

      reply	other threads:[~2021-09-22 22:24 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-22 12:00 Namjae Jeon
2021-09-22 14:21 ` Ralph Boehme
2021-09-22 21:33 ` ronnie sahlberg
2021-09-22 22:24   ` Steve French [this message]

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='CAH2r5mvaDzXeuooUq+_GkR_F5EeSPrt-yUGjPsfR=MVbv6joMw@mail.gmail.com' \
    --to=smfrench@gmail.com \
    --cc=linkinjeon@kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=lsahlber@redhat.com \
    --cc=ronniesahlberg@gmail.com \
    --cc=slow@samba.org \
    --subject='Re: [PATCH] ksmbd: check protocol id in ksmbd_verify_smb_message()' \
    /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

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.