[PATCH 0/2] LP#1704857 Fix CIFS oops

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 0/2] LP#1704857 Fix CIFS oops

Thadeu Lima de Souza Cascardo-3
[Impact]

Causes an oops, possibly causing a DOS on the system.

[Regression Potential]

Changes restricted to the CIFS filesystem code.

[Test Case]

Asked reported to explain how to reproduce the issue.


Christophe Jaillet (1):
  CIFS: Fix some return values in case of error in 'crypt_message'

Pavel Shilovsky (1):
  CIFS: Fix null pointer deref during read resp processing

 fs/cifs/cifsproto.h |  3 +--
 fs/cifs/cifssmb.c   | 15 ++++++++-------
 fs/cifs/smb2ops.c   |  8 +++++---
 3 files changed, 14 insertions(+), 12 deletions(-)

--
2.11.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 1/2] CIFS: Fix null pointer deref during read resp processing

Thadeu Lima de Souza Cascardo-3
From: Pavel Shilovsky <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1704857

Currently during receiving a read response mid->resp_buf can be
NULL when it is being passed to cifs_discard_remaining_data() from
cifs_readv_discard(). Fix it by always passing server->smallbuf
instead and initializing mid->resp_buf at the end of read response
processing.

Signed-off-by: Pavel Shilovsky <[hidden email]>
CC: Stable <[hidden email]>
Acked-by: Sachin Prabhu <[hidden email]>
Signed-off-by: Steve French <[hidden email]>
(cherry picked from commit 350be257ea83029daee974c72b1fe2e6f1f8e615)
Signed-off-by: Thadeu Lima de Souza Cascardo <[hidden email]>
---
 fs/cifs/cifsproto.h |  3 +--
 fs/cifs/cifssmb.c   | 15 ++++++++-------
 fs/cifs/smb2ops.c   |  4 ++--
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 64518f3194be..b0e8c9076061 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -76,8 +76,7 @@ extern void cifs_delete_mid(struct mid_q_entry *mid);
 extern void cifs_wake_up_task(struct mid_q_entry *mid);
 extern int cifs_handle_standard(struct TCP_Server_Info *server,
  struct mid_q_entry *mid);
-extern int cifs_discard_remaining_data(struct TCP_Server_Info *server,
-       char *buf);
+extern int cifs_discard_remaining_data(struct TCP_Server_Info *server);
 extern int cifs_call_async(struct TCP_Server_Info *server,
  struct smb_rqst *rqst,
  mid_receive_t *receive, mid_callback_t *callback,
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 6a9f3fb21801..de35dbd5aabf 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -1404,9 +1404,9 @@ openRetry:
  * current bigbuf.
  */
 int
-cifs_discard_remaining_data(struct TCP_Server_Info *server, char *buf)
+cifs_discard_remaining_data(struct TCP_Server_Info *server)
 {
- unsigned int rfclen = get_rfc1002_length(buf);
+ unsigned int rfclen = get_rfc1002_length(server->smallbuf);
  int remaining = rfclen + 4 - server->total_read;
 
  while (remaining > 0) {
@@ -1430,8 +1430,10 @@ cifs_readv_discard(struct TCP_Server_Info *server, struct mid_q_entry *mid)
  int length;
  struct cifs_readdata *rdata = mid->callback_data;
 
- length = cifs_discard_remaining_data(server, mid->resp_buf);
+ length = cifs_discard_remaining_data(server);
  dequeue_mid(mid, rdata->result);
+ mid->resp_buf = server->smallbuf;
+ server->smallbuf = NULL;
  return length;
 }
 
@@ -1463,7 +1465,7 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
 
  if (server->ops->is_status_pending &&
     server->ops->is_status_pending(buf, server, 0)) {
- cifs_discard_remaining_data(server, buf);
+ cifs_discard_remaining_data(server);
  return -1;
  }
 
@@ -1523,9 +1525,6 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
  cifs_dbg(FYI, "0: iov_base=%p iov_len=%u\n",
  rdata->iov[0].iov_base, server->total_read);
 
- mid->resp_buf = server->smallbuf;
- server->smallbuf = NULL;
-
  /* how much data is in the response? */
  data_len = server->ops->read_data_length(buf);
  if (data_offset + data_len > buflen) {
@@ -1548,6 +1547,8 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
  return cifs_readv_discard(server, mid);
 
  dequeue_mid(mid, false);
+ mid->resp_buf = server->smallbuf;
+ server->smallbuf = NULL;
  return length;
 }
 
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index aac124064deb..b0eb89eddad6 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -2037,7 +2037,7 @@ receive_encrypted_read(struct TCP_Server_Info *server, struct mid_q_entry **mid)
  if (rc)
  goto free_pages;
 
- rc = cifs_discard_remaining_data(server, buf);
+ rc = cifs_discard_remaining_data(server);
  if (rc)
  goto free_pages;
 
@@ -2063,7 +2063,7 @@ free_pages:
  kfree(pages);
  return rc;
 discard_data:
- cifs_discard_remaining_data(server, buf);
+ cifs_discard_remaining_data(server);
  goto free_pages;
 }
 
--
2.11.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 2/2] CIFS: Fix some return values in case of error in 'crypt_message'

Thadeu Lima de Souza Cascardo-3
In reply to this post by Thadeu Lima de Souza Cascardo-3
From: Christophe Jaillet <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1704857

