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

Commit 2ff017f5 authored by Nicholas Bellinger's avatar Nicholas Bellinger
Browse files

iscsi-target: Disable markers + remove dangerous local scope array usage 

This patch makes iscsi-target explictly disable OFMarker=Yes and IFMarker=yes
parameter key usage during iscsi login by setting IFMarkInt_Reject and
OFMarkInt_Reject values in iscsi_enforce_integrity_rules() to effectively
disable iscsi marker usage.  With this patch, an initiator proposer asking
to enable either marker parameter keys will be issued a 'No' response, and
the target sets OFMarkInt + IFMarkInt parameter key response to 'Irrelevant'.

With markers disabled during iscsi login, this patch removes the problematic
on-stack local-scope array for marker intervals in iscsit_do_rx_data() +
iscsit_do_tx_data(), and other related marker code in iscsi_target_util.c.
This fixes a potentional stack smashing scenario with small range markers
enabled and a large MRDSL as reported by DanC here:

[bug report] target: stack can be smashed
http://www.spinics.net/lists/target-devel/msg00453.html



Reported-by: default avatarDan Carpenter <error27@gmail.com>
Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
parent 784eb99e

drivers/target/iscsi/iscsi_target_parameters.c

+1 −1
Original line number Diff line number Diff line
@@ -1430,7 +1430,7 @@ static int iscsi_enforce_integrity_rules( 
	u8 DataSequenceInOrder = 0;

	u8 ErrorRecoveryLevel = 0, SessionType = 0;

	u8 IFMarker = 0, OFMarker = 0;

	u8 IFMarkInt_Reject = 0, OFMarkInt_Reject = 0;

	u8 IFMarkInt_Reject = 1, OFMarkInt_Reject = 1;

	u32 FirstBurstLength = 0, MaxBurstLength = 0;

	struct iscsi_param *param = NULL;

drivers/target/iscsi/iscsi_target_util.c

+6 −242
Original line number Diff line number Diff line
@@ -874,40 +874,6 @@ void iscsit_inc_session_usage_count(struct iscsi_session *sess) 
	spin_unlock_bh(&sess->session_usage_lock);

}



/*

 *	Used before iscsi_do[rx,tx]_data() to determine iov and [rx,tx]_marker

 *	array counts needed for sync and steering.

 */

static int iscsit_determine_sync_and_steering_counts(

	struct iscsi_conn *conn,

	struct iscsi_data_count *count)

{

	u32 length = count->data_length;

	u32 marker, markint;



	count->sync_and_steering = 1;



	marker = (count->type == ISCSI_RX_DATA) ?

			conn->of_marker : conn->if_marker;

	markint = (count->type == ISCSI_RX_DATA) ?

			(conn->conn_ops->OFMarkInt * 4) :

			(conn->conn_ops->IFMarkInt * 4);

	count->ss_iov_count = count->iov_count;



	while (length > 0) {

		if (length >= marker) {

			count->ss_iov_count += 3;

			count->ss_marker_count += 2;



			length -= marker;

			marker = markint;

		} else

			length = 0;

	}



	return 0;

}



