RE: [PATCH 5.2 02/45] smb3: fix unmount hang in open_shroot

2019-10-01 Thread Pavel Shilovskiy 
-Original Message-
From: Sasha Levin  
Sent: Tuesday, October 1, 2019 3:49 PM
> On Tue, Oct 01, 2019 at 08:41:43PM +0000, Pavel Shilovskiy wrote:
> >Hi Greg,
> >
> >Are you going to apply this patch to the 5.3.y stable kernel? The patch is 
> >applicable there too.
> 
> I will, yes.

Thanks!

Best regards,
Pavel Shilovsky

RE: [PATCH 5.2 02/45] smb3: fix unmount hang in open_shroot

2019-10-01 Thread Pavel Shilovskiy 
Hi Greg,

Are you going to apply this patch to the 5.3.y stable kernel? The patch is 
applicable there too.

Best regards,
Pavel Shilovsky

-Original Message-
From: Greg Kroah-Hartman  
Sent: Sunday, September 29, 2019 6:56 AM
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman ; sta...@vger.kernel.org; 
kbuild test robot ; Dan Carpenter ; 
Pavel Shilovskiy ; Steven French 
; Aurelien Aptel ; Sasha Levin 

Subject: [PATCH 5.2 02/45] smb3: fix unmount hang in open_shroot

From: Steve French 

[ Upstream commit 96d9f7ed00b86104bf03adeffc8980897e9694ab ]

An earlier patch "CIFS: fix deadlock in cached root handling"
did not completely address the deadlock in open_shroot. This patch addresses 
the deadlock.

In testing the recent patch:
  smb3: improve handling of share deleted (and share recreated) we were able to 
reproduce the open_shroot deadlock to one of the target servers in unmount in a 
delete share scenario.

Fixes: 7e5a70ad88b1e ("CIFS: fix deadlock in cached root handling")

This is version 2 of this patch. An earlier version of this patch "smb3: fix 
unmount hang in open_shroot" had a problem found by Dan.

Reported-by: kbuild test robot 
Reported-by: Dan Carpenter 

Suggested-by: Pavel Shilovsky 
Reviewed-by: Pavel Shilovsky 
Signed-off-by: Steve French 
CC: Aurelien Aptel 
CC: Stable 
Signed-off-by: Sasha Levin 
---
 fs/cifs/smb2ops.c | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c index 
42de31d206169..8ae8ef526b4a5 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -656,6 +656,15 @@ int open_shroot(unsigned int xid, struct cifs_tcon *tcon, 
struct cifs_fid *pfid)
return 0;
}
 
+   /*
+* We do not hold the lock for the open because in case
+* SMB2_open needs to reconnect, it will end up calling
+* cifs_mark_open_files_invalid() which takes the lock again
+* thus causing a deadlock
+*/
+
+   mutex_unlock(>crfid.fid_mutex);
+
if (smb3_encryption_required(tcon))
flags |= CIFS_TRANSFORM_REQ;
 
@@ -677,7 +686,7 @@ int open_shroot(unsigned int xid, struct cifs_tcon *tcon, 
struct cifs_fid *pfid)
 
rc = SMB2_open_init(tcon, [0], , , _path);
if (rc)
-   goto oshr_exit;
+   goto oshr_free;
smb2_set_next_command(tcon, [0]);
 
memset(_iov, 0, sizeof(qi_iov));
@@ -690,18 +699,10 @@ int open_shroot(unsigned int xid, struct cifs_tcon *tcon, 
struct cifs_fid *pfid)
  sizeof(struct smb2_file_all_info) +
  PATH_MAX * 2, 0, NULL);
if (rc)
-   goto oshr_exit;
+   goto oshr_free;
 
smb2_set_related([1]);
 
-   /*
-* We do not hold the lock for the open because in case
-* SMB2_open needs to reconnect, it will end up calling
-* cifs_mark_open_files_invalid() which takes the lock again
-* thus causing a deadlock
-*/
-
-   mutex_unlock(>crfid.fid_mutex);
rc = compound_send_recv(xid, ses, flags, 2, rqst,
resp_buftype, rsp_iov);
mutex_lock(>crfid.fid_mutex);
--
2.20.1

RE: [PATCH 4.4 041/241] cifs: fix strcat buffer overflow and reduce raciness in smb21_set_oplock_level()

2019-06-11 Thread Pavel Shilovskiy 
-Original Message-
From: Greg Kroah-Hartman  
Sent: Tuesday, June 11, 2019 12:20 AM
To: Pavel Shilovskiy 
Cc: linux-kernel@vger.kernel.org; sta...@vger.kernel.org; Christoph Probst 
; Steven French 
Subject: Re: [PATCH 4.4 041/241] cifs: fix strcat buffer overflow and reduce 
raciness in smb21_set_oplock_level()

On Mon, Jun 10, 2019 at 07:13:24PM +, Pavel Shilovskiy wrote:
> 
> -Original Message-
> From: Greg Kroah-Hartman 
> Sent: Sunday, June 9, 2019 9:40 AM
> To: linux-kernel@vger.kernel.org
> Cc: Greg Kroah-Hartman ; 
> sta...@vger.kernel.org; Christoph Probst ; Pavel 
> Shilovskiy ; Steven French 
> 
> Subject: [PATCH 4.4 041/241] cifs: fix strcat buffer overflow and 
> reduce raciness in smb21_set_oplock_level()
> 
> From: Christoph Probst 
> 
> commit 6a54b2e002c9d00b398d35724c79f9fe0d9b38fb upstream.
> 
> Change strcat to strncpy in the "None" case to fix a buffer overflow when 
> cinode->oplock is reset to 0 by another thread accessing the same cinode. It 
> is never valid to append "None" to any other message.
> 
> Consolidate multiple writes to cinode->oplock to reduce raciness.
> 
> Signed-off-by: Christoph Probst 
> Reviewed-by: Pavel Shilovsky 
> Signed-off-by: Steve French 
> CC: Stable 
> Signed-off-by: Greg Kroah-Hartman 
> 
> 
> Hi Greg,
> 
> This patch has been queued for 4.4.y and has already been merged into 
> 5.1.y (5.1.5). Are you going to apply it to other stable kernels: 4.9, 
> 4.14, 4.19?

It is already in the 4.9.179, 4.14.122, 4.19.46, 5.0.19, and 5.1.5 released 
kernels.  So I don't think I can merge it into them again :)

thanks,

greg k-h
-

You are right, I missed it somehow. Thanks for clarifying!

Best regards,
Pavel Shilovsky

RE: [PATCH 4.4 041/241] cifs: fix strcat buffer overflow and reduce raciness in smb21_set_oplock_level()

2019-06-10 Thread Pavel Shilovskiy 

-Original Message-
From: Greg Kroah-Hartman  
Sent: Sunday, June 9, 2019 9:40 AM
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman ; sta...@vger.kernel.org; 
Christoph Probst ; Pavel Shilovskiy ; 
Steven French 
Subject: [PATCH 4.4 041/241] cifs: fix strcat buffer overflow and reduce 
raciness in smb21_set_oplock_level()

From: Christoph Probst 

commit 6a54b2e002c9d00b398d35724c79f9fe0d9b38fb upstream.

Change strcat to strncpy in the "None" case to fix a buffer overflow when 
cinode->oplock is reset to 0 by another thread accessing the same cinode. It is 
never valid to append "None" to any other message.

Consolidate multiple writes to cinode->oplock to reduce raciness.

Signed-off-by: Christoph Probst 
Reviewed-by: Pavel Shilovsky 
Signed-off-by: Steve French 
CC: Stable 
Signed-off-by: Greg Kroah-Hartman 


Hi Greg,

This patch has been queued for 4.4.y and has already been merged into 5.1.y 
(5.1.5). Are you going to apply it to other stable kernels: 4.9, 4.14, 4.19?

Best regards,
Pavel Shilovsky

RE: [PATCH AUTOSEL 4.20 65/77] CIFS: Do not assume one credit for async responses

