Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 79d59d08 authored by Roland Dreier's avatar Roland Dreier Committed by Nicholas Bellinger
Browse files

iscsi-target: Fix wrong buffer / buffer overrun in iscsi_change_param_value() 



In non-leading connection login, iscsi_login_non_zero_tsih_s1() calls
iscsi_change_param_value() with the buffer it uses to hold the login
PDU, not a temporary buffer.  This leads to the login header getting
corrupted and login failing for non-leading connections in MC/S.

Fix this by adding a wrapper iscsi_change_param_sprintf() that handles
the temporary buffer itself to avoid confusion.  Also handle sending a
reject in case of failure in the wrapper, which lets the calling code
get quite a bit smaller and easier to read.

Finally, bump the size of the temporary buffer from 32 to 64 bytes to be
safe, since "MaxRecvDataSegmentLength=" by itself is 25 bytes; with a
trailing NUL, a value >= 1M will lead to a buffer overrun.  (This isn't
the default but we don't need to run right at the ragged edge here)

Reported-by: default avatarSantosh Kulkarni <santosh.kulkarni@calsoftinc.com>
Signed-off-by: default avatarRoland Dreier <roland@purestorage.com>
Cc: stable@vger.kernel.org # 3.10+
Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
parent 6cc44a6f

drivers/target/iscsi/iscsi_target_login.c

+31 −39
Original line number Diff line number Diff line
@@ -249,6 +249,28 @@ static void iscsi_login_set_conn_values( 
	mutex_unlock(&auth_id_lock);

}



static __printf(2, 3) int iscsi_change_param_sprintf(

	struct iscsi_conn *conn,

	const char *fmt, ...)

{

	va_list args;

	unsigned char buf[64];



	memset(buf, 0, sizeof buf);



	va_start(args, fmt);

	vsnprintf(buf, sizeof buf, fmt, args);

	va_end(args);



	if (iscsi_change_param_value(buf, conn->param_list, 0) < 0) {

		iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,

				ISCSI_LOGIN_STATUS_NO_RESOURCES);

		return -1;

	}



	return 0;

}



/*

 *	This is the leading connection of a new session,

 *	or session reinstatement.
@@ -339,7 +361,6 @@ static int iscsi_login_zero_tsih_s2( 
{

	struct iscsi_node_attrib *na;

	struct iscsi_session *sess = conn->sess;

	unsigned char buf[32];

	bool iser = false;



	sess->tpg = conn->tpg;
@@ -380,26 +401,16 @@ static int iscsi_login_zero_tsih_s2( 
	 *

	 * In our case, we have already located the struct iscsi_tiqn at this point.

	 */

	memset(buf, 0, 32);

	sprintf(buf, "TargetPortalGroupTag=%hu", sess->tpg->tpgt);

	if (iscsi_change_param_value(buf, conn->param_list, 0) < 0) {

		iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,

				ISCSI_LOGIN_STATUS_NO_RESOURCES);

	if (iscsi_change_param_sprintf(conn, "TargetPortalGroupTag=%hu", sess->tpg->tpgt))

		return -1;

	}



	/*

	 * Workaround for Initiators that have broken connection recovery logic.

	 *

	 * "We would really like to get rid of this." Linux-iSCSI.org team

	 */

	memset(buf, 0, 32);

	sprintf(buf, "ErrorRecoveryLevel=%d", na->default_erl);

	if (iscsi_change_param_value(buf, conn->param_list, 0) < 0) {

		iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,

				ISCSI_LOGIN_STATUS_NO_RESOURCES);

	if (iscsi_change_param_sprintf(conn, "ErrorRecoveryLevel=%d", na->default_erl))

		return -1;

	}



	if (iscsi_login_disable_FIM_keys(conn->param_list, conn) < 0)

		return -1;
@@ -411,12 +422,9 @@ static int iscsi_login_zero_tsih_s2( 
		unsigned long mrdsl, off;

		int rc;



		sprintf(buf, "RDMAExtensions=Yes");

		if (iscsi_change_param_value(buf, conn->param_list, 0) < 0) {

			iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,

				ISCSI_LOGIN_STATUS_NO_RESOURCES);

		if (iscsi_change_param_sprintf(conn, "RDMAExtensions=Yes"))

			return -1;

		}



		/*

		 * Make MaxRecvDataSegmentLength PAGE_SIZE aligned for

		 * Immediate Data + Unsolicitied Data-OUT if necessary..
@@ -446,12 +454,8 @@ static int iscsi_login_zero_tsih_s2( 
		pr_warn("Aligning ISER MaxRecvDataSegmentLength: %lu down"

			" to PAGE_SIZE\n", mrdsl);



		sprintf(buf, "MaxRecvDataSegmentLength=%lu\n", mrdsl);

		if (iscsi_change_param_value(buf, conn->param_list, 0) < 0) {

			iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,

				ISCSI_LOGIN_STATUS_NO_RESOURCES);

		if (iscsi_change_param_sprintf(conn, "MaxRecvDataSegmentLength=%lu\n", mrdsl))

			return -1;

		}

		/*

		 * ISER currently requires that ImmediateData + Unsolicited

		 * Data be disabled when protection / signature MRs are enabled.
@@ -461,19 +465,12 @@ check_prot: 
		   (TARGET_PROT_DOUT_STRIP | TARGET_PROT_DOUT_PASS |

		    TARGET_PROT_DOUT_INSERT)) {



			sprintf(buf, "ImmediateData=No");

			if (iscsi_change_param_value(buf, conn->param_list, 0) < 0) {

				iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,

						    ISCSI_LOGIN_STATUS_NO_RESOURCES);

			if (iscsi_change_param_sprintf(conn, "ImmediateData=No"))

				return -1;

			}



			sprintf(buf, "InitialR2T=Yes");

			if (iscsi_change_param_value(buf, conn->param_list, 0) < 0) {

				iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,

						    ISCSI_LOGIN_STATUS_NO_RESOURCES);

			if (iscsi_change_param_sprintf(conn, "InitialR2T=Yes"))

				return -1;

			}



			pr_debug("Forcing ImmediateData=No + InitialR2T=Yes for"

				 " T10-PI enabled ISER session\n");

		}
@@ -618,13 +615,8 @@ static int iscsi_login_non_zero_tsih_s2( 
	 *

	 * In our case, we have already located the struct iscsi_tiqn at this point.

	 */

	memset(buf, 0, 32);

	sprintf(buf, "TargetPortalGroupTag=%hu", sess->tpg->tpgt);

	if (iscsi_change_param_value(buf, conn->param_list, 0) < 0) {

		iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,

				ISCSI_LOGIN_STATUS_NO_RESOURCES);

	if (iscsi_change_param_sprintf(conn, "TargetPortalGroupTag=%hu", sess->tpg->tpgt))

		return -1;

	}



	return iscsi_login_disable_FIM_keys(conn->param_list, conn);

}