seems reasonable to merge the client fix - I will doublecheck it later
today or tomorrow and add it to for-next
On Sat, Feb 11, 2023 at 8:01 PM Namjae Jeon <linkinjeon@xxxxxxxxxx> wrote:
>
> 2023-02-12 5:53 GMT+09:00, Steve French <smfrench@xxxxxxxxx>:
> > Did you see any examples other than (SMB3) tree connect where the
> > Linux client padded a request beyond end of SMB/SMB3 frame?
> Yes. libsmb, but rfc1002 length is 8byte-aligned frame length. (i.e.
> ALIGN(clc_len, 8) == len) but cifs don't do that about smb2 tree
> connect, just frame length + 2.
> And I can not understand why cifs pad smb2 tree connect to 2bytes.
>
> note that smbclient, windows, MacOs, and Nautilus clients do not pad
> smb2 tree connect request.
>
> >
> > On Wed, Feb 8, 2023 at 3:41 AM Namjae Jeon <linkinjeon@xxxxxxxxxx> wrote:
> >>
> >> ksmbd allowed the actual frame length to be smaller than the rfc1002
> >> length. If allowed, it is possible to allocates a large amount of memory
> >> that can be limited by credit management and can eventually cause memory
> >> exhaustion problem. This patch do not allow it except SMB2 Negotiate
> >> request which will be validated when message handling proceeds.
> >> Also, cifs client pad smb2 tree connect to 2bytes.
> >>
> >> Signed-off-by: Namjae Jeon <linkinjeon@xxxxxxxxxx>
> >> ---
> >> fs/ksmbd/smb2misc.c | 23 +++++++++++------------
> >> 1 file changed, 11 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/fs/ksmbd/smb2misc.c b/fs/ksmbd/smb2misc.c
> >> index a717aa9b4af8..fc44f08b5939 100644
> >> --- a/fs/ksmbd/smb2misc.c
> >> +++ b/fs/ksmbd/smb2misc.c
> >> @@ -408,20 +408,19 @@ int ksmbd_smb2_check_message(struct ksmbd_work
> >> *work)
> >> goto validate_credit;
> >>
> >> /*
> >> - * windows client also pad up to 8 bytes when
> >> compounding.
> >> - * If pad is longer than eight bytes, log the server
> >> behavior
> >> - * (once), since may indicate a problem but allow it and
> >> - * continue since the frame is parseable.
> >> + * SMB2 NEGOTIATE request will be validated when message
> >> + * handling proceeds.
> >> */
> >> - if (clc_len < len) {
> >> - ksmbd_debug(SMB,
> >> - "cli req padded more than expected.
> >> Length %d not %d for cmd:%d mid:%llu\n",
> >> - len, clc_len, command,
> >> - le64_to_cpu(hdr->MessageId));
> >> - goto validate_credit;
> >> - }
> >> + if (command == SMB2_NEGOTIATE_HE)
> >> + goto validate_credit;
> >> +
> >> + /*
> >> + * cifs client pads smb2 tree connect to 2 bytes.
> >> + */
> >> + if (clc_len + 2 == len)
> >> + goto validate_credit;
> >>
> >> - ksmbd_debug(SMB,
> >> + pr_err_ratelimited(
> >> "cli req too short, len %d not %d. cmd:%d
> >> mid:%llu\n",
> >> len, clc_len, command,
> >> le64_to_cpu(hdr->MessageId));
> >> --
> >> 2.25.1
> >>
> >
> >
> > --
> > Thanks,
> >
> > Steve
> >
--
Thanks,
Steve
