From patchwork Tue Jan 8 18:30:56 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ross Lagerwall X-Patchwork-Id: 1022089 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=linux-cifs-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=citrix.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 43Z17J1TDkz9sDr for ; Wed, 9 Jan 2019 05:31:40 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729485AbfAHSbi (ORCPT ); Tue, 8 Jan 2019 13:31:38 -0500 Received: from smtp03.citrix.com ([162.221.156.55]:23679 "EHLO SMTP03.CITRIX.COM" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728752AbfAHSbi (ORCPT ); Tue, 8 Jan 2019 13:31:38 -0500 X-IronPort-AV: E=Sophos;i="5.56,455,1539648000"; d="scan'208";a="75085427" From: Ross Lagerwall To: CC: Steve French , Pavel Shilovsky , Ross Lagerwall Subject: [PATCH 1/2] cifs: Limit memory used by lock request calls to a page Date: Tue, 8 Jan 2019 18:30:56 +0000 Message-ID: <20190108183057.5981-1-ross.lagerwall@citrix.com> X-Mailer: git-send-email 2.17.2 MIME-Version: 1.0 Sender: linux-cifs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org The code tries to allocate a contiguous buffer with a size supplied by the server (maxBuf). This could fail if memory is fragmented since it results in high order allocations for commonly used server implementations. It is also wasteful since there are probably few locks in the usual case. Limit the buffer to be no larger than a page to avoid memory allocation failures due to fragmentation. Signed-off-by: Ross Lagerwall --- fs/cifs/file.c | 8 ++++++++ fs/cifs/smb2file.c | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/fs/cifs/file.c b/fs/cifs/file.c index e3e3a7550205..673f948e4760 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -1140,6 +1140,10 @@ cifs_push_mandatory_locks(struct cifsFileInfo *cfile) return -EINVAL; } + BUILD_BUG_ON(sizeof(struct smb_hdr) + sizeof(LOCKING_ANDX_RANGE) > + PAGE_SIZE); + max_buf = min_t(unsigned int, max_buf - sizeof(struct smb_hdr), + PAGE_SIZE); max_num = (max_buf - sizeof(struct smb_hdr)) / sizeof(LOCKING_ANDX_RANGE); buf = kcalloc(max_num, sizeof(LOCKING_ANDX_RANGE), GFP_KERNEL); @@ -1478,6 +1482,10 @@ cifs_unlock_range(struct cifsFileInfo *cfile, struct file_lock *flock, if (!max_buf) return -EINVAL; + BUILD_BUG_ON(sizeof(struct smb_hdr) + sizeof(LOCKING_ANDX_RANGE) > + PAGE_SIZE); + max_buf = min_t(unsigned int, max_buf - sizeof(struct smb_hdr), + PAGE_SIZE); max_num = (max_buf - sizeof(struct smb_hdr)) / sizeof(LOCKING_ANDX_RANGE); buf = kcalloc(max_num, sizeof(LOCKING_ANDX_RANGE), GFP_KERNEL); diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c index 4ed10dd086e6..eff01ed6db0a 100644 --- a/fs/cifs/smb2file.c +++ b/fs/cifs/smb2file.c @@ -128,6 +128,8 @@ smb2_unlock_range(struct cifsFileInfo *cfile, struct file_lock *flock, if (!max_buf) return -EINVAL; + BUILD_BUG_ON(sizeof(struct smb2_lock_element) > PAGE_SIZE); + max_buf = min_t(unsigned int, max_buf, PAGE_SIZE); max_num = max_buf / sizeof(struct smb2_lock_element); buf = kcalloc(max_num, sizeof(struct smb2_lock_element), GFP_KERNEL); if (!buf) @@ -264,6 +266,8 @@ smb2_push_mandatory_locks(struct cifsFileInfo *cfile) return -EINVAL; } + BUILD_BUG_ON(sizeof(struct smb2_lock_element) > PAGE_SIZE); + max_buf = min_t(unsigned int, max_buf, PAGE_SIZE); max_num = max_buf / sizeof(struct smb2_lock_element); buf = kcalloc(max_num, sizeof(struct smb2_lock_element), GFP_KERNEL); if (!buf) { From patchwork Tue Jan 8 18:30:57 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ross Lagerwall X-Patchwork-Id: 1022090 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=linux-cifs-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=citrix.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 43Z17K6BBfz9sDL for ; Wed, 9 Jan 2019 05:31:41 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729528AbfAHSbk (ORCPT ); Tue, 8 Jan 2019 13:31:40 -0500 Received: from smtp03.citrix.com ([162.221.156.55]:23679 "EHLO SMTP03.CITRIX.COM" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728752AbfAHSbk (ORCPT ); Tue, 8 Jan 2019 13:31:40 -0500 X-IronPort-AV: E=Sophos;i="5.56,455,1539648000"; d="scan'208";a="75085435" From: Ross Lagerwall To: CC: Steve French , Pavel Shilovsky , Ross Lagerwall Subject: [PATCH 2/2] cifs: Fix potential OOB access of lock element array Date: Tue, 8 Jan 2019 18:30:57 +0000 Message-ID: <20190108183057.5981-2-ross.lagerwall@citrix.com> X-Mailer: git-send-email 2.17.2 In-Reply-To: <20190108183057.5981-1-ross.lagerwall@citrix.com> References: <20190108183057.5981-1-ross.lagerwall@citrix.com> MIME-Version: 1.0 Sender: linux-cifs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org If maxBuf is small but non-zero, it could result in a zero sized lock element array which we would then try and access OOB. Signed-off-by: Ross Lagerwall --- fs/cifs/file.c | 8 ++++---- fs/cifs/smb2file.c | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/fs/cifs/file.c b/fs/cifs/file.c index 673f948e4760..5b6f8392d9db 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -1132,10 +1132,10 @@ cifs_push_mandatory_locks(struct cifsFileInfo *cfile) /* * Accessing maxBuf is racy with cifs_reconnect - need to store value - * and check it for zero before using. + * and check it before using. */ max_buf = tcon->ses->server->maxBuf; - if (!max_buf) { + if (max_buf < (sizeof(struct smb_hdr) + sizeof(LOCKING_ANDX_RANGE))) { free_xid(xid); return -EINVAL; } @@ -1476,10 +1476,10 @@ cifs_unlock_range(struct cifsFileInfo *cfile, struct file_lock *flock, /* * Accessing maxBuf is racy with cifs_reconnect - need to store value - * and check it for zero before using. + * and check it before using. */ max_buf = tcon->ses->server->maxBuf; - if (!max_buf) + if (max_buf < (sizeof(struct smb_hdr) + sizeof(LOCKING_ANDX_RANGE))) return -EINVAL; BUILD_BUG_ON(sizeof(struct smb_hdr) + sizeof(LOCKING_ANDX_RANGE) > diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c index eff01ed6db0a..b204e84b87fb 100644 --- a/fs/cifs/smb2file.c +++ b/fs/cifs/smb2file.c @@ -122,10 +122,10 @@ smb2_unlock_range(struct cifsFileInfo *cfile, struct file_lock *flock, /* * Accessing maxBuf is racy with cifs_reconnect - need to store value - * and check it for zero before using. + * and check it before using. */ max_buf = tcon->ses->server->maxBuf; - if (!max_buf) + if (max_buf < sizeof(struct smb2_lock_element)) return -EINVAL; BUILD_BUG_ON(sizeof(struct smb2_lock_element) > PAGE_SIZE);