2019-02-15 Thread Pavel Shilovskiy 
чт, 14 февр. 2019 г. в 18:40, Sasha Levin :
>
> From: Pavel Shilovsky 
>
> [ Upstream commit 0fd1d37b0501efc6e295f56ab55cdaff784aa50c ]
>
> If we don't receive a response we can't assume that the server
> granted one credit. Assume zero credits in such cases.
>
> Signed-off-by: Pavel Shilovsky 
> Reviewed-by: Ronnie Sahlberg 
> Signed-off-by: Steve French 
> Signed-off-by: Sasha Levin 
> ---
>  fs/cifs/smb2pdu.c | 15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index d1ae7cdb236d..3c44c51310c4 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -2834,9 +2834,10 @@ smb2_echo_callback(struct mid_q_entry *mid)
>  {
> struct TCP_Server_Info *server = mid->callback_data;
> struct smb2_echo_rsp *rsp = (struct smb2_echo_rsp *)mid->resp_buf;
> -   unsigned int credits_received = 1;
> +   unsigned int credits_received = 0;
>
> -   if (mid->mid_state == MID_RESPONSE_RECEIVED)
> +   if (mid->mid_state == MID_RESPONSE_RECEIVED
> +   || mid->mid_state == MID_RESPONSE_MALFORMED)
> credits_received = le16_to_cpu(rsp->sync_hdr.CreditRequest);
>
> DeleteMidQEntry(mid);
> @@ -3093,7 +3094,7 @@ smb2_readv_callback(struct mid_q_entry *mid)
> struct TCP_Server_Info *server = tcon->ses->server;
> struct smb2_sync_hdr *shdr =
> (struct smb2_sync_hdr 
> *)rdata->iov[0].iov_base;
> -   unsigned int credits_received = 1;
> +   unsigned int credits_received = 0;
> struct smb_rqst rqst = { .rq_iov = rdata->iov,
>  .rq_nvec = 2,
>  .rq_pages = rdata->pages,
> @@ -3132,6 +3133,9 @@ smb2_readv_callback(struct mid_q_entry *mid)
> task_io_account_read(rdata->got_bytes);
> cifs_stats_bytes_read(tcon, rdata->got_bytes);
> break;
> +   case MID_RESPONSE_MALFORMED:
> +   credits_received = le16_to_cpu(shdr->CreditRequest);
> +   /* fall through */
> default:
> if (rdata->result != -ENODATA)
> rdata->result = -EIO;
> @@ -3325,7 +3329,7 @@ smb2_writev_callback(struct mid_q_entry *mid)
> struct cifs_tcon *tcon = tlink_tcon(wdata->cfile->tlink);
> unsigned int written;
> struct smb2_write_rsp *rsp = (struct smb2_write_rsp *)mid->resp_buf;
> -   unsigned int credits_received = 1;
> +   unsigned int credits_received = 0;
>
> switch (mid->mid_state) {
> case MID_RESPONSE_RECEIVED:
> @@ -3353,6 +3357,9 @@ smb2_writev_callback(struct mid_q_entry *mid)
> case MID_RETRY_NEEDED:
> wdata->result = -EAGAIN;
> break;
> +   case MID_RESPONSE_MALFORMED:
> +   credits_received = le16_to_cpu(rsp->sync_hdr.CreditRequest);
> +   /* fall through */
> default:
> wdata->result = -EIO;
> break;
> --
> 2.19.1
>

Can you also apply the following patch to 4.20.y and 4.19.y, please?

https://patchwork.ozlabs.org/patch/1030180/

Best regards,
Pavel Shilovsky

RE: [PATCH AUTOSEL 4.20 40/42] CIFS: Move credit processing to mid callbacks for SMB3

2019-02-11 Thread Pavel Shilovskiy 


Sat, Feb 9 2019, 10:56AM, Sasha Levin  wrote:
>
> From: Pavel Shilovsky 
>
> [ Upstream commit ee258d79159afed52ca9372aeb9c1a51e89b32ee ]
>
> Currently we account for credits in the thread initiating a request
> and waiting for a response. The demultiplex thread receives the response,
> wakes up the thread and the latter collects credits from the response
> buffer and add them to the server structure on the client. This approach
> is not accurate, because it may race with reconnect events in the
> demultiplex thread which resets the number of credits.
>
> Fix this by moving credit processing to new mid callbacks that collect
> credits granted by the server from the response in the demultiplex thread.
>
> Signed-off-by: Pavel Shilovsky 
> Signed-off-by: Steve French 
> Signed-off-by: Sasha Levin 
> ---
>  fs/cifs/transport.c | 51 ++---
>  1 file changed, 34 insertions(+), 17 deletions(-)
>
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index 4dbf62bb51b2..0dab276eced8 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -781,12 +781,7 @@ cifs_setup_request(struct cifs_ses *ses, struct smb_rqst 
> *rqst)
>  }
>
>  static void
> -cifs_noop_callback(struct mid_q_entry *mid)
> -{
> -}
> -
> -static void
> -cifs_cancelled_callback(struct mid_q_entry *mid)
> +cifs_compound_callback(struct mid_q_entry *mid)
>  {
> struct TCP_Server_Info *server = mid->server;
> unsigned int optype = mid->optype;
> @@ -799,10 +794,23 @@ cifs_cancelled_callback(struct mid_q_entry *mid)
> cifs_dbg(FYI, "Bad state for cancelled MID\n");
> }
>
> -   DeleteMidQEntry(mid);
> add_credits(server, credits_received, optype);
>  }
>
> +static void
> +cifs_compound_last_callback(struct mid_q_entry *mid)
> +{
> +   cifs_compound_callback(mid);
> +   cifs_wake_up_task(mid);
> +}
> +
> +static void
> +cifs_cancelled_callback(struct mid_q_entry *mid)
> +{
> +   cifs_compound_callback(mid);
> +   DeleteMidQEntry(mid);
> +}
> +
>  int
>  compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
>const int flags, const int num_rqst, struct smb_rqst *rqst,
> @@ -880,11 +888,14 @@ compound_send_recv(const unsigned int xid, struct 
> cifs_ses *ses,
> midQ[i]->mid_state = MID_REQUEST_SUBMITTED;
> midQ[i]->optype = optype;
> /*
> -* We don't invoke the callback compounds unless it is the 
> last
> -* request.
> +* Invoke callback for every part of the compound chain
> +* to calculate credits properly. Wake up this thread only 
> when
> +* the last element is received.
>  */
> if (i < num_rqst - 1)
> -   midQ[i]->callback = cifs_noop_callback;
> +   midQ[i]->callback = cifs_compound_callback;
> +   else
> +   midQ[i]->callback = cifs_compound_last_callback;
> }
> cifs_in_send_inc(ses->server);
> rc = smb_send_rqst(ses->server, num_rqst, rqst, flags);
> @@ -898,8 +909,20 @@ compound_send_recv(const unsigned int xid, struct 
> cifs_ses *ses,
>
> mutex_unlock(>server->srv_mutex);
>
> -   if (rc < 0)
> +   if (rc < 0) {
> +   /* Sending failed for some reason - return credits back */
> +   for (i = 0; i < num_rqst; i++)
> +   add_credits(ses->server, credits[i], optype);
> goto out;
> +   }
> +
> +   /*
> +* At this point the request is passed to the network stack - we 
> assume
> +* that any credits taken from the server structure on the client have
> +* been spent and we can't return them back. Once we receive responses
> +* we will collect credits granted by the server in the mid callbacks
> +* and add those credits to the server structure.
> +*/
>
> /*
>  * Compounding is never used during session establish.
> @@ -932,11 +955,6 @@ compound_send_recv(const unsigned int xid, struct 
> cifs_ses *ses,
> }
> }
>
> -   for (i = 0; i < num_rqst; i++)
> -   if (!cancelled_mid[i] && midQ[i]->resp_buf
> -   && (midQ[i]->mid_state == MID_RESPONSE_RECEIVED))
> -   credits[i] = ses->server->ops->get_credits(midQ[i]);
> -
> for (i = 0; i < num_rqst; i++) {
> if (rc < 0)
> goto out;
> @@ -995,7 +1013,6 @@ out:
> for (i = 0; i < num_rqst; i++) {
> if (!cancelled_mid[i])
> cifs_delete_mid(midQ[i]);
> -   add_credits(ses->server, credits[i], optype);
> }
>
> return rc;
> --
> 2.19.1
>

Please apply the following two patches too:

https://patchwork.ozlabs.org/patch/1030180/

RE: [PATCH 4.20 14/57] CIFS: Do not hide EINTR after sending network packets

2019-01-15 Thread Pavel Shilovskiy 
Hi Greg,

I am wondering if it is possible to include exact stable kernel version (e.g. 
4.20.3 in this case) in the email. This would help to quickly understand which 
kernel version should be installed in order to get the fix.

Best regards,
Pavel Shilovsky

-Original Message-
From: Greg Kroah-Hartman  
Sent: Tuesday, January 15, 2019 8:36 AM
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman ; sta...@vger.kernel.org; 
Pavel Shilovskiy ; Jeff Layton ; 
Steven French 
Subject: [PATCH 4.20 14/57] CIFS: Do not hide EINTR after sending network 
packets

4.20-stable review patch.  If anyone has any objections, please let me know.

--

From: Pavel Shilovsky 

commit ee13919c2e8d1f904e035ad4b4239029a8994131 upstream.

Currently we hide EINTR code returned from sock_sendmsg() and return 0 instead. 
This makes a caller think that we successfully completed the network operation 
which is not true. Fix this by properly returning EINTR to callers.

Cc: 
Signed-off-by: Pavel Shilovsky 
Reviewed-by: Jeff Layton 
Signed-off-by: Steve French 
Signed-off-by: Greg Kroah-Hartman 

---
 fs/cifs/transport.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -385,7 +385,7 @@ smbd_done:
if (rc < 0 && rc != -EINTR)
cifs_dbg(VFS, "Error %d sending data on socket to server\n",
 rc);
-   else
+   else if (rc > 0)
rc = 0;
 
return rc;

RE: [Patch v8 00/16] CIFS: Implement SMB Direct protocol

2017-12-28 Thread Pavel Shilovskiy 
2017-11-22 16:38 GMT-08:00 Long Li :
> From: Long Li 
>
> Starting with SMB2 dialect 3.0, Microsoft introduced SMB Direct transport
> protocol for transferring upper layer (SMB2) payload over RDMA via Infiniband,
> RoCE or iWARP. The prococol is published in [MS-SMBD]
> (https://msdn.microsoft.com/en-us/library/hh536346.aspx).
>
> Change log:
> v2:
> Implemented RDMA read/write via memory registration.
> Re-arranged patches for review [Christoph Hellwig ].
> Restructured the code and fixed bugs on protocol timer and keepalive
> [Tom Talpey ].
>
> v3:
> Improved performance by introducing an additional queue for handling
> empty packets and reducing lock contention on IRQ path.
> Added light weight profiling by reading TSC.
> Improved the code for checking SMB versions when mounting with rdma option
> [Leon Romanovsky ].
> Moved to use pages instead of buffers for passing I/O for RDMA
> [Christoph Hellwig ].
> Removed redundant code and refactored I/O code paths
> [Christoph Hellwig , Tom Talpey ].
>
> v4:
> Fixed connectivity issues with iWAPR devices.
> Exported configurable protocol parameters to /proc/fs/cifs
> [Steve French ]
> Re-arranged patches for review
> [Pavel Shilovsky ].
>
> v5:
> Fixed compiling errors on ia64, i386 and when INFINIBAND is not
> configured. [kbuild test robot]
> Profiling is removed and will be introduced in a seperate patch.
>
> v6:
> Report internal code error via WARN_ON(). Change description in
> Kconfig [Pavel Shilovsky ].
>
> v7:
> Removed the use of #ifdef CONFIG_CIFS_SMB_DIRECT in upper layer
> code calling transport I/O. [Matthew Wilcox ,
> Tom Talpey ]
>
> v8:
> Fixed comment typos and removed unused code
> [Ronnie Sahlberg ]
> Removed the first 7 patches in v7, which have been merged.
> Restructed the code to move relevant function definitions to later patches.
> [Pavel Shilovsky ]
> Added a patch to disable signing when RDMA is in use. Signing will be
> enabled after it is properly implemented in SMB Direct code paths.
> Fixed an issue on passing the incorrect offset to smbd_buffer_descriptor_v1
> when RDMA read is used for SMB write. This was caused by recent CIFS
> patches to remove packet length from SMB2 packet header.
>
> Long Li (16):
>   CIFS: SMBD: Upper layer connects to SMBDirect session
>   CIFS: SMBD: Implement function to reconnect to a SMB Direct transport
>   CIFS: SMBD: Upper layer reconnects to SMB Direct session
>   CIFS: SMBD: Implement function to destroy a SMB Direct connection
>   CIFS: SMBD: Upper layer destroys SMB Direct session on shutdown or
> umount
>   CIFS: SMBD: Set SMB Direct maximum read or write size for I/O
>   CIFS: SMBD: Implement function to receive data via RDMA receive
>   CIFS: SMBD: Upper layer receives data via RDMA receive
>   CIFS: SMBD: Implement function to send data via RDMA send
>   CIFS: SMBD: Upper layer sends data via RDMA send
>   CIFS: SMBD: Implement RDMA memory registration
>   CIFS: SMBD: Upper layer performs SMB write via RDMA read through
> memory registration
>   CIFS: SMBD: Read correct returned data length for RDMA write (SMB
> read) I/O
>   CIFS: SMBD: Upper layer performs SMB read via RDMA write through
> memory registration
>   CIFS: SMBD: Add SMB Direct debug counters
>   CIFS: SMBD: Disable signing on SMB direct transport
>
>  fs/cifs/cifs_debug.c |  66 
>  fs/cifs/cifsglob.h   |  16 +-
>  fs/cifs/cifssmb.c|  15 +-
>  fs/cifs/connect.c|  46 ++-
>  fs/cifs/file.c   |  17 +-
>  fs/cifs/smb1ops.c|   4 +-
>  fs/cifs/smb2ops.c|  24 +-
>  fs/cifs/smb2pdu.c| 117 ++-
>  fs/cifs/smbdirect.c  | 947 
> +++
>  fs/cifs/smbdirect.h  |  72 
>  fs/cifs/transport.c  |   8 +-
>  11 files changed, 1308 insertions(+), 24 deletions(-)
>
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reviewed-by: Pavel Shilovsky 

--
Best regards,
Pavel Shilovsky

RE: [Patch v8 00/16] CIFS: Implement SMB Direct protocol

2017-12-28 Thread Pavel Shilovskiy 
2017-11-22 16:38 GMT-08:00 Long Li :
> From: Long Li 
>
> Starting with SMB2 dialect 3.0, Microsoft introduced SMB Direct transport
> protocol for transferring upper layer (SMB2) payload over RDMA via Infiniband,
> RoCE or iWARP. The prococol is published in [MS-SMBD]
> (https://msdn.microsoft.com/en-us/library/hh536346.aspx).
>
> Change log:
> v2:
> Implemented RDMA read/write via memory registration.
> Re-arranged patches for review [Christoph Hellwig ].
> Restructured the code and fixed bugs on protocol timer and keepalive
> [Tom Talpey ].
>
> v3:
> Improved performance by introducing an additional queue for handling
> empty packets and reducing lock contention on IRQ path.
> Added light weight profiling by reading TSC.
> Improved the code for checking SMB versions when mounting with rdma option
> [Leon Romanovsky ].
> Moved to use pages instead of buffers for passing I/O for RDMA
> [Christoph Hellwig ].
> Removed redundant code and refactored I/O code paths
> [Christoph Hellwig , Tom Talpey ].
>
> v4:
> Fixed connectivity issues with iWAPR devices.
> Exported configurable protocol parameters to /proc/fs/cifs
> [Steve French ]
> Re-arranged patches for review
> [Pavel Shilovsky ].
>
> v5:
> Fixed compiling errors on ia64, i386 and when INFINIBAND is not
> configured. [kbuild test robot]
> Profiling is removed and will be introduced in a seperate patch.
>
> v6:
> Report internal code error via WARN_ON(). Change description in
> Kconfig [Pavel Shilovsky ].
>
> v7:
> Removed the use of #ifdef CONFIG_CIFS_SMB_DIRECT in upper layer
> code calling transport I/O. [Matthew Wilcox ,
> Tom Talpey ]
>
> v8:
> Fixed comment typos and removed unused code
> [Ronnie Sahlberg ]
> Removed the first 7 patches in v7, which have been merged.
> Restructed the code to move relevant function definitions to later patches.
> [Pavel Shilovsky ]
> Added a patch to disable signing when RDMA is in use. Signing will be
> enabled after it is properly implemented in SMB Direct code paths.
> Fixed an issue on passing the incorrect offset to smbd_buffer_descriptor_v1
> when RDMA read is used for SMB write. This was caused by recent CIFS
> patches to remove packet length from SMB2 packet header.
>
> Long Li (16):
>   CIFS: SMBD: Upper layer connects to SMBDirect session
>   CIFS: SMBD: Implement function to reconnect to a SMB Direct transport
>   CIFS: SMBD: Upper layer reconnects to SMB Direct session
>   CIFS: SMBD: Implement function to destroy a SMB Direct connection
>   CIFS: SMBD: Upper layer destroys SMB Direct session on shutdown or
> umount
>   CIFS: SMBD: Set SMB Direct maximum read or write size for I/O
>   CIFS: SMBD: Implement function to receive data via RDMA receive
>   CIFS: SMBD: Upper layer receives data via RDMA receive
>   CIFS: SMBD: Implement function to send data via RDMA send
>   CIFS: SMBD: Upper layer sends data via RDMA send
>   CIFS: SMBD: Implement RDMA memory registration
>   CIFS: SMBD: Upper layer performs SMB write via RDMA read through
> memory registration
>   CIFS: SMBD: Read correct returned data length for RDMA write (SMB
> read) I/O
>   CIFS: SMBD: Upper layer performs SMB read via RDMA write through
> memory registration
>   CIFS: SMBD: Add SMB Direct debug counters
>   CIFS: SMBD: Disable signing on SMB direct transport
>
>  fs/cifs/cifs_debug.c |  66 
>  fs/cifs/cifsglob.h   |  16 +-
>  fs/cifs/cifssmb.c|  15 +-
>  fs/cifs/connect.c|  46 ++-
>  fs/cifs/file.c   |  17 +-
>  fs/cifs/smb1ops.c|   4 +-
>  fs/cifs/smb2ops.c|  24 +-
>  fs/cifs/smb2pdu.c| 117 ++-
>  fs/cifs/smbdirect.c  | 947 
> +++
>  fs/cifs/smbdirect.h  |  72 
>  fs/cifs/transport.c  |   8 +-
>  11 files changed, 1308 insertions(+), 24 deletions(-)
>
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reviewed-by: Pavel Shilovsky 

--
Best regards,
Pavel Shilovsky

RE: [Patch v7 03/22] CIFS: SMBD: Add rdma mount option

2017-11-16 Thread Pavel Shilovskiy 

2017-11-07 0:54 GMT-08:00 Long Li :
> From: Long Li 
>
> Add "rdma" to CIFS mount options to connect to SMB Direct.
> Add checks to validate this is used on SMB 3.X dialects.
>
> To connect to SMBDirect, use "mount.cifs -o rdma,vers=3.x".
> At the time of this patch, 3.x can be 3.0, 3.02 or 3.1.1.
>
> Signed-off-by: Long Li 
> ---
>  fs/cifs/cifs_debug.c |  2 ++
>  fs/cifs/cifsfs.c |  2 ++
>  fs/cifs/cifsglob.h   |  5 +
>  fs/cifs/connect.c| 15 ++-
>  4 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
> index 9727e1d..ba0870d 100644
> --- a/fs/cifs/cifs_debug.c
> +++ b/fs/cifs/cifs_debug.c
> @@ -171,6 +171,8 @@ static int cifs_debug_data_proc_show(struct seq_file *m, 
> void *v)
> ses->ses_count, ses->serverOS, ses->serverNOS,
> ses->capabilities, ses->status);
> }
> +   if (server->rdma)
> +   seq_printf(m, "RDMA\n\t");
> seq_printf(m, "TCP status: %d\n\tLocal Users To "
>"Server: %d SecMode: 0x%x Req On Wire: %d",
>server->tcpStatus, server->srv_count,
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 180b335..e15fbf1 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -327,6 +327,8 @@ cifs_show_address(struct seq_file *s, struct 
> TCP_Server_Info *server)
> default:
> seq_puts(s, "(unknown)");
> }
> +   if (server->rdma)
> +   seq_puts(s, ",rdma");
>  }
>
>  static void
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 808486c..5585516 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -530,6 +530,7 @@ struct smb_vol {
> bool nopersistent:1;
> bool resilient:1; /* noresilient not required since not fored for CA 
> */
> bool domainauto:1;
> +   bool rdma:1;
> unsigned int rsize;
> unsigned int wsize;
> bool sockopt_tcp_nodelay:1;
> @@ -646,6 +647,10 @@ struct TCP_Server_Info {
> boolsec_kerberos;   /* supports plain Kerberos */
> boolsec_mskerberos; /* supports legacy MS Kerberos */
> boollarge_buf;  /* is current buffer large? */
> +   /* use SMBD connection instead of socket */
> +   boolrdma;
> +   /* point to the SMBD connection if RDMA is used instead of socket */
> +   struct smbd_connection *smbd_conn;
> struct delayed_work echo; /* echo ping workqueue job */
> char*smallbuf;  /* pointer to current "small" buffer */
> char*bigbuf;/* pointer to current "big" buffer */
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 59647eb..b5a575f 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -92,7 +92,7 @@ enum {
> Opt_multiuser, Opt_sloppy, Opt_nosharesock,
> Opt_persistent, Opt_nopersistent,
> Opt_resilient, Opt_noresilient,
> -   Opt_domainauto,
> +   Opt_domainauto, Opt_rdma,
>
> /* Mount options which take numeric value */
> Opt_backupuid, Opt_backupgid, Opt_uid,
> @@ -183,6 +183,7 @@ static const match_table_t cifs_mount_option_tokens = {
> { Opt_resilient, "resilienthandles"},
> { Opt_noresilient, "noresilienthandles"},
> { Opt_domainauto, "domainauto"},
> +   { Opt_rdma, "rdma"},
>
> { Opt_backupuid, "backupuid=%s" },
> { Opt_backupgid, "backupgid=%s" },
> @@ -1538,6 +1539,9 @@ cifs_parse_mount_options(const char *mountdata, const 
> char *devname,
> case Opt_domainauto:
> vol->domainauto = true;
> break;
> +   case Opt_rdma:
> +   vol->rdma = true;
> +   break;
>
> /* Numeric Values */
> case Opt_backupuid:
> @@ -1928,6 +1932,11 @@ cifs_parse_mount_options(const char *mountdata, const 
> char *devname,
> goto cifs_parse_mount_err;
> }
>
> +   if (vol->rdma && vol->vals->protocol_id < SMB30_PROT_ID) {
> +   cifs_dbg(VFS, "SMB Direct requires Version >=3.0\n");
> +   goto cifs_parse_mount_err;
> +   }
> +
>  #ifndef CONFIG_KEYS
> /* Muliuser mounts require CONFIG_KEYS support */
> if (vol->multiuser) {
> @@ -2131,6 +2140,9 @@ static int match_server(struct TCP_Server_Info *server, 
> struct smb_vol *vol)
> if (server->echo_interval != vol->echo_interval * HZ)
> return 0;
>
> +   if (server->rdma != vol->rdma)
> +   return 0;
> +
> return 1;
>  }
>
> @@ -2229,6 +2241,7 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
> tcp_ses->noblocksnd =

RE: [Patch v7 03/22] CIFS: SMBD: Add rdma mount option

2017-11-16 Thread Pavel Shilovskiy 

2017-11-07 0:54 GMT-08:00 Long Li :
> From: Long Li 
>
> Add "rdma" to CIFS mount options to connect to SMB Direct.
> Add checks to validate this is used on SMB 3.X dialects.
>
> To connect to SMBDirect, use "mount.cifs -o rdma,vers=3.x".
> At the time of this patch, 3.x can be 3.0, 3.02 or 3.1.1.
>
> Signed-off-by: Long Li 
> ---
>  fs/cifs/cifs_debug.c |  2 ++
>  fs/cifs/cifsfs.c |  2 ++
>  fs/cifs/cifsglob.h   |  5 +
>  fs/cifs/connect.c| 15 ++-
>  4 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
> index 9727e1d..ba0870d 100644
> --- a/fs/cifs/cifs_debug.c
> +++ b/fs/cifs/cifs_debug.c
> @@ -171,6 +171,8 @@ static int cifs_debug_data_proc_show(struct seq_file *m, 
> void *v)
> ses->ses_count, ses->serverOS, ses->serverNOS,
> ses->capabilities, ses->status);
> }
> +   if (server->rdma)
> +   seq_printf(m, "RDMA\n\t");
> seq_printf(m, "TCP status: %d\n\tLocal Users To "
>"Server: %d SecMode: 0x%x Req On Wire: %d",
>server->tcpStatus, server->srv_count,
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 180b335..e15fbf1 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -327,6 +327,8 @@ cifs_show_address(struct seq_file *s, struct 
> TCP_Server_Info *server)
> default:
> seq_puts(s, "(unknown)");
> }
> +   if (server->rdma)
> +   seq_puts(s, ",rdma");
>  }
>
>  static void
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 808486c..5585516 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -530,6 +530,7 @@ struct smb_vol {
> bool nopersistent:1;
> bool resilient:1; /* noresilient not required since not fored for CA 
> */
> bool domainauto:1;
> +   bool rdma:1;
> unsigned int rsize;
> unsigned int wsize;
> bool sockopt_tcp_nodelay:1;
> @@ -646,6 +647,10 @@ struct TCP_Server_Info {
> boolsec_kerberos;   /* supports plain Kerberos */
> boolsec_mskerberos; /* supports legacy MS Kerberos */
> boollarge_buf;  /* is current buffer large? */
> +   /* use SMBD connection instead of socket */
> +   boolrdma;
> +   /* point to the SMBD connection if RDMA is used instead of socket */
> +   struct smbd_connection *smbd_conn;
> struct delayed_work echo; /* echo ping workqueue job */
> char*smallbuf;  /* pointer to current "small" buffer */
> char*bigbuf;/* pointer to current "big" buffer */
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 59647eb..b5a575f 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -92,7 +92,7 @@ enum {
> Opt_multiuser, Opt_sloppy, Opt_nosharesock,
> Opt_persistent, Opt_nopersistent,
> Opt_resilient, Opt_noresilient,
> -   Opt_domainauto,
> +   Opt_domainauto, Opt_rdma,
>
> /* Mount options which take numeric value */
> Opt_backupuid, Opt_backupgid, Opt_uid,
> @@ -183,6 +183,7 @@ static const match_table_t cifs_mount_option_tokens = {
> { Opt_resilient, "resilienthandles"},
> { Opt_noresilient, "noresilienthandles"},
> { Opt_domainauto, "domainauto"},
> +   { Opt_rdma, "rdma"},
>
> { Opt_backupuid, "backupuid=%s" },
> { Opt_backupgid, "backupgid=%s" },
> @@ -1538,6 +1539,9 @@ cifs_parse_mount_options(const char *mountdata, const 
> char *devname,
> case Opt_domainauto:
> vol->domainauto = true;
> break;
> +   case Opt_rdma:
> +   vol->rdma = true;
> +   break;
>
> /* Numeric Values */
> case Opt_backupuid:
> @@ -1928,6 +1932,11 @@ cifs_parse_mount_options(const char *mountdata, const 
> char *devname,
> goto cifs_parse_mount_err;
> }
>
> +   if (vol->rdma && vol->vals->protocol_id < SMB30_PROT_ID) {
> +   cifs_dbg(VFS, "SMB Direct requires Version >=3.0\n");
> +   goto cifs_parse_mount_err;
> +   }
> +
>  #ifndef CONFIG_KEYS
> /* Muliuser mounts require CONFIG_KEYS support */
> if (vol->multiuser) {
> @@ -2131,6 +2140,9 @@ static int match_server(struct TCP_Server_Info *server, 
> struct smb_vol *vol)
> if (server->echo_interval != vol->echo_interval * HZ)
> return 0;
>
> +   if (server->rdma != vol->rdma)
> +   return 0;
> +
> return 1;
>  }
>
> @@ -2229,6 +2241,7 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
> tcp_ses->noblocksnd = volume_info->noblocksnd;
> tcp_ses->noautotune = volume_info->noautotune;
>

RE: [Patch v7 02/22] CIFS: SMBD: Introduce kernel config option CONFIG_CIFS_SMB_DIRECT

2017-11-16 Thread Pavel Shilovskiy 
2017-11-07 0:54 GMT-08:00 Long Li :
> From: Long Li 
>
> Build SMB Direct code when this option is set.
>
> Signed-off-by: Long Li 
> ---
>  fs/cifs/Kconfig | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/fs/cifs/Kconfig b/fs/cifs/Kconfig
> index f724361..8d05fff 100644
> --- a/fs/cifs/Kconfig
> +++ b/fs/cifs/Kconfig
> @@ -191,6 +191,14 @@ config CIFS_SMB311
>   This dialect includes improved security negotiation features.
>   If unsure, say N
>
> +config CIFS_SMB_DIRECT
> +   bool "SMB Direct support (Experimental)"
> +   depends on CIFS && INFINIBAND
> +   help
> + Enables SMB Direct experimental support for SMB 3.0, 3.02 and 3.1.1.
> + SMB Direct allows transferring SMB packets over RDMA. If unsure,
> + say N.
> +
>  config CIFS_FSCACHE
>   bool "Provide CIFS client caching support"
>   depends on CIFS=m && FSCACHE || CIFS=y && FSCACHE=y
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reviewed-by: Pavel Shilovsky 

--
Best regards,
Pavel Shilovsky

RE: [Patch v7 02/22] CIFS: SMBD: Introduce kernel config option CONFIG_CIFS_SMB_DIRECT

2017-11-16 Thread Pavel Shilovskiy 
2017-11-07 0:54 GMT-08:00 Long Li :
> From: Long Li 
>
> Build SMB Direct code when this option is set.
>
> Signed-off-by: Long Li 
> ---
>  fs/cifs/Kconfig | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/fs/cifs/Kconfig b/fs/cifs/Kconfig
> index f724361..8d05fff 100644
> --- a/fs/cifs/Kconfig
> +++ b/fs/cifs/Kconfig
> @@ -191,6 +191,14 @@ config CIFS_SMB311
>   This dialect includes improved security negotiation features.
>   If unsure, say N
>
> +config CIFS_SMB_DIRECT
> +   bool "SMB Direct support (Experimental)"
> +   depends on CIFS && INFINIBAND
> +   help
> + Enables SMB Direct experimental support for SMB 3.0, 3.02 and 3.1.1.
> + SMB Direct allows transferring SMB packets over RDMA. If unsure,
> + say N.
> +
>  config CIFS_FSCACHE
>   bool "Provide CIFS client caching support"
>   depends on CIFS=m && FSCACHE || CIFS=y && FSCACHE=y
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reviewed-by: Pavel Shilovsky 

--
Best regards,
Pavel Shilovsky

RE: [Patch v7 01/22] CIFS: SMBD: Add parameter rdata to smb2_new_read_req

2017-11-16 Thread Pavel Shilovskiy 
2017-11-07 0:54 GMT-08:00 Long Li :
> From: Long Li 
>
> This patch is for preparing upper layer for doing SMB read via RDMA write.
>
> When we assemble the SMB read packet header, we need to know the I/O layout
> if this request is to use a RDMA write. rdata has all the information we need
> for memory registration. Add rdata to smb2_new_read_req.
>
> Signed-off-by: Long Li 
> ---
>  fs/cifs/smb2pdu.c | 14 +-
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index bab3da6..32ad590 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -2350,18 +2350,21 @@ SMB2_flush(const unsigned int xid, struct cifs_tcon 
> *tcon, u64 persistent_fid,
>   */
>  static int
>  smb2_new_read_req(void **buf, unsigned int *total_len,
> - struct cifs_io_parms *io_parms, unsigned int 
> remaining_bytes,
> - int request_type)
> +   struct cifs_io_parms *io_parms, struct cifs_readdata *rdata,
> +   unsigned int remaining_bytes, int request_type)
>  {
> int rc = -EACCES;
> struct smb2_read_plain_req *req = NULL;
> struct smb2_sync_hdr *shdr;
> +   struct TCP_Server_Info *server;
>
> rc = smb2_plain_req_init(SMB2_READ, io_parms->tcon, (void **) ,
>  total_len);
> if (rc)
> return rc;
> -   if (io_parms->tcon->ses->server == NULL)
> +
> +   server = io_parms->tcon->ses->server;
> +   if (server == NULL)
> return -ECONNABORTED;
>
> shdr = >sync_hdr;
> @@ -2489,7 +2492,8 @@ smb2_async_readv(struct cifs_readdata *rdata)
>
> server = io_parms.tcon->ses->server;
>
> -   rc = smb2_new_read_req((void **) , _len, _parms, 0, 0);
> +   rc = smb2_new_read_req(
> +   (void **) , _len, _parms, rdata, 0, 0);
> if (rc) {
> if (rc == -EAGAIN && rdata->credits) {
> /* credits was reset by reconnect */
> @@ -2557,7 +2561,7 @@ SMB2_read(const unsigned int xid, struct cifs_io_parms 
> *io_parms,
> struct cifs_ses *ses = io_parms->tcon->ses;
>
> *nbytes = 0;
> -   rc = smb2_new_read_req((void **), _len, io_parms, 0, 0);
> +   rc = smb2_new_read_req((void **), _len, io_parms, NULL, 0, 
> 0);
> if (rc)
> return rc;
>
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Looks good.

Reviewed-by: Pavel Shilovsky 

--
Best regards,
Pavel Shilovsky

RE: [Patch v7 01/22] CIFS: SMBD: Add parameter rdata to smb2_new_read_req

2017-11-16 Thread Pavel Shilovskiy 
2017-11-07 0:54 GMT-08:00 Long Li :
> From: Long Li 
>
> This patch is for preparing upper layer for doing SMB read via RDMA write.
>
> When we assemble the SMB read packet header, we need to know the I/O layout
> if this request is to use a RDMA write. rdata has all the information we need
> for memory registration. Add rdata to smb2_new_read_req.
>
> Signed-off-by: Long Li 
> ---
>  fs/cifs/smb2pdu.c | 14 +-
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index bab3da6..32ad590 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -2350,18 +2350,21 @@ SMB2_flush(const unsigned int xid, struct cifs_tcon 
> *tcon, u64 persistent_fid,
>   */
>  static int
>  smb2_new_read_req(void **buf, unsigned int *total_len,
> - struct cifs_io_parms *io_parms, unsigned int 
> remaining_bytes,
> - int request_type)
> +   struct cifs_io_parms *io_parms, struct cifs_readdata *rdata,
> +   unsigned int remaining_bytes, int request_type)
>  {
> int rc = -EACCES;
> struct smb2_read_plain_req *req = NULL;
> struct smb2_sync_hdr *shdr;
> +   struct TCP_Server_Info *server;
>
> rc = smb2_plain_req_init(SMB2_READ, io_parms->tcon, (void **) ,
>  total_len);
> if (rc)
> return rc;
> -   if (io_parms->tcon->ses->server == NULL)
> +
> +   server = io_parms->tcon->ses->server;
> +   if (server == NULL)
> return -ECONNABORTED;
>
> shdr = >sync_hdr;
> @@ -2489,7 +2492,8 @@ smb2_async_readv(struct cifs_readdata *rdata)
>
> server = io_parms.tcon->ses->server;
>
> -   rc = smb2_new_read_req((void **) , _len, _parms, 0, 0);
> +   rc = smb2_new_read_req(
> +   (void **) , _len, _parms, rdata, 0, 0);
> if (rc) {
> if (rc == -EAGAIN && rdata->credits) {
> /* credits was reset by reconnect */
> @@ -2557,7 +2561,7 @@ SMB2_read(const unsigned int xid, struct cifs_io_parms 
> *io_parms,
> struct cifs_ses *ses = io_parms->tcon->ses;
>
> *nbytes = 0;
> -   rc = smb2_new_read_req((void **), _len, io_parms, 0, 0);
> +   rc = smb2_new_read_req((void **), _len, io_parms, NULL, 0, 
> 0);
> if (rc)
> return rc;
>
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Looks good.

Reviewed-by: Pavel Shilovsky 

--
Best regards,
Pavel Shilovsky

RE: [PATCH 3.18 36/68] Handle mismatched open calls

2017-07-19 Thread Pavel Shilovskiy 
2017-07-14 9:43 Ben Hutchings :
> On Fri, 2017-05-05 at 11:32 -0700, Greg Kroah-Hartman wrote:
> > 3.18-stable review patch.  If anyone has any objections, please let me know.
> > 
> > --
> > 
> > From: Sachin Prabhu 
> > 
> > commit 38bd49064a1ecb67baad33598e3d824448ab11ec upstream.
> [...]
> > --- a/fs/cifs/smb2ops.c
> > +++ b/fs/cifs/smb2ops.c
> > @@ -1461,6 +1461,8 @@ struct smb_version_operations smb21_oper
> >     .clear_stats = smb2_clear_stats,
> >     .print_stats = smb2_print_stats,
> >     .is_oplock_break = smb2_is_valid_oplock_break,
> > +   .handle_cancelled_mid = smb2_handle_cancelled_mid,
> > +   .handle_cancelled_mid = smb2_handle_cancelled_mid,
> >     .downgrade_oplock = smb2_downgrade_oplock,
> >     .need_neg = smb2_need_neg,
> >     .negotiate = smb2_negotiate,
> > @@ -1542,6 +1544,8 @@ struct smb_version_operations smb30_oper
> >     .print_stats = smb2_print_stats,
> >     .dump_share_caps = smb2_dump_share_caps,
> >     .is_oplock_break = smb2_is_valid_oplock_break,
> > +   .handle_cancelled_mid = smb2_handle_cancelled_mid,
> > +   .handle_cancelled_mid = smb2_handle_cancelled_mid,
> >     .downgrade_oplock = smb2_downgrade_oplock,
> >     .need_neg = smb2_need_neg,
> >     .negotiate = smb2_negotiate,
> [...]
> 
> This doesn't look right.  handle_cancelled_mid should be initialised once in 
> each of the 3 smb_version_operations structures, shouldn't it?
>
> Ben.

Yes, you are right. Thanks for pointing it out.

Greg, I provided the patch to fix the above bug (see the attachment). Could you 
please look at it and apply to the 3.18.x kernel if it is suitable?

Best regards,
Pavel Shilovsky


0001-CIFS-Fix-handle_cancelled_mid-callback-initializatio.patch
Description: 0001-CIFS-Fix-handle_cancelled_mid-callback-initializatio.patch

RE: [PATCH 3.18 36/68] Handle mismatched open calls

2017-07-19 Thread Pavel Shilovskiy 
2017-07-14 9:43 Ben Hutchings :
> On Fri, 2017-05-05 at 11:32 -0700, Greg Kroah-Hartman wrote:
> > 3.18-stable review patch.  If anyone has any objections, please let me know.
> > 
> > --
> > 
> > From: Sachin Prabhu 
> > 
> > commit 38bd49064a1ecb67baad33598e3d824448ab11ec upstream.
> [...]
> > --- a/fs/cifs/smb2ops.c
> > +++ b/fs/cifs/smb2ops.c
> > @@ -1461,6 +1461,8 @@ struct smb_version_operations smb21_oper
> >     .clear_stats = smb2_clear_stats,
> >     .print_stats = smb2_print_stats,
> >     .is_oplock_break = smb2_is_valid_oplock_break,
> > +   .handle_cancelled_mid = smb2_handle_cancelled_mid,
> > +   .handle_cancelled_mid = smb2_handle_cancelled_mid,
> >     .downgrade_oplock = smb2_downgrade_oplock,
> >     .need_neg = smb2_need_neg,
> >     .negotiate = smb2_negotiate,
> > @@ -1542,6 +1544,8 @@ struct smb_version_operations smb30_oper
> >     .print_stats = smb2_print_stats,
> >     .dump_share_caps = smb2_dump_share_caps,
> >     .is_oplock_break = smb2_is_valid_oplock_break,
> > +   .handle_cancelled_mid = smb2_handle_cancelled_mid,
> > +   .handle_cancelled_mid = smb2_handle_cancelled_mid,
> >     .downgrade_oplock = smb2_downgrade_oplock,
> >     .need_neg = smb2_need_neg,
> >     .negotiate = smb2_negotiate,
> [...]
> 
> This doesn't look right.  handle_cancelled_mid should be initialised once in 
> each of the 3 smb_version_operations structures, shouldn't it?
>
> Ben.

Yes, you are right. Thanks for pointing it out.

Greg, I provided the patch to fix the above bug (see the attachment). Could you 
please look at it and apply to the 3.18.x kernel if it is suitable?

Best regards,
Pavel Shilovsky


0001-CIFS-Fix-handle_cancelled_mid-callback-initializatio.patch
Description: 0001-CIFS-Fix-handle_cancelled_mid-callback-initializatio.patch

RE: [PATCH 3.16 155/178] cifs: Do not send echoes before Negotiate is complete

2017-07-18 Thread Pavel Shilovskiy 
2017-07-16 6:57 Ben Hutchings :
> 
> 3.16.46-rc1 review patch.  If anyone has any objections, please let me know.
> 
> --
> 
> From: Sachin Prabhu 
> 
> commit 62a6cfddcc0a5313e7da3e8311ba16226fe0ac10 upstream.
> 
> commit 4fcd1813e640 ("Fix reconnect to not defer smb3 session reconnect long 
> after socket reconnect") added support for Negotiate requests to be initiated 
> by echo calls.
> 
> To avoid delays in calling echo after a reconnect, I added the patch 
> introduced by the commit b8c600120fc8 ("Call echo service immediately after 
> socket reconnect").
>
> This has however caused a regression with cifs shares which do not have 
> support for echo calls to trigger Negotiate requests. On connections which 
> need to call Negotiation, the echo calls trigger an error which triggers a 
> reconnect which in turn triggers another echo call. This results in a loop 
> which is only broken when an operation is performed on the cifs share. For an 
> idle share, it can DOS a server.
> 
> The patch uses the smb_operation can_echo() for cifs so that it is called 
> only if connection has been already been setup.
> 
> kernel bz: 194531
> 
> Signed-off-by: Sachin Prabhu 
> Tested-by: Jonathan Liu 
> Acked-by: Pavel Shilovsky 
> Signed-off-by: Steve French 
> Signed-off-by: Ben Hutchings 
> ---
> fs/cifs/smb1ops.c | 10 ++
> 1 file changed, 10 insertions(+)

I have just posted a backport of commit b8c600120fc8 ("Call echo service 
immediately after socket reconnect") for v3.16.x kernel to the stable mailing 
list. Please consider merging it too.

Best regards,
Pavel Shilovsky

RE: [PATCH 3.16 155/178] cifs: Do not send echoes before Negotiate is complete

2017-07-18 Thread Pavel Shilovskiy 
2017-07-16 6:57 Ben Hutchings :
> 
> 3.16.46-rc1 review patch.  If anyone has any objections, please let me know.
> 
> --
> 
> From: Sachin Prabhu 
> 
> commit 62a6cfddcc0a5313e7da3e8311ba16226fe0ac10 upstream.
> 
> commit 4fcd1813e640 ("Fix reconnect to not defer smb3 session reconnect long 
> after socket reconnect") added support for Negotiate requests to be initiated 
> by echo calls.
> 
> To avoid delays in calling echo after a reconnect, I added the patch 
> introduced by the commit b8c600120fc8 ("Call echo service immediately after 
> socket reconnect").
>
> This has however caused a regression with cifs shares which do not have 
> support for echo calls to trigger Negotiate requests. On connections which 
> need to call Negotiation, the echo calls trigger an error which triggers a 
> reconnect which in turn triggers another echo call. This results in a loop 
> which is only broken when an operation is performed on the cifs share. For an 
> idle share, it can DOS a server.
> 
> The patch uses the smb_operation can_echo() for cifs so that it is called 
> only if connection has been already been setup.
> 
> kernel bz: 194531
> 
> Signed-off-by: Sachin Prabhu 
> Tested-by: Jonathan Liu 
> Acked-by: Pavel Shilovsky 
> Signed-off-by: Steve French 
> Signed-off-by: Ben Hutchings 
> ---
> fs/cifs/smb1ops.c | 10 ++
> 1 file changed, 10 insertions(+)

I have just posted a backport of commit b8c600120fc8 ("Call echo service 
immediately after socket reconnect") for v3.16.x kernel to the stable mailing 
list. Please consider merging it too.

Best regards,
Pavel Shilovsky

RE: [PATCH][cifs-next] cifs: set oparms.create_options rather than or'ing in CREATE_OPEN_BACKUP_INTENT

2017-07-05 Thread Pavel Shilovskiy 
2017-07-05 5:47 GMT-07:00 Colin King :
> From: Colin Ian King 
>
> Currently oparms.create_options is uninitialized and the code is logically
> or'ing in CREATE_OPEN_BACKUP_INTENT onto a garbage value of
> oparms.create_options from the stack.  Fix this by just setting the value
> rather than or'ing in the setting.
>
> Detected by CoverityScan, CID#1447220 ("Unitialized scale value")
>
> Signed-off-by: Colin Ian King 
> ---
>  fs/cifs/smb2ops.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index 941c40b7a870..c805b7619083 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -1339,7 +1339,7 @@ get_smb2_acl_by_path(struct cifs_sb_info *cifs_sb,
> xid = get_xid();
>
> if (backup_cred(cifs_sb))
> -   oparms.create_options |= CREATE_OPEN_BACKUP_INTENT;
> +   oparms.create_options = CREATE_OPEN_BACKUP_INTENT;
> else
> oparms.create_options = 0;
>


Reviewed-by: Pavel Shilovsky 

--
Best regards,
Pavel Shilovsky

RE: [PATCH][cifs-next] cifs: set oparms.create_options rather than or'ing in CREATE_OPEN_BACKUP_INTENT

2017-07-05 Thread Pavel Shilovskiy 
2017-07-05 5:47 GMT-07:00 Colin King :
> From: Colin Ian King 
>
> Currently oparms.create_options is uninitialized and the code is logically
> or'ing in CREATE_OPEN_BACKUP_INTENT onto a garbage value of
> oparms.create_options from the stack.  Fix this by just setting the value
> rather than or'ing in the setting.
>
> Detected by CoverityScan, CID#1447220 ("Unitialized scale value")
>
> Signed-off-by: Colin Ian King 
> ---
>  fs/cifs/smb2ops.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index 941c40b7a870..c805b7619083 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -1339,7 +1339,7 @@ get_smb2_acl_by_path(struct cifs_sb_info *cifs_sb,
> xid = get_xid();
>
> if (backup_cred(cifs_sb))
> -   oparms.create_options |= CREATE_OPEN_BACKUP_INTENT;
> +   oparms.create_options = CREATE_OPEN_BACKUP_INTENT;
> else
> oparms.create_options = 0;
>


Reviewed-by: Pavel Shilovsky 

--
Best regards,
Pavel Shilovsky

RE: [PATCH] CIFS: fix circular locking dependency

2017-07-05 Thread Pavel Shilovskiy 


2017-06-29 7:01 GMT-07:00 Rabin Vincent :
> From: Rabin Vincent 
>
> When a CIFS filesystem is mounted with the forcemand option and the
> following command is run on it, lockdep warns about a circular locking
> dependency between CifsInodeInfo::lock_sem and the inode lock.
>
>  while echo foo > hello; do :; done & while touch -c hello; do :; done
>
> cifs_writev() takes the locks in the wrong order, but note that we can't
> only flip the order around because it releases the inode lock before the
> call to generic_write_sync() while it holds the lock_sem across that
> call.
>
> But, AFAICS, there is no need to hold the CifsInodeInfo::lock_sem across
> the generic_write_sync() call either, so we can release both the locks
> before generic_write_sync(), and change the order.
>
>  ==
>  WARNING: possible circular locking dependency detected
>  4.12.0-rc7+ #9 Not tainted
>  --
>  touch/487 is trying to acquire lock:
>   (>lock_sem){..}, at: cifsFileInfo_put+0x88f/0x16a0
>
>  but task is already holding lock:
>   (>s_type->i_mutex_key#11){+.+.+.}, at: utimes_common+0x3ad/0x870
>
>  which lock already depends on the new lock.
>
>  the existing dependency chain (in reverse order) is:
>
>  -> #1 (>s_type->i_mutex_key#11){+.+.+.}:
> __lock_acquire+0x1f74/0x38f0
> lock_acquire+0x1cc/0x600
> down_write+0x74/0x110
> cifs_strict_writev+0x3cb/0x8c0
> __vfs_write+0x4c1/0x930
> vfs_write+0x14c/0x2d0
> SyS_write+0xf7/0x240
> entry_SYSCALL_64_fastpath+0x1f/0xbe
>
>  -> #0 (>lock_sem){..}:
> check_prevs_add+0xfa0/0x1d10
> __lock_acquire+0x1f74/0x38f0
> lock_acquire+0x1cc/0x600
> down_write+0x74/0x110
> cifsFileInfo_put+0x88f/0x16a0
> cifs_setattr+0x992/0x1680
> notify_change+0x61a/0xa80
> utimes_common+0x3d4/0x870
> do_utimes+0x1c1/0x220
> SyS_utimensat+0x84/0x1a0
> entry_SYSCALL_64_fastpath+0x1f/0xbe
>
>  other info that might help us debug this:
>
>   Possible unsafe locking scenario:
>
> CPU0CPU1
> 
>lock(>s_type->i_mutex_key#11);
> lock(>lock_sem);
> lock(>s_type->i_mutex_key#11);
>lock(>lock_sem);
>
>   *** DEADLOCK ***
>
>  2 locks held by touch/487:
>   #0:  (sb_writers#10){.+.+.+}, at: mnt_want_write+0x41/0xb0
>   #1:  (>s_type->i_mutex_key#11){+.+.+.}, at: utimes_common+0x3ad/0x870
>
>  stack backtrace:
>  CPU: 0 PID: 487 Comm: touch Not tainted 4.12.0-rc7+ #9
>  Call Trace:
>   dump_stack+0xdb/0x185
>   print_circular_bug+0x45b/0x790
>   __lock_acquire+0x1f74/0x38f0
>   lock_acquire+0x1cc/0x600
>   down_write+0x74/0x110
>   cifsFileInfo_put+0x88f/0x16a0
>   cifs_setattr+0x992/0x1680
>   notify_change+0x61a/0xa80
>   utimes_common+0x3d4/0x870
>   do_utimes+0x1c1/0x220
>   SyS_utimensat+0x84/0x1a0
>   entry_SYSCALL_64_fastpath+0x1f/0xbe
>
> Fixes: 19dfc1f5f2ef03a52 ("cifs: fix the race in cifs_writev()")
> Signed-off-by: Rabin Vincent 
> ---
>  fs/cifs/file.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index fcef706..d16fa55 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -2810,12 +2810,12 @@ cifs_writev(struct kiocb *iocb, struct iov_iter *from)
> struct TCP_Server_Info *server = 
> tlink_tcon(cfile->tlink)->ses->server;
> ssize_t rc;
>
> +   inode_lock(inode);
> /*
>  * We need to hold the sem to be sure nobody modifies lock list
>  * with a brlock that prevents writing.
>  */
> down_read(>lock_sem);
> -   inode_lock(inode);
>
> rc = generic_write_checks(iocb, from);
> if (rc <= 0)
> @@ -2828,11 +2828,11 @@ cifs_writev(struct kiocb *iocb, struct iov_iter *from)
> else
> rc = -EACCES;
>  out:
> +   up_read(>lock_sem);
> inode_unlock(inode);
>
> if (rc > 0)
> rc = generic_write_sync(iocb, rc);
> -   up_read(>lock_sem);
> return rc;
>  }
>
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Acked-by: Pavel Shilovsky 

--
Best regards,
Pavel Shilovsky

RE: [PATCH] CIFS: fix circular locking dependency

2017-07-05 Thread Pavel Shilovskiy 


2017-06-29 7:01 GMT-07:00 Rabin Vincent :
> From: Rabin Vincent 
>
> When a CIFS filesystem is mounted with the forcemand option and the
> following command is run on it, lockdep warns about a circular locking
> dependency between CifsInodeInfo::lock_sem and the inode lock.
>
>  while echo foo > hello; do :; done & while touch -c hello; do :; done
>
> cifs_writev() takes the locks in the wrong order, but note that we can't
> only flip the order around because it releases the inode lock before the
> call to generic_write_sync() while it holds the lock_sem across that
> call.
>
> But, AFAICS, there is no need to hold the CifsInodeInfo::lock_sem across
> the generic_write_sync() call either, so we can release both the locks
> before generic_write_sync(), and change the order.
>
>  ==
>  WARNING: possible circular locking dependency detected
>  4.12.0-rc7+ #9 Not tainted
>  --
>  touch/487 is trying to acquire lock:
>   (>lock_sem){..}, at: cifsFileInfo_put+0x88f/0x16a0
>
>  but task is already holding lock:
>   (>s_type->i_mutex_key#11){+.+.+.}, at: utimes_common+0x3ad/0x870
>
>  which lock already depends on the new lock.
>
>  the existing dependency chain (in reverse order) is:
>
>  -> #1 (>s_type->i_mutex_key#11){+.+.+.}:
> __lock_acquire+0x1f74/0x38f0
> lock_acquire+0x1cc/0x600
> down_write+0x74/0x110
> cifs_strict_writev+0x3cb/0x8c0
> __vfs_write+0x4c1/0x930
> vfs_write+0x14c/0x2d0
> SyS_write+0xf7/0x240
> entry_SYSCALL_64_fastpath+0x1f/0xbe
>
>  -> #0 (>lock_sem){..}:
> check_prevs_add+0xfa0/0x1d10
> __lock_acquire+0x1f74/0x38f0
> lock_acquire+0x1cc/0x600
> down_write+0x74/0x110
> cifsFileInfo_put+0x88f/0x16a0
> cifs_setattr+0x992/0x1680
> notify_change+0x61a/0xa80
> utimes_common+0x3d4/0x870
> do_utimes+0x1c1/0x220
> SyS_utimensat+0x84/0x1a0
> entry_SYSCALL_64_fastpath+0x1f/0xbe
>
>  other info that might help us debug this:
>
>   Possible unsafe locking scenario:
>
> CPU0CPU1
> 
>lock(>s_type->i_mutex_key#11);
> lock(>lock_sem);
> lock(>s_type->i_mutex_key#11);
>lock(>lock_sem);
>
>   *** DEADLOCK ***
>
>  2 locks held by touch/487:
>   #0:  (sb_writers#10){.+.+.+}, at: mnt_want_write+0x41/0xb0
>   #1:  (>s_type->i_mutex_key#11){+.+.+.}, at: utimes_common+0x3ad/0x870
>
>  stack backtrace:
>  CPU: 0 PID: 487 Comm: touch Not tainted 4.12.0-rc7+ #9
>  Call Trace:
>   dump_stack+0xdb/0x185
>   print_circular_bug+0x45b/0x790
>   __lock_acquire+0x1f74/0x38f0
>   lock_acquire+0x1cc/0x600
>   down_write+0x74/0x110
>   cifsFileInfo_put+0x88f/0x16a0
>   cifs_setattr+0x992/0x1680
>   notify_change+0x61a/0xa80
>   utimes_common+0x3d4/0x870
>   do_utimes+0x1c1/0x220
>   SyS_utimensat+0x84/0x1a0
>   entry_SYSCALL_64_fastpath+0x1f/0xbe
>
> Fixes: 19dfc1f5f2ef03a52 ("cifs: fix the race in cifs_writev()")
> Signed-off-by: Rabin Vincent 
> ---
>  fs/cifs/file.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index fcef706..d16fa55 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -2810,12 +2810,12 @@ cifs_writev(struct kiocb *iocb, struct iov_iter *from)
> struct TCP_Server_Info *server = 
> tlink_tcon(cfile->tlink)->ses->server;
> ssize_t rc;
>
> +   inode_lock(inode);
> /*
>  * We need to hold the sem to be sure nobody modifies lock list
>  * with a brlock that prevents writing.
>  */
> down_read(>lock_sem);
> -   inode_lock(inode);
>
> rc = generic_write_checks(iocb, from);
> if (rc <= 0)
> @@ -2828,11 +2828,11 @@ cifs_writev(struct kiocb *iocb, struct iov_iter *from)
> else
> rc = -EACCES;
>  out:
> +   up_read(>lock_sem);
> inode_unlock(inode);
>
> if (rc > 0)
> rc = generic_write_sync(iocb, rc);
> -   up_read(>lock_sem);
> return rc;
>  }
>
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Acked-by: Pavel Shilovsky 

--
Best regards,
Pavel Shilovsky

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

2017-06-12 Thread Pavel Shilovskiy 
2017-06-11 0:12 GMT-07:00 Christophe JAILLET :
> '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 
> ---
>  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 c58691834eb2..cdcb3d95add8 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -1809,7 +1809,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;
> }
>
> @@ -1817,6 +1818,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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Thanks for catching this.

Reviewed-by: Pavel Shilovsky 

--
Best regards,
Pavel Shilovsky

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

2017-06-12 Thread Pavel Shilovskiy 
2017-06-11 0:12 GMT-07:00 Christophe JAILLET :
> '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 
> ---
>  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 c58691834eb2..cdcb3d95add8 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -1809,7 +1809,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;
> }
>
> @@ -1817,6 +1818,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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Thanks for catching this.

Reviewed-by: Pavel Shilovsky 

--
Best regards,
Pavel Shilovsky

RE: [PATCH 4.4 06/28] cifs: Do not send echoes before Negotiate is complete

2017-05-25 Thread Pavel Shilovskiy 
On Tu, May 9, 2017 6:01 PM, Pavel Shilovskiy wrote:
> On Tue, May 9, 2017 5:13 AM, Ben Hutchings wrote:
> > > commit 4fcd1813e640 ("Fix reconnect to not defer smb3 session 
> > > reconnect long after socket reconnect") added support for Negotiate 
> > > requests to be initiated by echo calls.
> > > 
> > > To avoid delays in calling echo after a reconnect, I added the patch 
> > > introduced by the commit b8c600120fc8 ("Call echo service 
> > > immediately after socket reconnect").
> > 
> > The second commit hasn't actually been applied to any stable branches (so 
> > this one didn't need to be).  Should it be?
> 
> Yes, 2nd commit hasn't been applied to stable branches, but the proposed fix 
> does the right thing anyway since it
> doesn't allow to call echo on connections before negotiate phase. The commit 
> b8c600120fc8 ("Call echo service 
> immediately after socket reconnect") just allows to trigger the wrong 
> behavior easily. So, I think it is better to have the 
> fix in stable branches.

The commit b8c600120fc8 ("Call echo service immediately after socket 
reconnect") does the right thing and fixes the problem with persistent handles 
reconnect, so, I suggest to take it to stable as well. I've just sent the 
backported version of the patch to stable@.

Best regards,
Pavel Shilovsky

RE: [PATCH 4.4 06/28] cifs: Do not send echoes before Negotiate is complete

2017-05-25 Thread Pavel Shilovskiy 
On Tu, May 9, 2017 6:01 PM, Pavel Shilovskiy wrote:
> On Tue, May 9, 2017 5:13 AM, Ben Hutchings wrote:
> > > commit 4fcd1813e640 ("Fix reconnect to not defer smb3 session 
> > > reconnect long after socket reconnect") added support for Negotiate 
> > > requests to be initiated by echo calls.
> > > 
> > > To avoid delays in calling echo after a reconnect, I added the patch 
> > > introduced by the commit b8c600120fc8 ("Call echo service 
> > > immediately after socket reconnect").
> > 
> > The second commit hasn't actually been applied to any stable branches (so 
> > this one didn't need to be).  Should it be?
> 
> Yes, 2nd commit hasn't been applied to stable branches, but the proposed fix 
> does the right thing anyway since it
> doesn't allow to call echo on connections before negotiate phase. The commit 
> b8c600120fc8 ("Call echo service 
> immediately after socket reconnect") just allows to trigger the wrong 
> behavior easily. So, I think it is better to have the 
> fix in stable branches.

The commit b8c600120fc8 ("Call echo service immediately after socket 
reconnect") does the right thing and fixes the problem with persistent handles 
reconnect, so, I suggest to take it to stable as well. I've just sent the 
backported version of the patch to stable@.

Best regards,
Pavel Shilovsky

RE: [PATCH 4.4 06/28] cifs: Do not send echoes before Negotiate is complete

2017-05-09 Thread Pavel Shilovskiy 
On Tue, May 9, 2017 5:13 AM, Ben Hutchings wrote:
> > commit 4fcd1813e640 ("Fix reconnect to not defer smb3 session 
> > reconnect long after socket reconnect") added support for Negotiate 
> > requests to be initiated by echo calls.
> > 
> > To avoid delays in calling echo after a reconnect, I added the patch 
> > introduced by the commit b8c600120fc8 ("Call echo service immediately 
> > after socket reconnect").
> 
> The second commit hasn't actually been applied to any stable branches (so 
> this one didn't need to be).  Should it be?

Yes, 2nd commit hasn't been applied to stable branches, but the proposed fix 
does the right thing anyway since it doesn't allow to call echo on connections 
before negotiate phase. The commit b8c600120fc8 ("Call echo service immediately 
after socket reconnect") just allows to trigger the wrong behavior easily. So, 
I think it is better to have the fix in stable branches.

Best regards,
Pavel Shilovsky

RE: [PATCH 4.4 06/28] cifs: Do not send echoes before Negotiate is complete

2017-05-09 Thread Pavel Shilovskiy 
On Tue, May 9, 2017 5:13 AM, Ben Hutchings wrote:
> > commit 4fcd1813e640 ("Fix reconnect to not defer smb3 session 
> > reconnect long after socket reconnect") added support for Negotiate 
> > requests to be initiated by echo calls.
> > 
> > To avoid delays in calling echo after a reconnect, I added the patch 
> > introduced by the commit b8c600120fc8 ("Call echo service immediately 
> > after socket reconnect").
> 
> The second commit hasn't actually been applied to any stable branches (so 
> this one didn't need to be).  Should it be?

Yes, 2nd commit hasn't been applied to stable branches, but the proposed fix 
does the right thing anyway since it doesn't allow to call echo on connections 
before negotiate phase. The commit b8c600120fc8 ("Call echo service immediately 
after socket reconnect") just allows to trigger the wrong behavior easily. So, 
I think it is better to have the fix in stable branches.

Best regards,
Pavel Shilovsky