| Message ID | 20210924021254.27096-6-linkinjeon@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | a bunch of patches that have not yet been reviewed | expand |
2021년 9월 24일 (금) 오전 11:13, Namjae Jeon <linkinjeon@kernel.org>님이 작성: > > This patch add MAX_STREAM_PROT_LEN macro and check if stream protocol > length exceeds maximum value. opencode pdu size check in > ksmbd_pdu_size_has_room(). > > 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: Namjae Jeon <linkinjeon@kernel.org> > --- > fs/ksmbd/connection.c | 10 ++++++---- > fs/ksmbd/smb_common.c | 6 ------ > fs/ksmbd/smb_common.h | 4 ++-- > 3 files changed, 8 insertions(+), 12 deletions(-) > > diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c > index af086d35398a..48b18b4ec117 100644 > --- a/fs/ksmbd/connection.c > +++ b/fs/ksmbd/connection.c > @@ -296,10 +296,12 @@ int ksmbd_conn_handler_loop(void *p) > pdu_size = get_rfc1002_len(hdr_buf); > ksmbd_debug(CONN, "RFC1002 header %u bytes\n", pdu_size); > > - /* make sure we have enough to get to SMB header end */ > - if (!ksmbd_pdu_size_has_room(pdu_size)) { > - ksmbd_debug(CONN, "SMB request too short (%u bytes)\n", > - pdu_size); > + /* > + * Check if pdu size is valid (min : smb header size, > + * max : 0x00FFFFFF). > + */ > + if (pdu_size < __SMB2_HEADER_STRUCTURE_SIZE || > + pdu_size > MAX_STREAM_PROT_LEN) { > continue; > } > > diff --git a/fs/ksmbd/smb_common.c b/fs/ksmbd/smb_common.c > index 5901b2884c60..20bd5b8e3c0a 100644 > --- a/fs/ksmbd/smb_common.c > +++ b/fs/ksmbd/smb_common.c > @@ -21,7 +21,6 @@ static const char basechars[43] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ_-!@#$%"; > #define MAGIC_CHAR '~' > #define PERIOD '.' > #define mangle(V) ((char)(basechars[(V) % MANGLE_BASE])) > -#define KSMBD_MIN_SUPPORTED_HEADER_SIZE (sizeof(struct smb2_hdr)) > > struct smb_protocol { > int index; > @@ -272,11 +271,6 @@ int ksmbd_init_smb_server(struct ksmbd_work *work) > return 0; > } > > -bool ksmbd_pdu_size_has_room(unsigned int pdu) > -{ > - return (pdu >= KSMBD_MIN_SUPPORTED_HEADER_SIZE - 4); > -} > - > int ksmbd_populate_dot_dotdot_entries(struct ksmbd_work *work, int info_level, > struct ksmbd_file *dir, > struct ksmbd_dir_info *d_info, > diff --git a/fs/ksmbd/smb_common.h b/fs/ksmbd/smb_common.h > index 994abede27e9..6e79e7577f6b 100644 > --- a/fs/ksmbd/smb_common.h > +++ b/fs/ksmbd/smb_common.h > @@ -48,6 +48,8 @@ > #define CIFS_DEFAULT_IOSIZE (64 * 1024) > #define MAX_CIFS_SMALL_BUFFER_SIZE 448 /* big enough for most */ > > +#define MAX_STREAM_PROT_LEN 0x00FFFFFF Do we need to append "SMB" to this macro name? Looks good to me. Acked-by: Hyunchul Lee <hyc.lee@gmail.com> > + > /* Responses when opening a file. */ > #define F_SUPERSEDED 0 > #define F_OPENED 1 > @@ -493,8 +495,6 @@ int ksmbd_lookup_dialect_by_id(__le16 *cli_dialects, __le16 dialects_count); > > int ksmbd_init_smb_server(struct ksmbd_work *work); > > -bool ksmbd_pdu_size_has_room(unsigned int pdu); > - > struct ksmbd_kstat; > int ksmbd_populate_dot_dotdot_entries(struct ksmbd_work *work, > int info_level, > -- > 2.25.1 > -- Thanks, Hyunchul
2021-09-25 17:41 GMT+09:00, Hyunchul Lee <hyc.lee@gmail.com>: > 2021년 9월 24일 (금) 오전 11:13, Namjae Jeon <linkinjeon@kernel.org>님이 작성: >> >> This patch add MAX_STREAM_PROT_LEN macro and check if stream protocol >> length exceeds maximum value. opencode pdu size check in >> ksmbd_pdu_size_has_room(). >> >> 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: Namjae Jeon <linkinjeon@kernel.org> >> --- >> fs/ksmbd/connection.c | 10 ++++++---- >> fs/ksmbd/smb_common.c | 6 ------ >> fs/ksmbd/smb_common.h | 4 ++-- >> 3 files changed, 8 insertions(+), 12 deletions(-) >> >> diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c >> index af086d35398a..48b18b4ec117 100644 >> --- a/fs/ksmbd/connection.c >> +++ b/fs/ksmbd/connection.c >> @@ -296,10 +296,12 @@ int ksmbd_conn_handler_loop(void *p) >> pdu_size = get_rfc1002_len(hdr_buf); >> ksmbd_debug(CONN, "RFC1002 header %u bytes\n", pdu_size); >> >> - /* make sure we have enough to get to SMB header end */ >> - if (!ksmbd_pdu_size_has_room(pdu_size)) { >> - ksmbd_debug(CONN, "SMB request too short (%u >> bytes)\n", >> - pdu_size); >> + /* >> + * Check if pdu size is valid (min : smb header size, >> + * max : 0x00FFFFFF). >> + */ >> + if (pdu_size < __SMB2_HEADER_STRUCTURE_SIZE || >> + pdu_size > MAX_STREAM_PROT_LEN) { >> continue; >> } >> >> diff --git a/fs/ksmbd/smb_common.c b/fs/ksmbd/smb_common.c >> index 5901b2884c60..20bd5b8e3c0a 100644 >> --- a/fs/ksmbd/smb_common.c >> +++ b/fs/ksmbd/smb_common.c >> @@ -21,7 +21,6 @@ static const char basechars[43] = >> "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ_-!@#$%"; >> #define MAGIC_CHAR '~' >> #define PERIOD '.' >> #define mangle(V) ((char)(basechars[(V) % MANGLE_BASE])) >> -#define KSMBD_MIN_SUPPORTED_HEADER_SIZE (sizeof(struct smb2_hdr)) >> >> struct smb_protocol { >> int index; >> @@ -272,11 +271,6 @@ int ksmbd_init_smb_server(struct ksmbd_work *work) >> return 0; >> } >> >> -bool ksmbd_pdu_size_has_room(unsigned int pdu) >> -{ >> - return (pdu >= KSMBD_MIN_SUPPORTED_HEADER_SIZE - 4); >> -} >> - >> int ksmbd_populate_dot_dotdot_entries(struct ksmbd_work *work, int >> info_level, >> struct ksmbd_file *dir, >> struct ksmbd_dir_info *d_info, >> diff --git a/fs/ksmbd/smb_common.h b/fs/ksmbd/smb_common.h >> index 994abede27e9..6e79e7577f6b 100644 >> --- a/fs/ksmbd/smb_common.h >> +++ b/fs/ksmbd/smb_common.h >> @@ -48,6 +48,8 @@ >> #define CIFS_DEFAULT_IOSIZE (64 * 1024) >> #define MAX_CIFS_SMALL_BUFFER_SIZE 448 /* big enough for most */ >> >> +#define MAX_STREAM_PROT_LEN 0x00FFFFFF > > Do we need to append "SMB" to this macro name? I named it with the name defined in specification. That is, no problem. > Looks good to me. > Acked-by: Hyunchul Lee <hyc.lee@gmail.com> Thanks for your review! > > >> + >> /* Responses when opening a file. */ >> #define F_SUPERSEDED 0 >> #define F_OPENED 1 >> @@ -493,8 +495,6 @@ int ksmbd_lookup_dialect_by_id(__le16 *cli_dialects, >> __le16 dialects_count); >> >> int ksmbd_init_smb_server(struct ksmbd_work *work); >> >> -bool ksmbd_pdu_size_has_room(unsigned int pdu); >> - >> struct ksmbd_kstat; >> int ksmbd_populate_dot_dotdot_entries(struct ksmbd_work *work, >> int info_level, >> -- >> 2.25.1 >> > > > -- > Thanks, > Hyunchul >
diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c index af086d35398a..48b18b4ec117 100644 --- a/fs/ksmbd/connection.c +++ b/fs/ksmbd/connection.c @@ -296,10 +296,12 @@ int ksmbd_conn_handler_loop(void *p) pdu_size = get_rfc1002_len(hdr_buf); ksmbd_debug(CONN, "RFC1002 header %u bytes\n", pdu_size); - /* make sure we have enough to get to SMB header end */ - if (!ksmbd_pdu_size_has_room(pdu_size)) { - ksmbd_debug(CONN, "SMB request too short (%u bytes)\n", - pdu_size); + /* + * Check if pdu size is valid (min : smb header size, + * max : 0x00FFFFFF). + */ + if (pdu_size < __SMB2_HEADER_STRUCTURE_SIZE || + pdu_size > MAX_STREAM_PROT_LEN) { continue; } diff --git a/fs/ksmbd/smb_common.c b/fs/ksmbd/smb_common.c index 5901b2884c60..20bd5b8e3c0a 100644 --- a/fs/ksmbd/smb_common.c +++ b/fs/ksmbd/smb_common.c @@ -21,7 +21,6 @@ static const char basechars[43] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ_-!@#$%"; #define MAGIC_CHAR '~' #define PERIOD '.' #define mangle(V) ((char)(basechars[(V) % MANGLE_BASE])) -#define KSMBD_MIN_SUPPORTED_HEADER_SIZE (sizeof(struct smb2_hdr)) struct smb_protocol { int index; @@ -272,11 +271,6 @@ int ksmbd_init_smb_server(struct ksmbd_work *work) return 0; } -bool ksmbd_pdu_size_has_room(unsigned int pdu) -{ - return (pdu >= KSMBD_MIN_SUPPORTED_HEADER_SIZE - 4); -} - int ksmbd_populate_dot_dotdot_entries(struct ksmbd_work *work, int info_level, struct ksmbd_file *dir, struct ksmbd_dir_info *d_info, diff --git a/fs/ksmbd/smb_common.h b/fs/ksmbd/smb_common.h index 994abede27e9..6e79e7577f6b 100644 --- a/fs/ksmbd/smb_common.h +++ b/fs/ksmbd/smb_common.h @@ -48,6 +48,8 @@ #define CIFS_DEFAULT_IOSIZE (64 * 1024) #define MAX_CIFS_SMALL_BUFFER_SIZE 448 /* big enough for most */ +#define MAX_STREAM_PROT_LEN 0x00FFFFFF + /* Responses when opening a file. */ #define F_SUPERSEDED 0 #define F_OPENED 1 @@ -493,8 +495,6 @@ int ksmbd_lookup_dialect_by_id(__le16 *cli_dialects, __le16 dialects_count); int ksmbd_init_smb_server(struct ksmbd_work *work); -bool ksmbd_pdu_size_has_room(unsigned int pdu); - struct ksmbd_kstat; int ksmbd_populate_dot_dotdot_entries(struct ksmbd_work *work, int info_level,
This patch add MAX_STREAM_PROT_LEN macro and check if stream protocol length exceeds maximum value. opencode pdu size check in ksmbd_pdu_size_has_room(). 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: Namjae Jeon <linkinjeon@kernel.org> --- fs/ksmbd/connection.c | 10 ++++++---- fs/ksmbd/smb_common.c | 6 ------ fs/ksmbd/smb_common.h | 4 ++-- 3 files changed, 8 insertions(+), 12 deletions(-)