- A complete fix for the problem when a command comes with zero transfer length and READ or WRITE data transfer direction

- Version changed to 1.0.0-rc2


git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@423 d57e44dd-8a1f-0410-8b47-8ef2f437770f

iscsi-scst/usr/chap.c

@@ -360,7 +360,7 @@ static int chap_initiator_auth_create_challenge(struct connection *conn)
 	text_key_add(conn, "CHAP_I", text);
 
 	/*
-	 * FIXME: does a random challenge length provide any benefits security-
+	 * ToDo: does a random challenge length provide any benefits security-
 	 * wise, or should we rather always use the max. allowed length of
 	 * 1024 for the (unencoded) challenge?
 	 */
@@ -578,7 +578,7 @@ static int chap_target_auth_create_response(struct connection *conn)
 	if (challenge_len == conn->auth.chap.challenge_size) {
 		if (!memcmp(challenge, conn->auth.chap.challenge,
 			    challenge_len)) {
-			//FIXME: RFC 3720 demands to close TCP conn.
+			/* ToDo: RFC 3720 demands to close TCP conn */
 			log_warning("CHAP target auth.: "
 				    "initiator %s reflected our challenge",
 				    conn->initiator);

iscsi-scst/usr/isns.c

@@ -550,7 +550,7 @@ static int recv_pdu(int fd, struct isns_io *rx, struct isns_hdr *hdr)
 		  transaction, sequence);
 
 	if (length + sizeof(*hdr) > BUFSIZE) {
-		log_error("FIXME we cannot handle this yet %u!", length);
+		log_error("ToDo: we cannot handle this yet %u!", length);
 		return -1;
 	}
 

scst/include/scst.h

@@ -50,7 +50,7 @@ typedef _Bool bool;
 /* Version numbers, the same as for the kernel */
 #define SCST_VERSION_CODE 0x00090601
 #define SCST_VERSION(a, b, c, d) (((a) << 24) + ((b) << 16) + ((c) << 8) + d)
-#define SCST_VERSION_STRING "1.0.0-rc1"
+#define SCST_VERSION_STRING "1.0.0-rc2"
 #define SCST_INTERFACE_VERSION SCST_VERSION_STRING "$Revision$" SCST_CONST_VERSION
 
 /*************************************************************

scst/include/scst_user.h

@@ -26,7 +26,7 @@
 
 #define DEV_USER_NAME			"scst_user"
 #define DEV_USER_PATH			"/dev/"
-#define DEV_USER_VERSION_NAME		"1.0.0-rc1"
+#define DEV_USER_VERSION_NAME		"1.0.0-rc2"
 #define DEV_USER_VERSION		DEV_USER_VERSION_NAME "$Revision$" SCST_CONST_VERSION
 
 /*

scst/src/dev_handlers/scst_user.c

@@ -490,11 +490,7 @@ static int dev_user_alloc_sg(struct scst_user_cmd *ucmd, int cached_buff)
 
 	TRACE_ENTRY();
 
-	/* User space can return from PARSE bufflen 0 and direction non-NONE */
-	if (unlikely(bufflen == 0)) {
-		cmd->data_direction = SCST_DATA_NONE;
-		goto out;
-	}
+	sBUG_ON(bufflen == 0);
 
 	gfp_mask = __GFP_NOWARN;
 	gfp_mask |= (scst_cmd_atomic(cmd) ? GFP_ATOMIC : GFP_KERNEL);

scst/src/dev_handlers/scst_vdisk.c

@@ -1094,9 +1094,11 @@ static void vdisk_exec_inquiry(struct scst_cmd *cmd)
 	length = scst_get_buf_first(cmd, &address);
 	TRACE_DBG("length %d", length);
 	if (unlikely(length <= 0)) {
-		PRINT_ERROR("scst_get_buf_first() failed: %d", length);
-		scst_set_cmd_error(cmd,
-		    SCST_LOAD_SENSE(scst_sense_hardw_error));
+		if (length < 0) {
+			PRINT_ERROR("scst_get_buf_first() failed: %d", length);
+			scst_set_cmd_error(cmd,
+				SCST_LOAD_SENSE(scst_sense_hardw_error));
+		}
 		goto out_free;
 	}
 
@@ -1257,10 +1259,13 @@ static void vdisk_exec_request_sense(struct scst_cmd *cmd)
 	length = scst_get_buf_first(cmd, &address);
 	TRACE_DBG("length %d", length);
 	if (unlikely(length < SCST_STANDARD_SENSE_LEN)) {
-		PRINT_ERROR("scst_get_buf_first() failed or too small "
-			"requested buffer (returned %d)", length);
-		scst_set_cmd_error(cmd,
-		    SCST_LOAD_SENSE(scst_sense_invalid_field_in_parm_list));
+		if (length != 0) {
+			PRINT_ERROR("scst_get_buf_first() failed or too small "
+				"requested buffer (returned %d)", length);
+			scst_set_cmd_error(cmd,
+				SCST_LOAD_SENSE(
+					scst_sense_invalid_field_in_parm_list));
+		}
 		if (length > 0)
 			goto out_put;
 		else
@@ -1423,9 +1428,11 @@ static void vdisk_exec_mode_sense(struct scst_cmd *cmd)
 
 	length = scst_get_buf_first(cmd, &address);
 	if (unlikely(length <= 0)) {
-		PRINT_ERROR("scst_get_buf_first() failed: %d", length);
-		scst_set_cmd_error(cmd,
-		    SCST_LOAD_SENSE(scst_sense_hardw_error));
+		if (length < 0) {
+			PRINT_ERROR("scst_get_buf_first() failed: %d", length);
+			scst_set_cmd_error(cmd,
+				SCST_LOAD_SENSE(scst_sense_hardw_error));
+		}
 		goto out_free;
 	}
 
@@ -1576,9 +1583,11 @@ static void vdisk_exec_mode_select(struct scst_cmd *cmd)
 
 	length = scst_get_buf_first(cmd, &address);
 	if (unlikely(length <= 0)) {
-		PRINT_ERROR("scst_get_buf_first() failed: %d", length);
-		scst_set_cmd_error(cmd,
-		    SCST_LOAD_SENSE(scst_sense_hardw_error));
+		if (length < 0) {
+			PRINT_ERROR("scst_get_buf_first() failed: %d", length);
+			scst_set_cmd_error(cmd,
+				SCST_LOAD_SENSE(scst_sense_hardw_error));
+		}
 		goto out;
 	}
 
@@ -1708,9 +1717,11 @@ static void vdisk_exec_read_capacity(struct scst_cmd *cmd)
 
 	length = scst_get_buf_first(cmd, &address);
 	if (unlikely(length <= 0)) {
-		PRINT_ERROR("scst_get_buf_first() failed: %d", length);
-		scst_set_cmd_error(cmd,
-		    SCST_LOAD_SENSE(scst_sense_hardw_error));
+		if (length < 0) {
+			PRINT_ERROR("scst_get_buf_first() failed: %d", length);
+			scst_set_cmd_error(cmd,
+				SCST_LOAD_SENSE(scst_sense_hardw_error));
+		}
 		goto out;
 	}
 
@@ -1760,9 +1771,11 @@ static void vdisk_exec_read_capacity16(struct scst_cmd *cmd)
 
 	length = scst_get_buf_first(cmd, &address);
 	if (unlikely(length <= 0)) {
-		PRINT_ERROR("scst_get_buf_first() failed: %d", length);
-		scst_set_cmd_error(cmd,
-		    SCST_LOAD_SENSE(scst_sense_hardw_error));
+		if (length < 0) {
+			PRINT_ERROR("scst_get_buf_first() failed: %d", length);
+			scst_set_cmd_error(cmd,
+				SCST_LOAD_SENSE(scst_sense_hardw_error));
+			}
 		goto out;
 	}
 
@@ -1816,14 +1829,16 @@ static void vdisk_exec_read_toc(struct scst_cmd *cmd)
 
 	length = scst_get_buf_first(cmd, &address);
 	if (unlikely(length <= 0)) {
-		PRINT_ERROR("scst_get_buf_first() failed: %d", length);
-		scst_set_cmd_error(cmd,
-		    SCST_LOAD_SENSE(scst_sense_hardw_error));
+		if (length < 0) {
+			PRINT_ERROR("scst_get_buf_first() failed: %d", length);
+			scst_set_cmd_error(cmd,
+				SCST_LOAD_SENSE(scst_sense_hardw_error));
+		}
 		goto out;
 	}
 
 	virt_dev = (struct scst_vdisk_dev *)cmd->dev->dh_priv;
-	/* FIXME when you have > 8TB ROM device. */
+	/* ToDo when you have > 8TB ROM device. */
 	nblocks = (uint32_t)virt_dev->nblocks;
 
 	/* Header */
@@ -2459,7 +2474,8 @@ static void vdisk_exec_verify(struct scst_cmd *cmd,
 				scst_set_cmd_error(cmd,
 				    SCST_LOAD_SENSE(scst_sense_read_error));
 			}
-			scst_put_buf(cmd, address_sav);
+			if (compare)
+				scst_put_buf(cmd, address_sav);
 			goto out_set_fs;
 		}
 		if (compare && memcmp(address, mem_verify, len_mem) != 0) {

scst/src/scst_lib.c

@@ -1646,7 +1646,6 @@ int scst_alloc_space(struct scst_cmd *cmd)
 	int atomic = scst_cmd_atomic(cmd);
 	int flags;
 	struct scst_tgt_dev *tgt_dev = cmd->tgt_dev;
-	int bufflen = cmd->bufflen;
 
 	TRACE_ENTRY();
 
@@ -1656,21 +1655,7 @@ int scst_alloc_space(struct scst_cmd *cmd)
 	if (cmd->no_sgv)
 		flags |= SCST_POOL_ALLOC_NO_CACHED;
 
-	if (unlikely(cmd->bufflen == 0)) {
-		/* ToDo: remove when 1.0.1 will be started */
-		TRACE(TRACE_MGMT_MINOR, "Warning: data direction %d or/and "
-			"zero buffer length. Opcode 0x%x, handler %s, target "
-			"%s", cmd->data_direction, cmd->cdb[0],
-			cmd->dev->handler->name, cmd->tgtt->name);
-		/*
-		 * Be on the safe side and alloc stub buffer. Neither target
-		 * drivers, nor user space will touch it, since bufflen
-		 * remains 0.
-		 */
-		bufflen = PAGE_SIZE;
-	}
-
-	cmd->sg = sgv_pool_alloc(tgt_dev->pool, bufflen, gfp_mask, flags,
+	cmd->sg = sgv_pool_alloc(tgt_dev->pool, cmd->bufflen, gfp_mask, flags,
 			&cmd->sg_cnt, &cmd->sgv, &cmd->dev->dev_mem_lim, NULL);
 	if (cmd->sg == NULL)
 		goto out;
@@ -1916,7 +1901,7 @@ int scst_get_cdb_info(struct scst_cmd *cmd)
 	cmd->data_direction = ptr->direction;
 	cmd->op_flags = ptr->flags;
 	res = (*ptr->get_trans_len)(cmd, ptr->off);
-#if 0 /* ToDo: enable when 1.0.1 will be started and fix all scst_get_buf_first() returns 0 cases */
+
 	if (unlikely(cmd->bufflen == 0)) {
 		/*
 		 * According to SPC bufflen 0 for data transfer commands isn't
@@ -1924,7 +1909,6 @@ int scst_get_cdb_info(struct scst_cmd *cmd)
 		 */
 		cmd->data_direction = SCST_DATA_NONE;
 	}
-#endif
 
 out:
 	TRACE_EXIT();
@@ -2355,8 +2339,10 @@ int scst_block_generic_dev_done(struct scst_cmd *cmd,
 
 			buffer_size = scst_get_buf_first(cmd, &buffer);
 			if (unlikely(buffer_size <= 0)) {
-				PRINT_ERROR("%s: Unable to get the buffer "
-					"(%d)",	__func__, buffer_size);
+				if (buffer_size < 0) {
+					PRINT_ERROR("%s: Unable to get the buffer "
+						"(%d)",	__func__, buffer_size);
+				}
 				goto out;
 			}
 
@@ -2408,8 +2394,10 @@ int scst_tape_generic_dev_done(struct scst_cmd *cmd,
 	case MODE_SELECT:
 		buffer_size = scst_get_buf_first(cmd, &buffer);
 		if (unlikely(buffer_size <= 0)) {
-			PRINT_ERROR("%s: Unable to get the buffer (%d)",
-				__func__, buffer_size);
+			if (buffer_size < 0) {
+				PRINT_ERROR("%s: Unable to get the buffer (%d)",
+					__func__, buffer_size);
+			}
 			goto out;
 		}
 		break;

scst/src/scst_targ.c

@@ -1293,7 +1293,9 @@ static int scst_report_luns_local(struct scst_cmd *cmd)
 	}
 
 	buffer_size = scst_get_buf_first(cmd, &buffer);
-	if (unlikely(buffer_size <= 0))
+	if (unlikely(buffer_size == 0))
+		goto out_compl;
+	else if (unlikely(buffer_size < 0))
 		goto out_err;
 
 	if (buffer_size < 16)
@@ -1340,13 +1342,17 @@ inc_dev_cnt:
 
 	/* Set the response header */
 	buffer_size = scst_get_buf_first(cmd, &buffer);
-	if (unlikely(buffer_size <= 0))
+	if (unlikely(buffer_size == 0))
+		goto out_compl;
+	else if (unlikely(buffer_size < 0))
 		goto out_err;
+
 	dev_cnt *= 8;
 	buffer[0] = (dev_cnt >> 24) & 0xff;
 	buffer[1] = (dev_cnt >> 16) & 0xff;
 	buffer[2] = (dev_cnt >> 8) & 0xff;
 	buffer[3] = dev_cnt & 0xff;
+
 	scst_put_buf(cmd, buffer);
 
 	dev_cnt += 8;
@@ -2220,9 +2226,16 @@ static int scst_done_cmd_check(struct scst_cmd **pcmd, int *pres)
 			uint8_t *address;
 
 			length = scst_get_buf_first(cmd, &address);
-			if (length <= 0)
+			if (length < 0) {
+				PRINT_ERROR("%s", "Unable to get "
+					"MODE_SENSE buffer");
+				scst_set_cmd_error(cmd,
+					SCST_LOAD_SENSE(
+						scst_sense_hardw_error));
 				goto out;
-			if (length > 2 && cmd->cdb[0] == MODE_SENSE)
+			} else if (length == 0) {
+				goto out;
+			} else if (length > 2 && cmd->cdb[0] == MODE_SENSE)
 				address[2] |= 0x80;   /* Write Protect*/
 			else if (length > 3 && cmd->cdb[0] == MODE_SENSE_10)
 				address[3] |= 0x80;   /* Write Protect*/
@@ -2252,12 +2265,13 @@ static int scst_done_cmd_check(struct scst_cmd **pcmd, int *pres)
 				}
 #endif
 				buffer[SCST_INQ_BYTE3] &= ~SCST_INQ_NORMACA_BIT;
-			} else {
+			} else if (buflen != 0) {
 				PRINT_ERROR("%s", "Unable to get INQUIRY "
 				    "buffer");
 				scst_set_cmd_error(cmd,
-				   SCST_LOAD_SENSE(scst_sense_hardw_error));
+					SCST_LOAD_SENSE(scst_sense_hardw_error));
 			}
+
 			if (buflen > 0)
 				scst_put_buf(cmd, buffer);
 		}

usr/fileio/common.c

@@ -1424,7 +1424,7 @@ static void exec_read_toc(struct vdisk_cmd *vcmd)
 		goto out;
 	}
 
-	/* FIXME when you have > 8TB ROM device. */
+	/* ToDo when you have > 8TB ROM device. */
 	nblocks = (uint32_t)dev->nblocks;
 
 	/* Header */