test-tool: Avoid that using receive_copy_results() triggers a use-after-free

Move the scsi_free_scsi_task() call from receive_copy_results() to the
callers of this function to avoid that accessing the unmarshalled data
triggers a use-after-free.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>

test-tool/iscsi-support.c

@@ -2956,22 +2956,27 @@ void populate_param_header(unsigned char *buf, int list_id, int str, int list_id
 	buf[15] = inline_data_len & 0xFF;
 }
 
-int receive_copy_results(struct scsi_device *sdev, enum scsi_copy_results_sa sa, int list_id, void **datap, int status, enum scsi_sense_key key, int *ascq, int num_ascq)
+int receive_copy_results(struct scsi_task **task, struct scsi_device *sdev,
+			 enum scsi_copy_results_sa sa, int list_id,
+			 void **datap, int status, enum scsi_sense_key key,
+			 int *ascq, int num_ascq)
 {
 	int ret;
-	struct scsi_task *task;
 
 	logging(LOG_VERBOSE, "Send RECEIVE COPY RESULTS");
 
-	task = scsi_cdb_receive_copy_results(sa, list_id, 1024);
+	*task = scsi_cdb_receive_copy_results(sa, list_id, 1024);
 	assert(task != NULL);
 
-	task = send_scsi_command(sdev, task, NULL);
+	*task = send_scsi_command(sdev, *task, NULL);
 
-	ret = check_result("RECEIVECOPYRESULT", sdev, task, status, key, ascq, num_ascq);
+	ret = check_result("RECEIVECOPYRESULT", sdev, *task, status, key, ascq,
+			   num_ascq);
+	if (ret < 0)
+		return ret;
 
-	if (task->status == SCSI_STATUS_GOOD && datap != NULL) {
+	if ((*task)->status == SCSI_STATUS_GOOD && datap != NULL) {
-		*datap = scsi_datain_unmarshall(task);
+		*datap = scsi_datain_unmarshall(*task);
 		if (*datap == NULL) {
 			logging(LOG_NORMAL,
 					"[FAIL] failed to unmarshall RECEIVE COPY RESULTS data. %s",
@@ -2980,11 +2985,8 @@ int receive_copy_results(struct scsi_device *sdev, enum scsi_copy_results_sa sa,
 		}
 	}
 
-	ret = check_result("RECEIVECOPYRESULT", sdev, task, status, key, ascq, num_ascq);
+	return check_result("RECEIVECOPYRESULT", sdev, *task, status, key, ascq,
-	if (task)
+			    num_ascq);
-		scsi_free_scsi_task(task);
-	
-	return ret;
 }
 
 #define TEST_ISCSI_TUR_MAX_RETRIES 5

test-tool/iscsi-support.h

@@ -337,6 +337,9 @@ int populate_tgt_desc(unsigned char *desc, enum ec_descr_type_code desc_type, in
 int populate_seg_desc_hdr(unsigned char *hdr, enum ec_descr_type_code desc_type, int dc, int cat, int src_index, int dst_index);
 int populate_seg_desc_b2b(unsigned char *desc, int dc, int cat, int src_index, int dst_index, int num_blks, uint64_t src_lba, uint64_t dst_lba);
 void populate_param_header(unsigned char *buf, int list_id, int str, int list_id_usage, int prio, int tgt_desc_len, int seg_desc_len, int inline_data_len);
-int receive_copy_results(struct scsi_device *sdev, enum scsi_copy_results_sa sa, int list_id, void **datap, int status, enum scsi_sense_key key, int *ascq, int num_ascq);
+int receive_copy_results(struct scsi_task **task, struct scsi_device *sdev,
+			 enum scsi_copy_results_sa sa, int list_id,
+			 void **datap, int status, enum scsi_sense_key key,
+			 int *ascq, int num_ascq);
 int test_iscsi_tur_until_good(struct scsi_device *iscsi_sd, int *num_uas);
 #endif	/* _ISCSI_SUPPORT_H_ */

test-tool/test_extendedcopy_descr_limits.c

@@ -52,6 +52,7 @@ void
 test_extendedcopy_descr_limits(void)
 {
 	int ret;
+	struct scsi_task *edl_task;
 	struct iscsi_data data;
 	unsigned char *xcopybuf;
 	struct scsi_copy_results_op_params *opp;
@@ -64,12 +65,12 @@ test_extendedcopy_descr_limits(void)
 	CHECK_FOR_DATALOSS;
 
 	logging(LOG_VERBOSE, "Issue RECEIVE COPY RESULTS (OPERATING PARAMS)");
-	ret = receive_copy_results(sd, SCSI_COPY_RESULTS_OP_PARAMS, 0,
+	ret = receive_copy_results(&edl_task, sd, SCSI_COPY_RESULTS_OP_PARAMS, 0,
-			(void **)&opp, EXPECT_STATUS_GOOD);
+				   (void **)&opp, EXPECT_STATUS_GOOD);
 	if (ret < 0) {
 		CU_PASS("[SKIPPED] Target does not support "
 				"RECEIVE_COPY_RESULTS. Skipping test");
-		return;
+		goto out;
 	}
 	CU_ASSERT_EQUAL(ret, 0);
 
@@ -95,7 +96,7 @@ test_extendedcopy_descr_limits(void)
 	if (ret == -2) {
 		CU_PASS("[SKIPPED] Target does not support "
 				"EXTENDED_COPY. Skipping test");
-		return;
+		goto out;
 	}
 	CU_ASSERT_EQUAL(ret, 0);
 
@@ -123,4 +124,7 @@ test_extendedcopy_descr_limits(void)
 		ret = extendedcopy(sd, &data, EXPECT_PARAM_LIST_LEN_ERR);
 		CU_ASSERT_EQUAL(ret, 0);
 	}
+
+out:
+	scsi_free_scsi_task(edl_task);
 }

test-tool/test_receive_copy_results_copy_status.c

@@ -30,6 +30,7 @@ void
 test_receive_copy_results_copy_status(void)
 {
 	int ret;
+	struct scsi_task *cs_task;
 	struct scsi_copy_results_copy_status *csp;
 	int tgt_desc_len = 0, seg_desc_len = 0;
 	int  offset = XCOPY_DESC_OFFSET, list_id = 1;
@@ -40,12 +41,14 @@ test_receive_copy_results_copy_status(void)
 	logging(LOG_VERBOSE, "Test RECEIVE COPY RESULTS, COPY STATUS");
 
 	logging(LOG_VERBOSE, "No copy in progress");
-	ret = receive_copy_results(sd, SCSI_COPY_RESULTS_COPY_STATUS,
+	ret = receive_copy_results(&cs_task, sd, SCSI_COPY_RESULTS_COPY_STATUS,
 			list_id, NULL, EXPECT_INVALID_FIELD_IN_CDB);
+	scsi_free_scsi_task(cs_task);
+	cs_task = NULL;
 	if (ret == -2) {
 		CU_PASS("[SKIPPED] Target does not support "
 				"RECEIVE_COPY_STATUS. Skipping test");
-		return;
+		goto out;
 	}
 	CU_ASSERT_EQUAL(ret, 0);
 
@@ -77,13 +80,16 @@ test_receive_copy_results_copy_status(void)
 	if (ret == -2) {
 		CU_PASS("[SKIPPED] Target does not support "
 				"EXTENDED_COPY. Skipping test");
-		return;
+		goto out;
 	}
 	CU_ASSERT_EQUAL(ret, 0);
 
 	logging(LOG_VERBOSE,
 			"Copy Status for the above Extended Copy command");
-	ret = receive_copy_results(sd, SCSI_COPY_RESULTS_COPY_STATUS,
+	ret = receive_copy_results(&cs_task, sd, SCSI_COPY_RESULTS_COPY_STATUS,
 			list_id, (void **)&csp, EXPECT_STATUS_GOOD);
 	CU_ASSERT_EQUAL(ret, 0);
+
+out:
+	scsi_free_scsi_task(cs_task);
 }

test-tool/test_receive_copy_results_op_params.c

@@ -30,16 +30,17 @@ void
 test_receive_copy_results_op_params(void)
 {
 	int ret;
+	struct scsi_task *op_task;
 	struct scsi_copy_results_op_params *opp;
 
 	logging(LOG_VERBOSE, LOG_BLANK_LINE);
 	logging(LOG_VERBOSE, "Test RECEIVE COPY RESULTS, OPERATING PARAMS");
 
-	ret = receive_copy_results(sd, SCSI_COPY_RESULTS_OP_PARAMS, 0,
+	ret = receive_copy_results(&op_task, sd, SCSI_COPY_RESULTS_OP_PARAMS, 0,
 			(void **)&opp, EXPECT_STATUS_GOOD);
 	if (ret == -2) {
 		CU_PASS("[SKIPPED] RECEIVE_COPY_RESULT is not implemented.");
-		return;
+		goto out;
 	}
 	CU_ASSERT_EQUAL(ret, 0);
 
@@ -47,4 +48,7 @@ test_receive_copy_results_op_params(void)
 			"max_target_desc=%d, max_seg_desc=%d",
 			opp->max_target_desc_count,
 			opp->max_segment_desc_count);
+
+out:
+	scsi_free_scsi_task(op_task);
 }