From patchwork Sat Aug 20 01:55:33 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Layton X-Patchwork-Id: 1082132 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter2.kernel.org (8.14.4/8.14.4) with ESMTP id p7K1tjDb012305 for ; Sat, 20 Aug 2011 01:55:52 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754035Ab1HTBzv (ORCPT ); Fri, 19 Aug 2011 21:55:51 -0400 Received: from mail-gx0-f174.google.com ([209.85.161.174]:43461 "EHLO mail-gx0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754247Ab1HTBzv (ORCPT ); Fri, 19 Aug 2011 21:55:51 -0400 Received: by gxk21 with SMTP id 21so2599632gxk.19 for ; Fri, 19 Aug 2011 18:55:50 -0700 (PDT) Received: by 10.100.235.26 with SMTP id i26mr22948anh.96.1313805350540; Fri, 19 Aug 2011 18:55:50 -0700 (PDT) Received: from salusa.poochiereds.net (cpe-075-177-182-191.nc.res.rr.com [75.177.182.191]) by mx.google.com with ESMTPS id b5sm3122509anm.21.2011.08.19.18.55.49 (version=SSLv3 cipher=OTHER); Fri, 19 Aug 2011 18:55:50 -0700 (PDT) From: Jeff Layton To: smfrench@gmail.com Cc: piastryyy@gmail.com, linux-cifs@vger.kernel.org Subject: [PATCH 08/14] cifs: clean up checkSMB Date: Fri, 19 Aug 2011 21:55:33 -0400 Message-Id: <1313805339-1233-9-git-send-email-jlayton@redhat.com> X-Mailer: git-send-email 1.7.6 In-Reply-To: <1313805339-1233-1-git-send-email-jlayton@redhat.com> References: <1313805339-1233-1-git-send-email-jlayton@redhat.com> Sender: linux-cifs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter2.kernel.org [140.211.167.43]); Sat, 20 Aug 2011 01:55:52 +0000 (UTC) The variable names in this function are so ambiguous that it's very difficult to know what it's doing. Rename them to make it a bit more clear. Also, remove a redundant length check. cifsd checks to make sure that the rfclen isn't larger than the maximum frame size when it does the receive. Finally, change checkSMB to return a real error code (-EIO) when it finds an error. That will help simplify some coming changes in the callers. Signed-off-by: Jeff Layton --- fs/cifs/connect.c | 2 +- fs/cifs/misc.c | 51 +++++++++++++++++++++++++-------------------------- 2 files changed, 26 insertions(+), 27 deletions(-) diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 5adbeec..28007fd 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -422,7 +422,7 @@ readv_from_socket(struct TCP_Server_Info *server, struct kvec *iov_orig, /* * FIXME: allocation here may cause deadlocks under memory pressure. - * Switch this to use a fixed, per-socket buffer? + * Switch this to use a fixed, per-socket buffer. */ iov = kmalloc(sizeof(*iov_orig) * nr_segs, GFP_KERNEL); if (!iov) diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c index 7c16933..4a1801b 100644 --- a/fs/cifs/misc.c +++ b/fs/cifs/misc.c @@ -420,19 +420,22 @@ check_smb_hdr(struct smb_hdr *smb, __u16 mid) } int -checkSMB(struct smb_hdr *smb, __u16 mid, unsigned int length) +checkSMB(struct smb_hdr *smb, __u16 mid, unsigned int total_read) { - __u32 len = be32_to_cpu(smb->smb_buf_length); + __u32 rfclen = be32_to_cpu(smb->smb_buf_length); __u32 clc_len; /* calculated length */ - cFYI(0, "checkSMB Length: 0x%x, smb_buf_length: 0x%x", length, len); + cFYI(0, "checkSMB Length: 0x%x, smb_buf_length: 0x%x", + total_read, rfclen); - if (length < 2 + sizeof(struct smb_hdr)) { - if ((length >= sizeof(struct smb_hdr) - 1) + /* is this frame too small to even get to a BCC? */ + if (total_read < 2 + sizeof(struct smb_hdr)) { + if ((total_read >= sizeof(struct smb_hdr) - 1) && (smb->Status.CifsError != 0)) { + /* it's an error return */ smb->WordCount = 0; /* some error cases do not return wct and bcc */ return 0; - } else if ((length == sizeof(struct smb_hdr) + 1) && + } else if ((total_read == sizeof(struct smb_hdr) + 1) && (smb->WordCount == 0)) { char *tmp = (char *)smb; /* Need to work around a bug in two servers here */ @@ -452,39 +455,35 @@ checkSMB(struct smb_hdr *smb, __u16 mid, unsigned int length) } else { cERROR(1, "Length less than smb header size"); } - return 1; - } - if (len > CIFSMaxBufSize + MAX_CIFS_HDR_SIZE - 4) { - cERROR(1, "smb length greater than MaxBufSize, mid=%d", - smb->Mid); - return 1; + return -EIO; } + /* otherwise, there is enough to get to the BCC */ if (check_smb_hdr(smb, mid)) - return 1; + return -EIO; clc_len = smbCalcSize(smb); - if (4 + len != length) { + if (4 + rfclen != total_read) { cERROR(1, "Length read does not match RFC1001 length %d", - len); - return 1; + rfclen); + return -EIO; } - if (4 + len != clc_len) { + if (4 + rfclen != clc_len) { /* check if bcc wrapped around for large read responses */ - if ((len > 64 * 1024) && (len > clc_len)) { + if ((rfclen > 64 * 1024) && (rfclen > clc_len)) { /* check if lengths match mod 64K */ - if (((4 + len) & 0xFFFF) == (clc_len & 0xFFFF)) + if (((4 + rfclen) & 0xFFFF) == (clc_len & 0xFFFF)) return 0; /* bcc wrapped */ } cFYI(1, "Calculated size %u vs length %u mismatch for mid=%u", - clc_len, 4 + len, smb->Mid); + clc_len, 4 + rfclen, smb->Mid); - if (4 + len < clc_len) { + if (4 + rfclen < clc_len) { cERROR(1, "RFC1001 size %u smaller than SMB for mid=%u", - len, smb->Mid); - return 1; - } else if (len > clc_len + 512) { + rfclen, smb->Mid); + return -EIO; + } else if (rfclen > clc_len + 512) { /* * Some servers (Windows XP in particular) send more * data than the lengths in the SMB packet would @@ -495,8 +494,8 @@ checkSMB(struct smb_hdr *smb, __u16 mid, unsigned int length) * data to 512 bytes. */ cERROR(1, "RFC1001 size %u more than 512 bytes larger " - "than SMB for mid=%u", len, smb->Mid); - return 1; + "than SMB for mid=%u", rfclen, smb->Mid); + return -EIO; } } return 0;