'rc' is known to be 0 at this point. So if 'init_sg' or 'kzalloc' fails, we
should return -ENOMEM instead.

Also remove a useless 'rc' in a debug message as it is meaningless here.

Fixes: 026e93dc0a3ee ("CIFS: Encrypt SMB3 requests before sending")
Signed-off-by: Christophe JAILLET <[hidden email]>
Reviewed-by: Pavel Shilovsky <[hidden email]>
Reviewed-by: Aurelien Aptel <[hidden email]>
Signed-off-by: Steve French <[hidden email]>
CC: Stable <[hidden email]>
(cherry picked from commit 517a6e43c4872c89794af5b377fa085e47345952)
Signed-off-by: Thadeu Lima de Souza Cascardo <[hidden email]>
---
 fs/cifs/smb2ops.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index b0eb89eddad6..df4274b61ceb 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -1650,7 +1650,8 @@ crypt_message(struct TCP_Server_Info *server, struct smb_rqst *rqst, int enc)
 
  sg = init_sg(rqst, sign);
  if (!sg) {
- cifs_dbg(VFS, "%s: Failed to init sg %d", __func__, rc);
+ cifs_dbg(VFS, "%s: Failed to init sg", __func__);
+ rc = -ENOMEM;
  goto free_req;
  }
 
@@ -1658,6 +1659,7 @@ crypt_message(struct TCP_Server_Info *server, struct smb_rqst *rqst, int enc)
  iv = kzalloc(iv_len, GFP_KERNEL);
  if (!iv) {
  cifs_dbg(VFS, "%s: Failed to alloc IV", __func__);
+ rc = -ENOMEM;
  goto free_sg;
  }
  iv[0] = 3;
--
2.11.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

ACK: [PATCH 0/2] LP#1704857 Fix CIFS oops

Kleber Souza
In reply to this post by Thadeu Lima de Souza Cascardo-3
On 07/17/17 21:29, Thadeu Lima de Souza Cascardo wrote:

> [Impact]
>
> Causes an oops, possibly causing a DOS on the system.
>
> [Regression Potential]
>
> Changes restricted to the CIFS filesystem code.
>
> [Test Case]
>
> Asked reported to explain how to reproduce the issue.
>
>
> Christophe Jaillet (1):
>   CIFS: Fix some return values in case of error in 'crypt_message'
>
> Pavel Shilovsky (1):
>   CIFS: Fix null pointer deref during read resp processing
>
>  fs/cifs/cifsproto.h |  3 +--
>  fs/cifs/cifssmb.c   | 15 ++++++++-------
>  fs/cifs/smb2ops.c   |  8 +++++---
>  3 files changed, 14 insertions(+), 12 deletions(-)
>

Acked-by: Kleber Sacilotto de Souza <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 0/2] LP#1704857 Fix CIFS oops

Thadeu Lima de Souza Cascardo-3
In reply to this post by Thadeu Lima de Souza Cascardo-3
On Mon, Jul 17, 2017 at 04:29:11PM -0300, Thadeu Lima de Souza Cascardo wrote:

> [Impact]
>
> Causes an oops, possibly causing a DOS on the system.
>
> [Regression Potential]
>
> Changes restricted to the CIFS filesystem code.
>
> [Test Case]
>
> Asked reported to explain how to reproduce the issue.
>

Tried Connectathon and Large Compile test always causes the issue to
reproduce on 4.4.0-86. With those patches applied, the bug does not
occur anymore when running that test.

Cascardo.

>
> Christophe Jaillet (1):
>   CIFS: Fix some return values in case of error in 'crypt_message'
>
> Pavel Shilovsky (1):
>   CIFS: Fix null pointer deref during read resp processing
>
>  fs/cifs/cifsproto.h |  3 +--
>  fs/cifs/cifssmb.c   | 15 ++++++++-------
>  fs/cifs/smb2ops.c   |  8 +++++---
>  3 files changed, 14 insertions(+), 12 deletions(-)
>
> --
> 2.11.0
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

ACK: [PATCH 0/2] LP#1704857 Fix CIFS oops

Stefan Bader-2
In reply to this post by Thadeu Lima de Souza Cascardo-3
On 17.07.2017 21:29, Thadeu Lima de Souza Cascardo wrote:

> [Impact]
>
> Causes an oops, possibly causing a DOS on the system.
>
> [Regression Potential]
>
> Changes restricted to the CIFS filesystem code.
>
> [Test Case]
>
> Asked reported to explain how to reproduce the issue.
>
>
> Christophe Jaillet (1):
>   CIFS: Fix some return values in case of error in 'crypt_message'
>
> Pavel Shilovsky (1):
>   CIFS: Fix null pointer deref during read resp processing
>
>  fs/cifs/cifsproto.h |  3 +--
>  fs/cifs/cifssmb.c   | 15 ++++++++-------
>  fs/cifs/smb2ops.c   |  8 +++++---
>  3 files changed, 14 insertions(+), 12 deletions(-)
>
Successful testing.
Acked-by: Stefan Bader <[hidden email]>



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

APPLIED: [PATCH 0/2] LP#1704857 Fix CIFS oops

Kleber Souza
In reply to this post by Thadeu Lima de Souza Cascardo-3
Applied to xenial/master-next branch.

Thanks,
Kleber

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Loading...