| Message ID | 20210922120057.45789-1-linkinjeon@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ksmbd: check protocol id in ksmbd_verify_smb_message() | expand |
Am 22.09.21 um 14:00 schrieb Namjae Jeon: > 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> reviewed-by: me. -slow
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 >
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 > >
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 {
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(-)