| Message ID | 20211002131212.130629-10-slow@samba.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Buffer validation patches | expand |
2021-10-02 22:12 GMT+09:00, Ralph Boehme <slow@samba.org>: > Note: we already have the same check in is_chained_smb2_message(), but there > it > only applies to compound requests, so we have to repeat the check here to > cover > both cases. > > Cc: Namjae Jeon <linkinjeon@kernel.org> > Cc: Tom Talpey <tom@talpey.com> > Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> > Cc: Steve French <smfrench@gmail.com> > Cc: Hyunchul Lee <hyc.lee@gmail.com> > Signed-off-by: Ralph Boehme <slow@samba.org> > --- > fs/ksmbd/smb2misc.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/ksmbd/smb2misc.c b/fs/ksmbd/smb2misc.c > index 7ed266eb6c5e..541b39b7a84b 100644 > --- a/fs/ksmbd/smb2misc.c > +++ b/fs/ksmbd/smb2misc.c > @@ -338,6 +338,9 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work) > if (check_smb2_hdr(hdr)) > return 1; > > + if (len < sizeof(struct smb2_pdu) - 4) > + return 1; The point I'm talking about is an issue caused by you moving the header buffer size here first( and you remove header buffer size check in ksmbd_conn_handler_loop().) When I apply patch till 09/14 number, check_user_session() and get_ksmbd_tcon() is still has problem that access header without header buffer size check. And there still are functions to access header without header buffer size check. Please check allocate_rsp_buf(), is_transform_hdr() and init_rsp_hdr() in __handle_ksmbd_work(). > + > if (hdr->StructureSize != SMB2_HEADER_STRUCTURE_SIZE) { > ksmbd_debug(SMB, "Illegal structure size %u\n", > le16_to_cpu(hdr->StructureSize)); > -- > 2.31.1 > >
Am 03.10.21 um 03:11 schrieb Namjae Jeon: > 2021-10-02 22:12 GMT+09:00, Ralph Boehme <slow@samba.org>: >> Note: we already have the same check in is_chained_smb2_message(), but there >> it >> only applies to compound requests, so we have to repeat the check here to >> cover >> both cases. >> >> Cc: Namjae Jeon <linkinjeon@kernel.org> >> Cc: Tom Talpey <tom@talpey.com> >> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> >> Cc: Steve French <smfrench@gmail.com> >> Cc: Hyunchul Lee <hyc.lee@gmail.com> >> Signed-off-by: Ralph Boehme <slow@samba.org> >> --- >> fs/ksmbd/smb2misc.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/fs/ksmbd/smb2misc.c b/fs/ksmbd/smb2misc.c >> index 7ed266eb6c5e..541b39b7a84b 100644 >> --- a/fs/ksmbd/smb2misc.c >> +++ b/fs/ksmbd/smb2misc.c >> @@ -338,6 +338,9 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work) >> if (check_smb2_hdr(hdr)) >> return 1; >> >> + if (len < sizeof(struct smb2_pdu) - 4) >> + return 1; > The point I'm talking about is an issue caused by you moving the > header buffer size here first( and you remove header buffer size check > in ksmbd_conn_handler_loop().) > When I apply patch till 09/14 number, check_user_session() and > get_ksmbd_tcon() is still has problem that access header without > header buffer size check. > > And there still are functions to access header without header buffer > size check. Please check allocate_rsp_buf(), is_transform_hdr() and > init_rsp_hdr() in __handle_ksmbd_work(). oh, that's a problem, missed that, sorry. Looks like we have to keep the validation at this layer. :( -slow
diff --git a/fs/ksmbd/smb2misc.c b/fs/ksmbd/smb2misc.c index 7ed266eb6c5e..541b39b7a84b 100644 --- a/fs/ksmbd/smb2misc.c +++ b/fs/ksmbd/smb2misc.c @@ -338,6 +338,9 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work) if (check_smb2_hdr(hdr)) return 1; + if (len < sizeof(struct smb2_pdu) - 4) + return 1; + if (hdr->StructureSize != SMB2_HEADER_STRUCTURE_SIZE) { ksmbd_debug(SMB, "Illegal structure size %u\n", le16_to_cpu(hdr->StructureSize));
Note: we already have the same check in is_chained_smb2_message(), but there it only applies to compound requests, so we have to repeat the check here to cover both cases. Cc: Namjae Jeon <linkinjeon@kernel.org> Cc: Tom Talpey <tom@talpey.com> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Steve French <smfrench@gmail.com> Cc: Hyunchul Lee <hyc.lee@gmail.com> Signed-off-by: Ralph Boehme <slow@samba.org> --- fs/ksmbd/smb2misc.c | 3 +++ 1 file changed, 3 insertions(+)