From patchwork Wed May 30 17:53:47 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steve French X-Patchwork-Id: 10439503 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 6EFC2601E9 for ; Wed, 30 May 2018 17:55:18 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5486F27D0C for ; Wed, 30 May 2018 17:55:18 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 48F6129154; Wed, 30 May 2018 17:55:18 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.8 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI, T_DKIM_INVALID, T_TVD_MIME_EPI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8EC2027D0C for ; Wed, 30 May 2018 17:55:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753427AbeE3RzR (ORCPT ); Wed, 30 May 2018 13:55:17 -0400 Received: from mail-pl0-f65.google.com ([209.85.160.65]:42999 "EHLO mail-pl0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753718AbeE3RyI (ORCPT ); Wed, 30 May 2018 13:54:08 -0400 Received: by mail-pl0-f65.google.com with SMTP id u6-v6so11538192pls.9 for ; Wed, 30 May 2018 10:54:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=ohgzZY4ir6K0PE4l5K07Opg0LLrfyMaFUy/Rw0IA+S4=; b=YmjpnBIWmQyZND3Cect73Kns5o1cpmIuq3IzvxJIsSai52rsqCRDyIhrKn1QXk85Sk qb8D21gm9XVuVXLEZ1r444heajOZzDFpZm9Iu48YUGs9mJIKe4Lzslap0+r8K8QZ12Je 785VVu9jnop3ybZ/sv6wLbyvrQVQP6rrnVEr7VYslBuVl9ROLCSXUTpylgQnNEtPyFt8 ZJ/FPtUv5BLI2TWA2OiUvc1Dpv7O2USRLS6L+RuVOGshcemMOJ5V24iLVYYKxu+7pFjP d75AdiCoQXWPRskCX2QjXvNZscTBuzqmaVAzVCFzH764cboLsswWOaBIfCsgo1hiU+o6 Uj5w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=ohgzZY4ir6K0PE4l5K07Opg0LLrfyMaFUy/Rw0IA+S4=; b=hzSXnb1FQdruShFx3ebCAoh1yDZC2RiRw0ZMQjHXRtQyTmtHSQJGtLvGVDnGhEufZE Fjt+qsogmKQmOhCOy0p5BsQKEeYjNArOgeUX2w4tUUxl227VjTNHYKw+d4V/E2m495we Pb1M6cuJ0aOYqmlZ1cT/eifo7GrLLgTkbVLQWXDaxx3cs7rjfJ6BvOmb1ZmiNMyvdLlH 6N9Za/VoWWnqhGcOs27kkgiNXdmU77q2iugYWNEz+vnZLWPZl7R9Z9qPaOh77sQP18yY XNCwhlrouKhNj0II0xFIy5OT/INpKk05DrXp7bgQf0jfCzy2cbiwf1mfOvC7DeT3ncwi 77nQ== X-Gm-Message-State: ALKqPwfv4sip4EgQZDV/+lf8IvWL+rn5B1ks+r6pAIG6m02t1Rl0Ktse TpwIKuZKnY7wSn/VCDs584ogxdZrffFBX5YYHd1jMw== X-Google-Smtp-Source: ADUXVKJJWZ3vnbkMzJQ/VcRDrJhrflfQ1LJS5OL5tvZJe2VdXwFf0T6AE+s+bcnFpfT5jdYOiEsEMvHO8daokjPx7g0= X-Received: by 2002:a17:902:8541:: with SMTP id d1-v6mr3917385plo.106.1527702848018; Wed, 30 May 2018 10:54:08 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a17:90a:bd8f:0:0:0:0 with HTTP; Wed, 30 May 2018 10:53:47 -0700 (PDT) In-Reply-To: <20180530004715.859-2-lsahlber@redhat.com> References: <20180530004715.859-1-lsahlber@redhat.com> <20180530004715.859-2-lsahlber@redhat.com> From: Steve French Date: Wed, 30 May 2018 12:53:47 -0500 Message-ID: Subject: Re: [PATCH 01/15] cifs: update smb2_check_message to handle PDUs without a 4 byte length header To: Ronnie Sahlberg Cc: linux-cifs Sender: linux-cifs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP to get it to build I had to fix a couple lines (get_neg_ctxt_len) and merged in with your patch 1- see attached On Tue, May 29, 2018 at 7:47 PM, Ronnie Sahlberg wrote: > Signed-off-by: Ronnie Sahlberg > --- > fs/cifs/smb2misc.c | 43 ++++++++++++++++--------------------------- > 1 file changed, 16 insertions(+), 27 deletions(-) > > diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c > index f7f3ad760401..0a12c2133770 100644 > --- a/fs/cifs/smb2misc.c > +++ b/fs/cifs/smb2misc.c > @@ -131,25 +131,20 @@ static __u32 get_neg_ctxt_len(struct smb2_hdr *hdr, __u32 len, __u32 non_ctxlen, > #endif /* CIFS_SMB311 */ > > int > -smb2_check_message(char *buf, unsigned int length, struct TCP_Server_Info *srvr) > +smb2_check_message(char *buf, unsigned int len, struct TCP_Server_Info *srvr) > { > - struct smb2_pdu *pdu = (struct smb2_pdu *)buf; > - struct smb2_hdr *hdr = &pdu->hdr; > - struct smb2_sync_hdr *shdr = get_sync_hdr(buf); > + struct smb2_sync_hdr *shdr = (struct smb2_sync_hdr *)(buf + srvr->vals->header_preamble_size); > + struct smb2_sync_pdu *pdu = (struct smb2_sync_pdu *)shdr; > __u64 mid; > - __u32 len = get_rfc1002_length(buf); > __u32 clc_len; /* calculated length */ > int command; > - > - /* BB disable following printk later */ > - cifs_dbg(FYI, "%s length: 0x%x, smb_buf_length: 0x%x\n", > - __func__, length, len); > + int pdu_size = sizeof(struct smb2_sync_pdu); > + int hdr_size = sizeof(struct smb2_sync_hdr); > > /* > * Add function to do table lookup of StructureSize by command > * ie Validate the wct via smb2_struct_sizes table above > */ > - > if (shdr->ProtocolId == SMB2_TRANSFORM_PROTO_NUM) { > struct smb2_transform_hdr *thdr = > (struct smb2_transform_hdr *)buf; > @@ -173,8 +168,8 @@ smb2_check_message(char *buf, unsigned int length, struct TCP_Server_Info *srvr) > } > > mid = le64_to_cpu(shdr->MessageId); > - if (length < sizeof(struct smb2_pdu)) { > - if ((length >= sizeof(struct smb2_hdr)) > + if (len < pdu_size) { > + if ((len >= hdr_size) > && (shdr->Status != 0)) { > pdu->StructureSize2 = 0; > /* > @@ -227,31 +222,25 @@ smb2_check_message(char *buf, unsigned int length, struct TCP_Server_Info *srvr) > } > } > > - if (srvr->vals->header_preamble_size + len != length) { > - cifs_dbg(VFS, "Total length %u RFC1002 length %zu mismatch mid %llu\n", > - length, srvr->vals->header_preamble_size + len, mid); > - return 1; > - } > - > - clc_len = smb2_calc_size(hdr, srvr); > + clc_len = smb2_calc_size(buf, srvr); > > #ifdef CONFIG_CIFS_SMB311 > if (shdr->Command == SMB2_NEGOTIATE) > clc_len += get_neg_ctxt_len(hdr, len, clc_len, > srvr->vals->header_preamble_size); > #endif /* SMB311 */ > - if (srvr->vals->header_preamble_size + len != clc_len) { > - cifs_dbg(FYI, "Calculated size %u length %zu mismatch mid %llu\n", > - clc_len, srvr->vals->header_preamble_size + len, mid); > + if (len != clc_len) { > + cifs_dbg(FYI, "Calculated size %u length %u mismatch mid %llu\n", > + clc_len, len, mid); > /* create failed on symlink */ > if (command == SMB2_CREATE_HE && > shdr->Status == STATUS_STOPPED_ON_SYMLINK) > return 0; > /* Windows 7 server returns 24 bytes more */ > - if (clc_len + 24 - srvr->vals->header_preamble_size == len && command == SMB2_OPLOCK_BREAK_HE) > + if (clc_len + 24 == len && command == SMB2_OPLOCK_BREAK_HE) > return 0; > /* server can return one byte more due to implied bcc[0] */ > - if (clc_len == srvr->vals->header_preamble_size + len + 1) > + if (clc_len == len + 1) > return 0; > > /* > @@ -261,10 +250,10 @@ smb2_check_message(char *buf, unsigned int length, struct TCP_Server_Info *srvr) > * Log the server error (once), but allow it and continue > * since the frame is parseable. > */ > - if (clc_len < srvr->vals->header_preamble_size /* RFC1001 header size */ + len) { > + if (clc_len < len) { > printk_once(KERN_WARNING > - "SMB2 server sent bad RFC1001 len %d not %zu\n", > - len, clc_len - srvr->vals->header_preamble_size); > + "SMB2 server sent bad RFC1001 len %d not %u\n", > + len, clc_len); > return 0; > } > > -- > 2.13.3 > From 5b540f13736fe4f7e11fd7d2ab27ae29b91c8ad0 Mon Sep 17 00:00:00 2001 From: Ronnie Sahlberg Date: Wed, 30 May 2018 10:47:01 +1000 Subject: [PATCH 1/2] cifs: update smb2_check_message to handle PDUs without a 4 byte length header Signed-off-by: Ronnie Sahlberg Signed-off-by: Steve French --- fs/cifs/smb2misc.c | 49 ++++++++++++++++++---------------------------- 1 file changed, 19 insertions(+), 30 deletions(-) diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c index f7f3ad760401..ba1a20cf9846 100644 --- a/fs/cifs/smb2misc.c +++ b/fs/cifs/smb2misc.c @@ -94,8 +94,8 @@ static const __le16 smb2_rsp_struct_sizes[NUMBER_OF_SMB2_COMMANDS] = { }; #ifdef CONFIG_CIFS_SMB311 -static __u32 get_neg_ctxt_len(struct smb2_hdr *hdr, __u32 len, __u32 non_ctxlen, - size_t hdr_preamble_size) +static __u32 get_neg_ctxt_len(struct smb2_sync_hdr *hdr, __u32 len, + __u32 non_ctxlen, size_t hdr_preamble_size) { __u16 neg_count; __u32 nc_offset, size_of_pad_before_neg_ctxts; @@ -131,25 +131,20 @@ static __u32 get_neg_ctxt_len(struct smb2_hdr *hdr, __u32 len, __u32 non_ctxlen, #endif /* CIFS_SMB311 */ int -smb2_check_message(char *buf, unsigned int length, struct TCP_Server_Info *srvr) +smb2_check_message(char *buf, unsigned int len, struct TCP_Server_Info *srvr) { - struct smb2_pdu *pdu = (struct smb2_pdu *)buf; - struct smb2_hdr *hdr = &pdu->hdr; - struct smb2_sync_hdr *shdr = get_sync_hdr(buf); + struct smb2_sync_hdr *shdr = (struct smb2_sync_hdr *)(buf + srvr->vals->header_preamble_size); + struct smb2_sync_pdu *pdu = (struct smb2_sync_pdu *)shdr; __u64 mid; - __u32 len = get_rfc1002_length(buf); __u32 clc_len; /* calculated length */ int command; - - /* BB disable following printk later */ - cifs_dbg(FYI, "%s length: 0x%x, smb_buf_length: 0x%x\n", - __func__, length, len); + int pdu_size = sizeof(struct smb2_sync_pdu); + int hdr_size = sizeof(struct smb2_sync_hdr); /* * Add function to do table lookup of StructureSize by command * ie Validate the wct via smb2_struct_sizes table above */ - if (shdr->ProtocolId == SMB2_TRANSFORM_PROTO_NUM) { struct smb2_transform_hdr *thdr = (struct smb2_transform_hdr *)buf; @@ -173,8 +168,8 @@ smb2_check_message(char *buf, unsigned int length, struct TCP_Server_Info *srvr) } mid = le64_to_cpu(shdr->MessageId); - if (length < sizeof(struct smb2_pdu)) { - if ((length >= sizeof(struct smb2_hdr)) + if (len < pdu_size) { + if ((len >= hdr_size) && (shdr->Status != 0)) { pdu->StructureSize2 = 0; /* @@ -227,31 +222,25 @@ smb2_check_message(char *buf, unsigned int length, struct TCP_Server_Info *srvr) } } - if (srvr->vals->header_preamble_size + len != length) { - cifs_dbg(VFS, "Total length %u RFC1002 length %zu mismatch mid %llu\n", - length, srvr->vals->header_preamble_size + len, mid); - return 1; - } - - clc_len = smb2_calc_size(hdr, srvr); + clc_len = smb2_calc_size(buf, srvr); #ifdef CONFIG_CIFS_SMB311 if (shdr->Command == SMB2_NEGOTIATE) - clc_len += get_neg_ctxt_len(hdr, len, clc_len, + clc_len += get_neg_ctxt_len(shdr, len, clc_len, srvr->vals->header_preamble_size); #endif /* SMB311 */ - if (srvr->vals->header_preamble_size + len != clc_len) { - cifs_dbg(FYI, "Calculated size %u length %zu mismatch mid %llu\n", - clc_len, srvr->vals->header_preamble_size + len, mid); + if (len != clc_len) { + cifs_dbg(FYI, "Calculated size %u length %u mismatch mid %llu\n", + clc_len, len, mid); /* create failed on symlink */ if (command == SMB2_CREATE_HE && shdr->Status == STATUS_STOPPED_ON_SYMLINK) return 0; /* Windows 7 server returns 24 bytes more */ - if (clc_len + 24 - srvr->vals->header_preamble_size == len && command == SMB2_OPLOCK_BREAK_HE) + if (clc_len + 24 == len && command == SMB2_OPLOCK_BREAK_HE) return 0; /* server can return one byte more due to implied bcc[0] */ - if (clc_len == srvr->vals->header_preamble_size + len + 1) + if (clc_len == len + 1) return 0; /* @@ -261,10 +250,10 @@ smb2_check_message(char *buf, unsigned int length, struct TCP_Server_Info *srvr) * Log the server error (once), but allow it and continue * since the frame is parseable. */ - if (clc_len < srvr->vals->header_preamble_size /* RFC1001 header size */ + len) { + if (clc_len < len) { printk_once(KERN_WARNING - "SMB2 server sent bad RFC1001 len %d not %zu\n", - len, clc_len - srvr->vals->header_preamble_size); + "SMB2 server sent bad RFC1001 len %d not %u\n", + len, clc_len); return 0; } -- 2.17.0