Commit 565a4462 authored by melon.masou's avatar melon.masou Committed by Ben Walker
Browse files

iscsi: fix segfault when r2t 



Fixes #2781

This patch fixes two issue causing segfault on r2t:
1. pdu buffer is allocated from immediate_data_pool, but data_buf_len is set as data_out_pool
2. task->desired_data_transfer_length is rewrite by iscsi_send_r2t, which causes a wrong calculated pdu->data_buf_len

Signed-off-by: default avatarmelon.masou <melon.masou@outlook.com>
Change-Id: I151859afff7104f29ad7f0ec57a8479d88b742bd
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/15542


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
parent 3d9b0628

lib/iscsi/iscsi.c

+16 −4
Original line number Diff line number Diff line
@@ -4156,6 +4156,16 @@ iscsi_pdu_hdr_op_snack(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) 
	return rc;

}



static inline uint32_t

iscsi_get_mobj_max_data_len(struct spdk_mobj *mobj)

{

	if (mobj->mp == g_iscsi.pdu_immediate_data_pool) {

		return iscsi_get_max_immediate_data_size();

	} else {

		return SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH;

	}

}



static int

iscsi_pdu_hdr_op_data(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu)

{
@@ -4169,6 +4179,7 @@ iscsi_pdu_hdr_op_data(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) 
	uint32_t DataSN;

	uint32_t buffer_offset;

	uint32_t len;

	uint32_t current_desired_data_transfer_length;

	int F_bit;

	int rc;
@@ -4195,6 +4206,7 @@ iscsi_pdu_hdr_op_data(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) 
	}



	lun_dev = spdk_scsi_dev_get_lun(conn->dev, task->lun_id);

	current_desired_data_transfer_length = task->desired_data_transfer_length;



	if (pdu->data_segment_len > task->desired_data_transfer_length) {

		SPDK_ERRLOG("the dataout pdu data length is larger than the value sent by R2T PDU\n");
@@ -4272,7 +4284,7 @@ iscsi_pdu_hdr_op_data(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) 
			 * SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH to merge them into a

			 * single subtask.

			 */

			pdu->data_buf_len = spdk_min(task->desired_data_transfer_length,

			pdu->data_buf_len = spdk_min(current_desired_data_transfer_length,

						     SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH);

		}

	} else {
@@ -4280,7 +4292,7 @@ iscsi_pdu_hdr_op_data(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) 
		pdu->mobj[0] = mobj;

		pdu->data = (void *)((uint64_t)mobj->buf + mobj->data_len);

		pdu->data_from_mempool = true;

		pdu->data_buf_len = SPDK_BDEV_BUF_SIZE_WITH_MD(SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH);

		pdu->data_buf_len = SPDK_BDEV_BUF_SIZE_WITH_MD(iscsi_get_mobj_max_data_len(mobj));



		iscsi_task_set_mobj(task, NULL);

	}
@@ -4626,7 +4638,7 @@ iscsi_pdu_payload_read(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) 
		pdu->mobj[0] = mobj;

		pdu->data = mobj->buf;

		pdu->data_from_mempool = true;

	} else if (mobj->data_len == SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH && read_len > 0) {

	} else if (mobj->data_len == iscsi_get_mobj_max_data_len(mobj) && read_len > 0) {

		mobj = pdu->mobj[1];

		if (mobj == NULL) {

			/* The first data buffer just ran out. Allocate the second data buffer and
@@ -4650,7 +4662,7 @@ iscsi_pdu_payload_read(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) 
	}



	/* copy the actual data into local buffer */

	read_len = spdk_min(read_len, SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH - mobj->data_len);

	read_len = spdk_min(read_len, iscsi_get_mobj_max_data_len(mobj) - mobj->data_len);



	if (read_len > 0) {

		rc = iscsi_conn_read_data_segment(conn,

test/unit/lib/iscsi/iscsi.c/iscsi_ut.c

+21 −15
Original line number Diff line number Diff line
@@ -122,6 +122,21 @@ DEFINE_STUB(spdk_scsi_lun_get_dif_ctx, bool, 
	    (struct spdk_scsi_lun *lun, struct spdk_scsi_task *task,

	     struct spdk_dif_ctx *dif_ctx), false);



static void

alloc_mock_mobj(struct spdk_mobj *mobj, int len)

{

	mobj->buf = calloc(1, SPDK_BDEV_BUF_SIZE_WITH_MD(len));

	SPDK_CU_ASSERT_FATAL(mobj->buf != NULL);



	g_iscsi.pdu_immediate_data_pool = (struct spdk_mempool *)100;

	g_iscsi.pdu_data_out_pool = (struct spdk_mempool *)200;

	if (len == SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH) {

		mobj->mp = g_iscsi.pdu_data_out_pool;

	} else {

		mobj->mp = g_iscsi.pdu_immediate_data_pool;

	}

}



static void

op_login_check_target_test(void)

{
@@ -2037,11 +2052,8 @@ pdu_payload_read_test(void) 


	g_iscsi.FirstBurstLength = SPDK_ISCSI_FIRST_BURST_LENGTH;



	mobj1.buf = calloc(1, SPDK_BDEV_BUF_SIZE_WITH_MD(SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH));

	SPDK_CU_ASSERT_FATAL(mobj1.buf != NULL);



	mobj2.buf = calloc(1, SPDK_BDEV_BUF_SIZE_WITH_MD(SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH));

	SPDK_CU_ASSERT_FATAL(mobj2.buf != NULL);

	alloc_mock_mobj(&mobj1, SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH);

	alloc_mock_mobj(&mobj2, SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH);



	MOCK_SET(spdk_mempool_get, &mobj1);
@@ -2212,14 +2224,9 @@ data_out_pdu_sequence_test(void) 


	TAILQ_INSERT_TAIL(&dev.luns, &lun, tailq);



	mobj1.buf = calloc(1, SPDK_BDEV_BUF_SIZE_WITH_MD(SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH));

	SPDK_CU_ASSERT_FATAL(mobj1.buf != NULL);



	mobj2.buf = calloc(1, SPDK_BDEV_BUF_SIZE_WITH_MD(SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH));

	SPDK_CU_ASSERT_FATAL(mobj2.buf != NULL);



	mobj3.buf = calloc(1, SPDK_BDEV_BUF_SIZE_WITH_MD(SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH));

	SPDK_CU_ASSERT_FATAL(mobj3.buf != NULL);

	alloc_mock_mobj(&mobj1, SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH);

	alloc_mock_mobj(&mobj2, SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH);

	alloc_mock_mobj(&mobj3, SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH);



	/* Test scenario is as follows.

	 *
@@ -2382,8 +2389,7 @@ immediate_data_and_data_out_pdu_sequence_test(void) 


	TAILQ_INSERT_TAIL(&dev.luns, &lun, tailq);



	mobj.buf = calloc(1, SPDK_BDEV_BUF_SIZE_WITH_MD(SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH));

	SPDK_CU_ASSERT_FATAL(mobj.buf != NULL);

	alloc_mock_mobj(&mobj, SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH);



	/* Test scenario is as follows.

	 *