cmd's bufflen and data_len overflow cleanups

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

scst/include/scst.h

@@ -2041,7 +2041,7 @@ struct scst_cmd {
 	 * VERIFY, which transfer different amount of data (if any), than
 	 * processed.
 	 */
-	int data_len;
+	int64_t data_len;
 
 	/* Completion routine */
 	void (*scst_cmd_done) (struct scst_cmd *cmd, int next_state,
@@ -3161,7 +3161,7 @@ static inline int scst_cmd_get_bufflen(struct scst_cmd *cmd)
  * Returns cmd's data_len. See the corresponding field's description in
  * struct scst_cmd above.
  */
-static inline int scst_cmd_get_data_len(struct scst_cmd *cmd)
+static inline int64_t scst_cmd_get_data_len(struct scst_cmd *cmd)
 {
 	return cmd->data_len;
 }

scst/include/scst_user.h

@@ -131,10 +131,10 @@ struct scst_user_scsi_cmd_parse {
 
 	aligned_i64 lba;
 
-	int32_t timeout;
+	aligned_i64 data_len;
 	int32_t bufflen;
-	int32_t data_len;
 	int32_t out_bufflen;
+	int32_t timeout;
 
 	uint32_t op_flags;
 
@@ -171,7 +171,7 @@ struct scst_user_scsi_cmd_exec {
 
 	aligned_i64 lba;
 
-	int32_t data_len;
+	aligned_i64 data_len;
 	int32_t bufflen;
 	int32_t alloc_len;
 	aligned_u64 pbuf;
@@ -235,9 +235,9 @@ struct scst_user_scsi_cmd_reply_parse {
 			uint8_t data_direction;
 			uint16_t cdb_len;
 			aligned_i64 lba;
-			uint32_t op_flags;
-			int32_t data_len;
+			aligned_i64 data_len;
 			int32_t bufflen;
+			uint32_t op_flags;
 			int32_t out_bufflen;
 		};
 		struct {

scst/src/dev_handlers/scst_user.c

@@ -903,8 +903,9 @@ static int dev_user_exec(struct scst_cmd *cmd)
 	TRACE_ENTRY();
 
 	TRACE_DBG("Preparing EXEC for user space (ucmd=%p, h=%d, lba %lld, "
-		"bufflen %d, data_len %d, ubuff %lx)", ucmd, ucmd->h,
-		(long long)cmd->lba, cmd->bufflen, cmd->data_len, ucmd->ubuff);
+		"bufflen %d, data_len %lld, ubuff %lx)", ucmd, ucmd->h,
+		(long long)cmd->lba, cmd->bufflen, (long long)cmd->data_len,
+		ucmd->ubuff);
 
 	if (cmd->data_direction & SCST_DATA_WRITE)
 		dev_user_flush_dcache(ucmd);
@@ -1330,9 +1331,9 @@ static int dev_user_process_reply_parse(struct scst_user_cmd *ucmd,
 		goto out_inval;
 
 	TRACE_DBG("ucmd %p, queue_type %x, data_direction, %x, lba %lld, "
-		"bufflen %d, data_len %d, pbuf %llx, cdb_len %d, op_flags %x",
+		"bufflen %d, data_len %lld, pbuf %llx, cdb_len %d, op_flags %x",
 		ucmd, preply->queue_type, preply->data_direction,
-		(long long)preply->lba, preply->bufflen, preply->data_len,
+		(long long)preply->lba, preply->bufflen, (long long)preply->data_len,
 		reply->alloc_reply.pbuf, preply->cdb_len, preply->op_flags);
 
 	cmd->queue_type = preply->queue_type;

scst/src/dev_handlers/scst_vdisk.c

@@ -995,7 +995,7 @@ static enum compl_status_e vdisk_synchronize_cache(struct vdisk_cmd_params *p)
 	const uint8_t *cdb = cmd->cdb;
 	struct scst_device *dev = cmd->dev;
 	const loff_t loff = p->loff;
-	int data_len = scst_cmd_get_data_len(cmd);
+	int64_t data_len = scst_cmd_get_data_len(cmd);
 	int immed = cdb[1] & 0x2;
 	enum compl_status_e res;
 
@@ -1165,7 +1165,7 @@ static vdisk_op_fn nullio_ops[256] = {
 static bool vdisk_parse_offset(struct vdisk_cmd_params *p, struct scst_cmd *cmd)
 {
 	uint64_t lba_start;
-	int data_len;
+	int64_t data_len;
 	uint8_t *cdb = cmd->cdb;
 	int opcode = cdb[0];
 	loff_t loff;
@@ -3867,7 +3867,7 @@ static enum compl_status_e fileio_exec_verify(struct vdisk_cmd_params *p)
 	struct scst_vdisk_dev *virt_dev = cmd->dev->dh_priv;
 	struct file *fd = virt_dev->fd;
 	uint8_t *mem_verify = NULL;
-	int data_len = scst_cmd_get_data_len(cmd);
+	int64_t data_len = scst_cmd_get_data_len(cmd);
 
 	TRACE_ENTRY();
 
@@ -3886,8 +3886,8 @@ static enum compl_status_e fileio_exec_verify(struct vdisk_cmd_params *p)
 	 */
 
 	compare = scst_cmd_get_data_direction(cmd) == SCST_DATA_WRITE;
-	TRACE_DBG("VERIFY with BYTCHK=%d at offset %lld and len %d\n",
-		  compare, loff, data_len);
+	TRACE_DBG("VERIFY with BYTCHK=%d at offset %lld and len %lld\n",
+		  compare, loff, (long long)data_len);
 
 	/* SEEK */
 	old_fs = get_fs();

scst/src/scst_lib.c

@@ -6207,6 +6207,8 @@ static int get_cdb_info_serv_act_in(struct scst_cmd *cmd,
 	case SAI_READ_CAPACITY_16:
 		cmd->op_name = "READ CAPACITY(16)";
 		cmd->bufflen = get_unaligned_be32(&cmd->cdb[10]);
+		if (unlikely(cmd->bufflen & SCST_MAX_VALID_BUFFLEN_MASK))
+			goto out_inval_bufflen10;
 		cmd->op_flags |= SCST_IMPLICIT_HQ | SCST_LBA_NOT_VALID |
 				SCST_REG_RESERVE_ALLOWED |
 				SCST_WRITE_EXCL_ALLOWED |
@@ -6216,6 +6218,8 @@ static int get_cdb_info_serv_act_in(struct scst_cmd *cmd,
 		cmd->op_name = "GET LBA STATUS";
 		cmd->lba = get_unaligned_be64(&cmd->cdb[2]);
 		cmd->bufflen = get_unaligned_be32(&cmd->cdb[10]);
+		if (unlikely(cmd->bufflen & SCST_MAX_VALID_BUFFLEN_MASK))
+			goto out_inval_bufflen10;
 		cmd->op_flags |= SCST_WRITE_EXCL_ALLOWED;
 		break;
 	default:
@@ -6225,8 +6229,15 @@ static int get_cdb_info_serv_act_in(struct scst_cmd *cmd,
 
 	cmd->data_len = cmd->bufflen;
 
+out:
 	TRACE_EXIT_RES(res);
 	return res;
+
+out_inval_bufflen10:
+	PRINT_ERROR("Too big bufflen %x (op %x)", cmd->bufflen, cmd->cdb[0]);
+	scst_set_invalid_field_in_cdb(cmd, 10, 0);
+	res = 1;
+	goto out;
 }
 
 static int get_cdb_info_single(struct scst_cmd *cmd,
@@ -6398,6 +6409,12 @@ static int get_cdb_info_verify12(struct scst_cmd *cmd,
 	cmd->lba = get_unaligned_be32(cmd->cdb + sdbops->info_lba_off);
 	if (cmd->cdb[1] & BYTCHK) {
 		cmd->bufflen = get_unaligned_be32(cmd->cdb + sdbops->info_len_off);
+		if (unlikely(cmd->bufflen & SCST_MAX_VALID_BUFFLEN_MASK)) {
+			PRINT_ERROR("Too big bufflen %x (op %x)",
+				cmd->bufflen, cmd->cdb[0]);
+			scst_set_invalid_field_in_cdb(cmd, sdbops->info_len_off, 0);
+			return 1;
+		}
 		cmd->data_len = cmd->bufflen;
 		cmd->data_direction = SCST_DATA_WRITE;
 	} else {
@@ -6414,6 +6431,12 @@ static int get_cdb_info_verify16(struct scst_cmd *cmd,
 	cmd->lba = get_unaligned_be64(cmd->cdb + sdbops->info_lba_off);
 	if (cmd->cdb[1] & BYTCHK) {
 		cmd->bufflen = get_unaligned_be32(cmd->cdb + sdbops->info_len_off);
+		if (unlikely(cmd->bufflen & SCST_MAX_VALID_BUFFLEN_MASK)) {
+			PRINT_ERROR("Too big bufflen %x (op %x)",
+				cmd->bufflen, cmd->cdb[0]);
+			scst_set_invalid_field_in_cdb(cmd, sdbops->info_len_off, 0);
+			return 1;
+		}
 		cmd->data_len = cmd->bufflen;
 		cmd->data_direction = SCST_DATA_WRITE;
 	} else {
@@ -6481,6 +6504,12 @@ static int get_cdb_info_len_4(struct scst_cmd *cmd,
 	cmd->op_flags |= SCST_LBA_NOT_VALID;
 	cmd->lba = 0;
 	cmd->bufflen = get_unaligned_be32(cmd->cdb + sdbops->info_len_off);
+	if (unlikely(cmd->bufflen & SCST_MAX_VALID_BUFFLEN_MASK)) {
+		PRINT_ERROR("Too big bufflen %x (op %x)", cmd->bufflen,
+			cmd->cdb[0]);
+		scst_set_invalid_field_in_cdb(cmd, sdbops->info_len_off, 0);
+		return 1;
+	}
 	cmd->data_len = cmd->bufflen;
 	return 0;
 }
@@ -6548,6 +6577,12 @@ static int get_cdb_info_lba_4_len_4(struct scst_cmd *cmd,
 {
 	cmd->lba = get_unaligned_be32(cmd->cdb + sdbops->info_lba_off);
 	cmd->bufflen = get_unaligned_be32(cmd->cdb + sdbops->info_len_off);
+	if (unlikely(cmd->bufflen & SCST_MAX_VALID_BUFFLEN_MASK)) {
+		PRINT_ERROR("Too big bufflen %x (op %x)", cmd->bufflen,
+			cmd->cdb[0]);
+		scst_set_invalid_field_in_cdb(cmd, sdbops->info_len_off, 0);
+		return 1;
+	}
 	cmd->data_len = cmd->bufflen;
 	return 0;
 }
@@ -6557,6 +6592,12 @@ static int get_cdb_info_lba_8_len_4(struct scst_cmd *cmd,
 {
 	cmd->lba = get_unaligned_be64(cmd->cdb + sdbops->info_lba_off);
 	cmd->bufflen = get_unaligned_be32(cmd->cdb + sdbops->info_len_off);
+	if (unlikely(cmd->bufflen & SCST_MAX_VALID_BUFFLEN_MASK)) {
+		PRINT_ERROR("Too big bufflen %x (op %x)", cmd->bufflen,
+			cmd->cdb[0]);
+		scst_set_invalid_field_in_cdb(cmd, sdbops->info_len_off, 0);
+		return 1;
+	}
 	cmd->data_len = cmd->bufflen;
 	return 0;
 }
@@ -6918,8 +6959,8 @@ int scst_sbc_generic_parse(struct scst_cmd *cmd)
 	else if (cmd->op_flags & SCST_LONG_TIMEOUT)
 		cmd->timeout = SCST_GENERIC_DISK_LONG_TIMEOUT;
 
-	TRACE_DBG("res %d, bufflen %d, data_len %d, direct %d",
-	      res, cmd->bufflen, cmd->data_len, cmd->data_direction);
+	TRACE_DBG("res %d, bufflen %d, data_len %lld, direct %d", res,
+		cmd->bufflen, (long long)cmd->data_len, cmd->data_direction);
 
 	TRACE_EXIT_RES(res);
 	return res;

scst/src/scst_pres.c

@@ -1164,7 +1164,8 @@ static int scst_pr_register_with_spec_i_pt(struct scst_cmd *cmd,
 	struct list_head *rollback_list)
 {
 	int res = 0;
-	int offset, ext_size;
+	int offset;
+	unsigned int ext_size;
 	__be64 action_key;
 	struct scst_device *dev = cmd->dev;
 	struct scst_dev_registrant *reg;
@@ -1653,7 +1654,7 @@ void scst_pr_register_and_move(struct scst_cmd *cmd, uint8_t *buffer,
 {
 	int aptpl;
 	int unreg;
-	int tid_buffer_size;
+	unsigned int tid_buffer_size;
 	__be64 key, action_key;
 	struct scst_device *dev = cmd->dev;
 	struct scst_tgt_dev *tgt_dev = cmd->tgt_dev;

scst/src/scst_priv.h

@@ -111,6 +111,9 @@ extern unsigned long scst_trace_flag;
 
 #define SCST_DEF_LBA_DATA_LEN		     -1
 
+/* Used to prevent overflow of int cmd->bufflen. Assumes max blocksize is 4K */
+#define SCST_MAX_VALID_BUFFLEN_MASK	     (~((1 << (32 - 12)) - 1))
+
 #define SCST_MAX_EACH_INTERNAL_IO_SIZE	     (128*1024)
 #define SCST_MAX_IN_FLIGHT_INTERNAL_COMMANDS 32
 

scst/src/scst_targ.c

@@ -521,12 +521,12 @@ int scst_pre_parse(struct scst_cmd *cmd)
 #endif
 
 	TRACE_DBG("op_name <%s> (cmd %p), direction=%d "
-		"(expected %d, set %s), lba %lld, bufflen=%d, data_len %d, "
+		"(expected %d, set %s), lba %lld, bufflen=%d, data_len %lld, "
 		"out_bufflen=%d (expected len %d, out expected len %d), "
 		"flags=0x%x", cmd->op_name, cmd, cmd->data_direction,
 		cmd->expected_data_direction,
 		scst_cmd_is_expected_set(cmd) ? "yes" : "no",
-		(long long)cmd->lba, cmd->bufflen, cmd->data_len,
+		(long long)cmd->lba, cmd->bufflen, (long long)cmd->data_len,
 		cmd->out_bufflen, cmd->expected_transfer_len,
 		cmd->expected_out_transfer_len, cmd->op_flags);
 
@@ -613,8 +613,7 @@ static int scst_parse_cmd(struct scst_cmd *cmd)
 	if (state == SCST_CMD_STATE_DEFAULT)
 		state = SCST_CMD_STATE_PREPARE_SPACE;
 
-	if (unlikely(state == SCST_CMD_STATE_PRE_XMIT_RESP) ||
-	    unlikely(state == SCST_CMD_STATE_PREPROCESSING_DONE))
+	if (unlikely(cmd->status != 0))
 		goto set_res;
 
 	if (unlikely(!(cmd->op_flags & SCST_INFO_VALID))) {
@@ -817,15 +816,15 @@ set_res:
 	}
 
 	TRACE(TRACE_SCSI, "op_name <%s> (cmd %p), direction=%d "
-		"(expected %d, set %s), lba=%lld, bufflen=%d, data len %d, "
+		"(expected %d, set %s), lba=%lld, bufflen=%d, data len %lld, "
 		"out_bufflen=%d, (expected len %d, out expected len %d), "
-		"flags=0x%x", cmd->op_name, cmd, cmd->data_direction,
-		cmd->expected_data_direction,
+		"flags=0x%x, internal %d", cmd->op_name, cmd,
+		cmd->data_direction, cmd->expected_data_direction,
 		scst_cmd_is_expected_set(cmd) ? "yes" : "no",
 		(unsigned long long)cmd->lba,
-		cmd->bufflen, cmd->data_len, cmd->out_bufflen,
+		cmd->bufflen, (long long)cmd->data_len, cmd->out_bufflen,
 		cmd->expected_transfer_len, cmd->expected_out_transfer_len,
-		cmd->op_flags);
+		cmd->op_flags, cmd->internal);
 
 #ifdef CONFIG_SCST_EXTRACHECKS
 	switch (state) {
@@ -901,9 +900,9 @@ out:
 			 (cmd->state >= SCST_CMD_STATE_LAST_ACTIVE)) &&
 			(cmd->state != SCST_CMD_STATE_PREPROCESSING_DONE))))) {
 		PRINT_CRIT_ERROR("Not initialized data_len for going to "
-			"execute command or bad state (cmd %p, data_len %d, "
-			"completed %d, state %d)", cmd, cmd->data_len,
-			cmd->completed, cmd->state);
+			"execute command or bad state (cmd %p, data_len %lld, "
+			"completed %d, state %d)", cmd,
+			(long long)cmd->data_len, cmd->completed, cmd->state);
 		sBUG();
 	}
 #endif

usr/fileio/common.c

@@ -184,7 +184,7 @@ static int do_exec(struct vdisk_cmd *vcmd)
 	struct scst_user_scsi_cmd_exec *cmd = &vcmd->cmd->exec_cmd;
 	struct scst_user_scsi_cmd_reply_exec *reply = &vcmd->reply->exec_reply;
 	uint64_t lba_start = cmd->lba;
-	loff_t data_len = cmd->data_len;
+	int64_t data_len = cmd->data_len;
 	uint8_t *cdb = cmd->cdb;
 	int opcode = cdb[0];
 	loff_t loff;
@@ -250,11 +250,11 @@ static int do_exec(struct vdisk_cmd *vcmd)
 	loff = (loff_t)lba_start << dev->block_shift;
 	TRACE_DBG("cmd %d, buf %"PRIx64", lba_start %"PRId64", loff %"PRId64
 		", data_len %"PRId64, vcmd->cmd->cmd_h, cmd->pbuf, lba_start,
-		(uint64_t)loff, (uint64_t)data_len);
+		(uint64_t)loff, data_len);
 	if ((loff < 0) || (data_len < 0) || ((loff + data_len) > dev->file_size)) {
 	    	PRINT_INFO("Access beyond the end of the device "
 			"(%"PRId64" of %"PRId64", len %"PRId64")", (uint64_t)loff,
-			(uint64_t)dev->file_size, (uint64_t)data_len);
+			(uint64_t)dev->file_size, data_len);
 		set_cmd_error(vcmd, SCST_LOAD_SENSE(
 				scst_sense_block_out_range_error));
 		goto out;
@@ -268,7 +268,7 @@ static int do_exec(struct vdisk_cmd *vcmd)
 		if (cdb[1] & 0x8) {
 			TRACE(TRACE_ORDER, "FUA(%d): loff=%"PRId64", "
 				"data_len=%"PRId64, fua, (uint64_t)loff,
-				(uint64_t)data_len);
+				data_len);
 		}
 		break;
 	}
@@ -343,7 +343,7 @@ static int do_exec(struct vdisk_cmd *vcmd)
 				((loff_t)lba_start << dev->block_shift);
 		TRACE(TRACE_ORDER, "SYNCHRONIZE_CACHE: "
 			"loff=%"PRId64", data_len=%"PRId64", immed=%d",
-			(uint64_t)loff, (uint64_t)data_len, immed);
+			(uint64_t)loff, data_len, immed);
 		if (immed) {
 			/* ToDo: backgroung exec */
 			exec_fsync(vcmd);
@@ -1617,7 +1617,7 @@ static void exec_verify(struct vdisk_cmd *vcmd, loff_t loff)
 	struct scst_user_scsi_cmd_exec *cmd = &vcmd->cmd->exec_cmd;
 	struct scst_user_scsi_cmd_reply_exec *reply = &vcmd->reply->exec_reply;
 	loff_t err;
-	int length = cmd->bufflen;
+	int64_t length = cmd->bufflen;
 	uint8_t *address = (uint8_t *)(unsigned long)cmd->pbuf;
 	int compare;
 	int fd = vcmd->fd;
@@ -1655,16 +1655,17 @@ static void exec_verify(struct vdisk_cmd *vcmd, loff_t loff)
 		compare = 1;
 
 	while (length > 0) {
-		int len_mem = (length > (int)sizeof(mem_verify)) ?
+		int64_t len_mem = (length > (int)sizeof(mem_verify)) ?
 					(int)sizeof(mem_verify) : length;
-		TRACE_DBG("Verify: length %d - len_mem %d", length, len_mem);
+		TRACE_DBG("Verify: length %"PRId64" - len_mem %"PRId64,
+			length, len_mem);
 
 		if (!dev->nullio)
 			err = read(fd, (char *)mem_verify, len_mem);
 		else
 			err = len_mem;
 		if ((err < 0) || (err < len_mem)) {
-			PRINT_ERROR("read() returned %"PRId64" from %d "
+			PRINT_ERROR("read() returned %"PRId64" from %"PRId64" "
 				"(errno %d)", (uint64_t)err, len_mem, errno);
 			if (err == -EAGAIN)
 				set_busy(reply);
@@ -1675,7 +1676,7 @@ static void exec_verify(struct vdisk_cmd *vcmd, loff_t loff)
 			goto out;
 		}
 		if (compare && memcmp(address, mem_verify, len_mem) != 0) {
-			TRACE_DBG("Verify: error memcmp length %d", length);
+			TRACE_DBG("Verify: error memcmp length %"PRId64, length);
 			set_cmd_error(vcmd,
 			    SCST_LOAD_SENSE(scst_sense_miscompare_error));
 			goto out;
@@ -1685,7 +1686,7 @@ static void exec_verify(struct vdisk_cmd *vcmd, loff_t loff)
 	}
 
 	if (length < 0) {
-		PRINT_ERROR("Failure: %d", length);
+		PRINT_ERROR("Failure: %"PRId64, length);
 		set_cmd_error(vcmd, SCST_LOAD_SENSE(scst_sense_hardw_error));
 	}