diff mbox series

cifs: fix a credits leak for compund commands

Message ID 20181010030705.9681-2-lsahlber@redhat.com
State New
Headers show
Series cifs: fix a credits leak for compund commands | expand

Commit Message

Ronnie Sahlberg Oct. 10, 2018, 3:07 a.m. UTC
When processing the mids for compounds we would only add credits based on
the last successful mid in the compound which would leak credits and
eventually triggering a re-connect.

Fix this by splitting the mid processing part into two loops instead of one
where the first loop just waits for all mids and then counts how many
credits we were granted for the whole compound.

Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/transport.c | 56 ++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 36 insertions(+), 20 deletions(-)

Comments

Steve French Oct. 10, 2018, 3:16 a.m. UTC | #1
merged into cifs-2.6.git for-next and rebased as requested
On Tue, Oct 9, 2018 at 10:07 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
>
> When processing the mids for compounds we would only add credits based on
> the last successful mid in the compound which would leak credits and
> eventually triggering a re-connect.
>
> Fix this by splitting the mid processing part into two loops instead of one
> where the first loop just waits for all mids and then counts how many
> credits we were granted for the whole compound.
>
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> ---
>  fs/cifs/transport.c | 56 ++++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 36 insertions(+), 20 deletions(-)
>
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index b48f43963da6..cb34afaffe02 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -786,7 +786,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
>         int i, j, rc = 0;
>         int timeout, optype;
>         struct mid_q_entry *midQ[MAX_COMPOUND];
> -       unsigned int credits = 1;
> +       unsigned int credits = 0;
>         char *buf;
>
>         timeout = flags & CIFS_TIMEOUT_MASK;
> @@ -851,17 +851,20 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
>
>         mutex_unlock(&ses->server->srv_mutex);
>
> -       for (i = 0; i < num_rqst; i++) {
> -               if (rc < 0)
> -                       goto out;
> +       if (rc < 0)
> +               goto out;
>
> -               if ((ses->status == CifsNew) || (optype & CIFS_NEG_OP))
> -                       smb311_update_preauth_hash(ses, rqst[i].rq_iov,
> -                                                  rqst[i].rq_nvec);
> +       /*
> +        * Compounding is never used during session establish.
> +        */
> +       if ((ses->status == CifsNew) || (optype & CIFS_NEG_OP))
> +               smb311_update_preauth_hash(ses, rqst[0].rq_iov,
> +                                          rqst[0].rq_nvec);
>
> -               if (timeout == CIFS_ASYNC_OP)
> -                       goto out;
> +       if (timeout == CIFS_ASYNC_OP)
> +               goto out;
>
> +       for (i = 0; i < num_rqst; i++) {
>                 rc = wait_for_response(ses->server, midQ[i]);
>                 if (rc != 0) {
>                         cifs_dbg(FYI, "Cancelling wait for mid %llu\n",
> @@ -877,10 +880,20 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
>                         }
>                         spin_unlock(&GlobalMid_Lock);
>                 }
> +       }
> +
> +       for (i = 0; i < num_rqst; i++)
> +               credits += ses->server->ops->get_credits(midQ[i]);
> +       if (!credits)
> +               credits = 1;
> +
> +       for (i = 0; i < num_rqst; i++) {
> +               if (rc < 0)
> +                       goto out;
>
>                 rc = cifs_sync_mid_result(midQ[i], ses->server);
>                 if (rc != 0) {
> -                       add_credits(ses->server, 1, optype);
> +                       add_credits(ses->server, credits, optype);
>                         return rc;
>                 }
>
> @@ -901,23 +914,26 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
>                 else
>                         resp_buf_type[i] = CIFS_SMALL_BUFFER;
>
> -               if ((ses->status == CifsNew) || (optype & CIFS_NEG_OP)) {
> -                       struct kvec iov = {
> -                               .iov_base = resp_iov[i].iov_base,
> -                               .iov_len = resp_iov[i].iov_len
> -                       };
> -                       smb311_update_preauth_hash(ses, &iov, 1);
> -               }
> -
> -               credits = ses->server->ops->get_credits(midQ[i]);
> -
>                 rc = ses->server->ops->check_receive(midQ[i], ses->server,
>                                                      flags & CIFS_LOG_ERROR);
>
>                 /* mark it so buf will not be freed by cifs_delete_mid */
>                 if ((flags & CIFS_NO_RESP) == 0)
>                         midQ[i]->resp_buf = NULL;
> +
>         }
> +
> +       /*
> +        * Compounding is never used during session establish.
> +        */
> +       if ((ses->status == CifsNew) || (optype & CIFS_NEG_OP)) {
> +               struct kvec iov = {
> +                       .iov_base = resp_iov[0].iov_base,
> +                       .iov_len = resp_iov[0].iov_len
> +               };
> +               smb311_update_preauth_hash(ses, &iov, 1);
> +       }
> +
>  out:
>         /*
>          * This will dequeue all mids. After this it is important that the
> --
> 2.13.6
>
diff mbox series

Patch

diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index b48f43963da6..cb34afaffe02 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -786,7 +786,7 @@  compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
 	int i, j, rc = 0;
 	int timeout, optype;
 	struct mid_q_entry *midQ[MAX_COMPOUND];
-	unsigned int credits = 1;
+	unsigned int credits = 0;
 	char *buf;
 
 	timeout = flags & CIFS_TIMEOUT_MASK;
@@ -851,17 +851,20 @@  compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
 
 	mutex_unlock(&ses->server->srv_mutex);
 
-	for (i = 0; i < num_rqst; i++) {
-		if (rc < 0)
-			goto out;
+	if (rc < 0)
+		goto out;
 
-		if ((ses->status == CifsNew) || (optype & CIFS_NEG_OP))
-			smb311_update_preauth_hash(ses, rqst[i].rq_iov,
-						   rqst[i].rq_nvec);
+	/*
+	 * Compounding is never used during session establish.
+	 */
+	if ((ses->status == CifsNew) || (optype & CIFS_NEG_OP))
+		smb311_update_preauth_hash(ses, rqst[0].rq_iov,
+					   rqst[0].rq_nvec);
 
-		if (timeout == CIFS_ASYNC_OP)
-			goto out;
+	if (timeout == CIFS_ASYNC_OP)
+		goto out;
 
+	for (i = 0; i < num_rqst; i++) {
 		rc = wait_for_response(ses->server, midQ[i]);
 		if (rc != 0) {
 			cifs_dbg(FYI, "Cancelling wait for mid %llu\n",
@@ -877,10 +880,20 @@  compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
 			}
 			spin_unlock(&GlobalMid_Lock);
 		}
+	}
+
+	for (i = 0; i < num_rqst; i++)
+		credits += ses->server->ops->get_credits(midQ[i]);
+	if (!credits)
+		credits = 1;
+
+	for (i = 0; i < num_rqst; i++) {
+		if (rc < 0)
+			goto out;
 
 		rc = cifs_sync_mid_result(midQ[i], ses->server);
 		if (rc != 0) {
-			add_credits(ses->server, 1, optype);
+			add_credits(ses->server, credits, optype);
 			return rc;
 		}
 
@@ -901,23 +914,26 @@  compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
 		else
 			resp_buf_type[i] = CIFS_SMALL_BUFFER;
 
-		if ((ses->status == CifsNew) || (optype & CIFS_NEG_OP)) {
-			struct kvec iov = {
-				.iov_base = resp_iov[i].iov_base,
-				.iov_len = resp_iov[i].iov_len
-			};
-			smb311_update_preauth_hash(ses, &iov, 1);
-		}
-
-		credits = ses->server->ops->get_credits(midQ[i]);
-
 		rc = ses->server->ops->check_receive(midQ[i], ses->server,
 						     flags & CIFS_LOG_ERROR);
 
 		/* mark it so buf will not be freed by cifs_delete_mid */
 		if ((flags & CIFS_NO_RESP) == 0)
 			midQ[i]->resp_buf = NULL;
+
 	}
+
+	/*
+	 * Compounding is never used during session establish.
+	 */
+	if ((ses->status == CifsNew) || (optype & CIFS_NEG_OP)) {
+		struct kvec iov = {
+			.iov_base = resp_iov[0].iov_base,
+			.iov_len = resp_iov[0].iov_len
+		};
+		smb311_update_preauth_hash(ses, &iov, 1);
+	}
+
 out:
 	/*
 	 * This will dequeue all mids. After this it is important that the