[2/2] cifs: Fix potential OOB access of lock element array
diff mbox series

Message ID 20190108183057.5981-2-ross.lagerwall@citrix.com
State New
Headers show
Series
  • [1/2] cifs: Limit memory used by lock request calls to a page
Related show

Commit Message

Ross Lagerwall Jan. 8, 2019, 6:30 p.m. UTC
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 <ross.lagerwall@citrix.com>
---
 fs/cifs/file.c     | 8 ++++----
 fs/cifs/smb2file.c | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

Comments

Steve French Jan. 10, 2019, 5:45 a.m. UTC | #1
adding cc:stable and merged into cifs-2.6.git for-next

On Tue, Jan 8, 2019 at 12:33 PM Ross Lagerwall
<ross.lagerwall@citrix.com> wrote:
>
> 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 <ross.lagerwall@citrix.com>
> ---
>  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);
> --
> 2.17.2
>

Patch
diff mbox series

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);