Commit ef68e831 authored by Pavel Shilovsky's avatar Pavel Shilovsky Committed by Steve French
Browse files

CIFS: Do not reconnect TCP session in add_credits() 



When executing add_credits() we currently call cifs_reconnect()
if the number of credits is zero and there are no requests in
flight. In this case we may call cifs_reconnect() recursively
twice and cause memory corruption given the following sequence
of functions:

mid1.callback() -> add_credits() -> cifs_reconnect() ->
-> mid2.callback() -> add_credits() -> cifs_reconnect().

Fix this by avoiding to call cifs_reconnect() in add_credits()
and checking for zero credits in the demultiplex thread.

Cc: <stable@vger.kernel.org>
Signed-off-by: default avatarPavel Shilovsky <pshilov@microsoft.com>
Reviewed-by: default avatarRonnie Sahlberg <lsahlber@redhat.com>
Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
parent b0b2cac7

fs/cifs/connect.c

+21 −0
Original line number Diff line number Diff line
@@ -720,6 +720,21 @@ server_unresponsive(struct TCP_Server_Info *server) 
	return false;

}



static inline bool

zero_credits(struct TCP_Server_Info *server)

{

	int val;



	spin_lock(&server->req_lock);

	val = server->credits + server->echo_credits + server->oplock_credits;

	if (server->in_flight == 0 && val == 0) {

		spin_unlock(&server->req_lock);

		return true;

	}

	spin_unlock(&server->req_lock);

	return false;

}



static int

cifs_readv_from_socket(struct TCP_Server_Info *server, struct msghdr *smb_msg)

{
@@ -732,6 +747,12 @@ cifs_readv_from_socket(struct TCP_Server_Info *server, struct msghdr *smb_msg) 
	for (total_read = 0; msg_data_left(smb_msg); total_read += length) {

		try_to_freeze();



		/* reconnect if no credits and no requests in flight */

		if (zero_credits(server)) {

			cifs_reconnect(server);

			return -ECONNABORTED;

		}



		if (server_unresponsive(server))

			return -ECONNABORTED;

		if (cifs_rdma_enabled(server) && server->smbd_conn)

fs/cifs/smb2ops.c

+25 −7
Original line number Diff line number Diff line
@@ -34,6 +34,7 @@ 
#include "cifs_ioctl.h"

#include "smbdirect.h"



/* Change credits for different ops and return the total number of credits */

static int

change_conf(struct TCP_Server_Info *server)

{
@@ -41,17 +42,15 @@ change_conf(struct TCP_Server_Info *server) 
	server->oplock_credits = server->echo_credits = 0;

	switch (server->credits) {

	case 0:

		return -1;

		return 0;

	case 1:

		server->echoes = false;

		server->oplocks = false;

		cifs_dbg(VFS, "disabling echoes and oplocks\n");

		break;

	case 2:

		server->echoes = true;

		server->oplocks = false;

		server->echo_credits = 1;

		cifs_dbg(FYI, "disabling oplocks\n");

		break;

	default:

		server->echoes = true;
@@ -64,14 +63,15 @@ change_conf(struct TCP_Server_Info *server) 
		server->echo_credits = 1;

	}

	server->credits -= server->echo_credits + server->oplock_credits;

	return 0;

	return server->credits + server->echo_credits + server->oplock_credits;

}



static void

smb2_add_credits(struct TCP_Server_Info *server, const unsigned int add,

		 const int optype)

{

	int *val, rc = 0;

	int *val, rc = -1;



	spin_lock(&server->req_lock);

	val = server->ops->get_credits_field(server, optype);
@@ -101,8 +101,26 @@ smb2_add_credits(struct TCP_Server_Info *server, const unsigned int add, 
	}

	spin_unlock(&server->req_lock);

	wake_up(&server->request_q);

	if (rc)

		cifs_reconnect(server);



	if (server->tcpStatus == CifsNeedReconnect)

		return;



	switch (rc) {

	case -1:

		/* change_conf hasn't been executed */

		break;

	case 0:

		cifs_dbg(VFS, "Possible client or server bug - zero credits\n");

		break;

	case 1:

		cifs_dbg(VFS, "disabling echoes and oplocks\n");

		break;

	case 2:

		cifs_dbg(FYI, "disabling oplocks\n");

		break;

	default:

		cifs_dbg(FYI, "add %u credits total=%d\n", add, rc);

	}

}



static void

CRA Git | Maintained and supported by SUSTech CRA and CCSE