| Message ID | 20210923034855.612832-1-linkinjeon@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ksmbd: add the check to vaildate if stream protocol length exceeds maximum value | expand |
On 9/22/2021 11:48 PM, Namjae Jeon wrote: > This patch add MAX_STREAM_PROT_LEN macro and check if stream protocol > length exceeds maximum value 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> > Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> > --- > fs/ksmbd/smb_common.c | 3 ++- > fs/ksmbd/smb_common.h | 2 ++ > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fs/ksmbd/smb_common.c b/fs/ksmbd/smb_common.c > index 5901b2884c60..ebc835ab414c 100644 > --- a/fs/ksmbd/smb_common.c > +++ b/fs/ksmbd/smb_common.c > @@ -274,7 +274,8 @@ int ksmbd_init_smb_server(struct ksmbd_work *work) > > bool ksmbd_pdu_size_has_room(unsigned int pdu) > { > - return (pdu >= KSMBD_MIN_SUPPORTED_HEADER_SIZE - 4); > + return (pdu >= KSMBD_MIN_SUPPORTED_HEADER_SIZE - 4 && > + pdu <= MAX_STREAM_PROT_LEN); Incorrect. If the pdu is already 2^24-1 bytes long, there is no "room". I don't think a "<" fixes this either. One byte isn't sufficient to allow any significant addition, in all likelihood. What, exactly, is this check protecting? Tom. > } > > int ksmbd_populate_dot_dotdot_entries(struct ksmbd_work *work, int info_level, > diff --git a/fs/ksmbd/smb_common.h b/fs/ksmbd/smb_common.h > index 994abede27e9..10b8d7224dfa 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 >
2021-09-24 0:05 GMT+09:00, Tom Talpey <tom@talpey.com>: > > > On 9/22/2021 11:48 PM, Namjae Jeon wrote: >> This patch add MAX_STREAM_PROT_LEN macro and check if stream protocol >> length exceeds maximum value 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> >> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> >> --- >> fs/ksmbd/smb_common.c | 3 ++- >> fs/ksmbd/smb_common.h | 2 ++ >> 2 files changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/fs/ksmbd/smb_common.c b/fs/ksmbd/smb_common.c >> index 5901b2884c60..ebc835ab414c 100644 >> --- a/fs/ksmbd/smb_common.c >> +++ b/fs/ksmbd/smb_common.c >> @@ -274,7 +274,8 @@ int ksmbd_init_smb_server(struct ksmbd_work *work) >> >> bool ksmbd_pdu_size_has_room(unsigned int pdu) >> { >> - return (pdu >= KSMBD_MIN_SUPPORTED_HEADER_SIZE - 4); >> + return (pdu >= KSMBD_MIN_SUPPORTED_HEADER_SIZE - 4 && >> + pdu <= MAX_STREAM_PROT_LEN); > > Incorrect. If the pdu is already 2^24-1 bytes long, there is no "room". > > I don't think a "<" fixes this either. One byte isn't sufficient to > allow any significant addition, in all likelihood. > > What, exactly, is this check protecting? Ah, Sorry, The function name and comment seems to create a misunderstood. It is to check the min/max size of pdu. If pdu size is bigger than maximum stream protocol length (0x00FFFFFF), ksmbd don't handle such invalid requests. > > Tom. > >> } >> >> int ksmbd_populate_dot_dotdot_entries(struct ksmbd_work *work, int >> info_level, >> diff --git a/fs/ksmbd/smb_common.h b/fs/ksmbd/smb_common.h >> index 994abede27e9..10b8d7224dfa 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 >> >
diff --git a/fs/ksmbd/smb_common.c b/fs/ksmbd/smb_common.c index 5901b2884c60..ebc835ab414c 100644 --- a/fs/ksmbd/smb_common.c +++ b/fs/ksmbd/smb_common.c @@ -274,7 +274,8 @@ int ksmbd_init_smb_server(struct ksmbd_work *work) bool ksmbd_pdu_size_has_room(unsigned int pdu) { - return (pdu >= KSMBD_MIN_SUPPORTED_HEADER_SIZE - 4); + return (pdu >= KSMBD_MIN_SUPPORTED_HEADER_SIZE - 4 && + pdu <= MAX_STREAM_PROT_LEN); } int ksmbd_populate_dot_dotdot_entries(struct ksmbd_work *work, int info_level, diff --git a/fs/ksmbd/smb_common.h b/fs/ksmbd/smb_common.h index 994abede27e9..10b8d7224dfa 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
This patch add MAX_STREAM_PROT_LEN macro and check if stream protocol length exceeds maximum value 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> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> --- fs/ksmbd/smb_common.c | 3 ++- fs/ksmbd/smb_common.h | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-)