/*

 *	Setup conn->if_marker and conn->of_marker values based upon

 *	the initial marker-less interval. (see iSCSI v19 A.2)
@@ -1431,8 +1397,7 @@ static int iscsit_do_rx_data( 
	struct iscsi_data_count *count)

{

	int data = count->data_length, rx_loop = 0, total_rx = 0, iov_len;

	u32 rx_marker_val[count->ss_marker_count], rx_marker_iov = 0;

	struct kvec iov[count->ss_iov_count], *iov_p;

	struct kvec *iov_p;

	struct msghdr msg;



	if (!conn || !conn->sock || !conn->conn_ops)
@@ -1440,93 +1405,8 @@ static int iscsit_do_rx_data( 


	memset(&msg, 0, sizeof(struct msghdr));



	if (count->sync_and_steering) {

		int size = 0;

		u32 i, orig_iov_count = 0;

		u32 orig_iov_len = 0, orig_iov_loc = 0;

		u32 iov_count = 0, per_iov_bytes = 0;

		u32 *rx_marker, old_rx_marker = 0;

		struct kvec *iov_record;



		memset(&rx_marker_val, 0,

				count->ss_marker_count * sizeof(u32));

		memset(&iov, 0, count->ss_iov_count * sizeof(struct kvec));



		iov_record = count->iov;

		orig_iov_count = count->iov_count;

		rx_marker = &conn->of_marker;



		i = 0;

		size = data;

		orig_iov_len = iov_record[orig_iov_loc].iov_len;

		while (size > 0) {

			pr_debug("rx_data: #1 orig_iov_len %u,"

			" orig_iov_loc %u\n", orig_iov_len, orig_iov_loc);

			pr_debug("rx_data: #2 rx_marker %u, size"

				" %u\n", *rx_marker, size);



			if (orig_iov_len >= *rx_marker) {

				iov[iov_count].iov_len = *rx_marker;

				iov[iov_count++].iov_base =

					(iov_record[orig_iov_loc].iov_base +

						per_iov_bytes);



				iov[iov_count].iov_len = (MARKER_SIZE / 2);

				iov[iov_count++].iov_base =

					&rx_marker_val[rx_marker_iov++];

				iov[iov_count].iov_len = (MARKER_SIZE / 2);

				iov[iov_count++].iov_base =

					&rx_marker_val[rx_marker_iov++];

				old_rx_marker = *rx_marker;



				/*

				 * OFMarkInt is in 32-bit words.

				 */

				*rx_marker = (conn->conn_ops->OFMarkInt * 4);

				size -= old_rx_marker;

				orig_iov_len -= old_rx_marker;

				per_iov_bytes += old_rx_marker;



				pr_debug("rx_data: #3 new_rx_marker"

					" %u, size %u\n", *rx_marker, size);

			} else {

				iov[iov_count].iov_len = orig_iov_len;

				iov[iov_count++].iov_base =

					(iov_record[orig_iov_loc].iov_base +

						per_iov_bytes);



				per_iov_bytes = 0;

				*rx_marker -= orig_iov_len;

				size -= orig_iov_len;



				if (size)

					orig_iov_len =

					iov_record[++orig_iov_loc].iov_len;



				pr_debug("rx_data: #4 new_rx_marker"

					" %u, size %u\n", *rx_marker, size);

			}

		}

		data += (rx_marker_iov * (MARKER_SIZE / 2));



		iov_p	= &iov[0];

		iov_len	= iov_count;



		if (iov_count > count->ss_iov_count) {

			pr_err("iov_count: %d, count->ss_iov_count:"

				" %d\n", iov_count, count->ss_iov_count);

			return -1;

		}

		if (rx_marker_iov > count->ss_marker_count) {

			pr_err("rx_marker_iov: %d, count->ss_marker"

				"_count: %d\n", rx_marker_iov,

				count->ss_marker_count);

			return -1;

		}

	} else {

	iov_p = count->iov;

	iov_len	= count->iov_count;

	}



	while (total_rx < data) {

		rx_loop = kernel_recvmsg(conn->sock, &msg, iov_p, iov_len,
@@ -1541,16 +1421,6 @@ static int iscsit_do_rx_data( 
				rx_loop, total_rx, data);

	}



	if (count->sync_and_steering) {

		int j;

		for (j = 0; j < rx_marker_iov; j++) {

			pr_debug("rx_data: #5 j: %d, offset: %d\n",

				j, rx_marker_val[j]);

			conn->of_marker_offset = rx_marker_val[j];

		}

		total_rx -= (rx_marker_iov * (MARKER_SIZE / 2));

	}



	return total_rx;

}
@@ -1559,8 +1429,7 @@ static int iscsit_do_tx_data( 
	struct iscsi_data_count *count)

{

	int data = count->data_length, total_tx = 0, tx_loop = 0, iov_len;

	u32 tx_marker_val[count->ss_marker_count], tx_marker_iov = 0;

	struct kvec iov[count->ss_iov_count], *iov_p;

	struct kvec *iov_p;

	struct msghdr msg;



	if (!conn || !conn->sock || !conn->conn_ops)
@@ -1573,98 +1442,8 @@ static int iscsit_do_tx_data( 


	memset(&msg, 0, sizeof(struct msghdr));



	if (count->sync_and_steering) {

		int size = 0;

		u32 i, orig_iov_count = 0;

		u32 orig_iov_len = 0, orig_iov_loc = 0;

		u32 iov_count = 0, per_iov_bytes = 0;

		u32 *tx_marker, old_tx_marker = 0;

		struct kvec *iov_record;



		memset(&tx_marker_val, 0,

			count->ss_marker_count * sizeof(u32));

		memset(&iov, 0, count->ss_iov_count * sizeof(struct kvec));



		iov_record = count->iov;

		orig_iov_count = count->iov_count;

		tx_marker = &conn->if_marker;



		i = 0;

		size = data;

		orig_iov_len = iov_record[orig_iov_loc].iov_len;

		while (size > 0) {

			pr_debug("tx_data: #1 orig_iov_len %u,"

			" orig_iov_loc %u\n", orig_iov_len, orig_iov_loc);

			pr_debug("tx_data: #2 tx_marker %u, size"

				" %u\n", *tx_marker, size);



			if (orig_iov_len >= *tx_marker) {

				iov[iov_count].iov_len = *tx_marker;

				iov[iov_count++].iov_base =

					(iov_record[orig_iov_loc].iov_base +

						per_iov_bytes);



				tx_marker_val[tx_marker_iov] =

						(size - *tx_marker);

				iov[iov_count].iov_len = (MARKER_SIZE / 2);

				iov[iov_count++].iov_base =

					&tx_marker_val[tx_marker_iov++];

				iov[iov_count].iov_len = (MARKER_SIZE / 2);

				iov[iov_count++].iov_base =

					&tx_marker_val[tx_marker_iov++];

				old_tx_marker = *tx_marker;



				/*

				 * IFMarkInt is in 32-bit words.

				 */

				*tx_marker = (conn->conn_ops->IFMarkInt * 4);

				size -= old_tx_marker;

				orig_iov_len -= old_tx_marker;

				per_iov_bytes += old_tx_marker;



				pr_debug("tx_data: #3 new_tx_marker"

					" %u, size %u\n", *tx_marker, size);

				pr_debug("tx_data: #4 offset %u\n",

					tx_marker_val[tx_marker_iov-1]);

			} else {

				iov[iov_count].iov_len = orig_iov_len;

				iov[iov_count++].iov_base

					= (iov_record[orig_iov_loc].iov_base +

						per_iov_bytes);



				per_iov_bytes = 0;

				*tx_marker -= orig_iov_len;

				size -= orig_iov_len;



				if (size)

					orig_iov_len =

					iov_record[++orig_iov_loc].iov_len;



				pr_debug("tx_data: #5 new_tx_marker"

					" %u, size %u\n", *tx_marker, size);

			}

		}



		data += (tx_marker_iov * (MARKER_SIZE / 2));



		iov_p = &iov[0];

		iov_len = iov_count;



		if (iov_count > count->ss_iov_count) {

			pr_err("iov_count: %d, count->ss_iov_count:"

				" %d\n", iov_count, count->ss_iov_count);

			return -1;

		}

		if (tx_marker_iov > count->ss_marker_count) {

			pr_err("tx_marker_iov: %d, count->ss_marker"

				"_count: %d\n", tx_marker_iov,

				count->ss_marker_count);

			return -1;

		}

	} else {

	iov_p = count->iov;

	iov_len = count->iov_count;

	}



	while (total_tx < data) {

		tx_loop = kernel_sendmsg(conn->sock, &msg, iov_p, iov_len,
@@ -1679,9 +1458,6 @@ static int iscsit_do_tx_data( 
					tx_loop, total_tx, data);

	}



	if (count->sync_and_steering)

		total_tx -= (tx_marker_iov * (MARKER_SIZE / 2));



	return total_tx;

}
@@ -1702,12 +1478,6 @@ int rx_data( 
	c.data_length = data;

	c.type = ISCSI_RX_DATA;



	if (conn->conn_ops->OFMarker &&

	   (conn->conn_state >= TARG_CONN_STATE_LOGGED_IN)) {

		if (iscsit_determine_sync_and_steering_counts(conn, &c) < 0)

			return -1;

	}



	return iscsit_do_rx_data(conn, &c);

}
@@ -1728,12 +1498,6 @@ int tx_data( 
	c.data_length = data;

	c.type = ISCSI_TX_DATA;



	if (conn->conn_ops->IFMarker &&

	   (conn->conn_state >= TARG_CONN_STATE_LOGGED_IN)) {

		if (iscsit_determine_sync_and_steering_counts(conn, &c) < 0)

			return -1;

	}



	return iscsit_do_tx_data(conn, &c);

}