From 8d4d13fdb1f4af6831ed2b3f6531922a233e5901 Mon Sep 17 00:00:00 2001 From: Vladislav Bolkhovitin Date: Fri, 25 Jun 2010 17:05:36 +0000 Subject: [PATCH] Fix cdb_len handling to avoid crashes on attempts to copy negative cdb_len bytes + minor scst_user residual fix git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@1788 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- doc/scst_user_spec.txt | 8 ++++---- scst/include/scst.h | 8 ++++---- scst/include/scst_user.h | 8 ++++---- scst/src/dev_handlers/scst_user.c | 10 +++++++--- scst/src/scst_lib.c | 7 +------ scst/src/scst_priv.h | 2 -- scst/src/scst_targ.c | 8 +++----- usr/fileio/common.c | 1 + 8 files changed, 24 insertions(+), 28 deletions(-) diff --git a/doc/scst_user_spec.txt b/doc/scst_user_spec.txt index 333b265bb..5c67dc0cc 100644 --- a/doc/scst_user_spec.txt +++ b/doc/scst_user_spec.txt @@ -368,7 +368,7 @@ struct scst_user_scsi_cmd_parse uint64_t sess_h; uint8_t cdb[SCST_MAX_CDB_SIZE]; - int16_t cdb_len; + uint16_t cdb_len; uint16_t ext_cdb_len; uint32_t timeout; @@ -483,7 +483,7 @@ struct scst_user_scsi_cmd_alloc_mem uint64_t sess_h; uint8_t cdb[SCST_MAX_CDB_SIZE]; - int16_t cdb_len; + uint16_t cdb_len; uint16_t ext_cdb_len; int32_t alloc_len; @@ -586,7 +586,7 @@ struct scst_user_scsi_cmd_exec uint64_t sess_h; uint8_t cdb[SCST_MAX_CDB_SIZE]; - int16_t cdb_len; + uint16_t cdb_len; uint16_t ext_cdb_len; int32_t data_len; @@ -843,7 +843,7 @@ struct scst_user_scsi_cmd_reply_parse { uint8_t queue_type; uint8_t data_direction; - int16_t cdb_len; + uint16_t cdb_len; uint32_t op_flags; int32_t data_len; int32_t bufflen; diff --git a/scst/include/scst.h b/scst/include/scst.h index f430d7611..cd74191fd 100644 --- a/scst/include/scst.h +++ b/scst/include/scst.h @@ -1733,7 +1733,7 @@ struct scst_cmd { /* CDB and its len */ uint8_t cdb[SCST_MAX_CDB_SIZE]; - short cdb_len; /* it might be -1 */ + unsigned short cdb_len; unsigned short ext_cdb_len; uint8_t *ext_cdb; @@ -2469,7 +2469,7 @@ bool scst_initiator_has_luns(struct scst_tgt *tgt, const char *initiator_name); struct scst_cmd *scst_rx_cmd(struct scst_session *sess, const uint8_t *lun, int lun_len, const uint8_t *cdb, - int cdb_len, int atomic); + unsigned int cdb_len, int atomic); void scst_cmd_init_done(struct scst_cmd *cmd, enum scst_exec_context pref_context); @@ -2700,7 +2700,7 @@ static inline const uint8_t *scst_cmd_get_cdb(struct scst_cmd *cmd) } /* Returns cmd's CDB length */ -static inline int scst_cmd_get_cdb_len(struct scst_cmd *cmd) +static inline unsigned int scst_cmd_get_cdb_len(struct scst_cmd *cmd) { return cmd->cdb_len; } @@ -2712,7 +2712,7 @@ static inline const uint8_t *scst_cmd_get_ext_cdb(struct scst_cmd *cmd) } /* Returns cmd's extended CDB length */ -static inline int scst_cmd_get_ext_cdb_len(struct scst_cmd *cmd) +static inline unsigned int scst_cmd_get_ext_cdb_len(struct scst_cmd *cmd) { return cmd->ext_cdb_len; } diff --git a/scst/include/scst_user.h b/scst/include/scst_user.h index 8f725b656..11c4edba3 100644 --- a/scst/include/scst_user.h +++ b/scst/include/scst_user.h @@ -114,7 +114,7 @@ struct scst_user_scsi_cmd_parse { aligned_u64 sess_h; uint8_t cdb[SCST_MAX_CDB_SIZE]; - int16_t cdb_len; + uint16_t cdb_len; uint16_t ext_cdb_len; int32_t timeout; @@ -138,7 +138,7 @@ struct scst_user_scsi_cmd_alloc_mem { aligned_u64 sess_h; uint8_t cdb[SCST_MAX_CDB_SIZE]; - int16_t cdb_len; + uint16_t cdb_len; uint16_t ext_cdb_len; int32_t alloc_len; @@ -153,7 +153,7 @@ struct scst_user_scsi_cmd_exec { aligned_u64 sess_h; uint8_t cdb[SCST_MAX_CDB_SIZE]; - int16_t cdb_len; + uint16_t cdb_len; uint16_t ext_cdb_len; int32_t data_len; @@ -218,7 +218,7 @@ struct scst_user_scsi_cmd_reply_parse { struct { uint8_t queue_type; uint8_t data_direction; - int16_t cdb_len; + uint16_t cdb_len; uint32_t op_flags; int32_t data_len; int32_t bufflen; diff --git a/scst/src/dev_handlers/scst_user.c b/scst/src/dev_handlers/scst_user.c index d79d859b9..91b52b8c3 100644 --- a/scst/src/dev_handlers/scst_user.c +++ b/scst/src/dev_handlers/scst_user.c @@ -833,7 +833,6 @@ static int dev_user_parse(struct scst_cmd *cmd) ucmd->user_cmd.parse_cmd.expected_out_transfer_len = cmd->expected_out_transfer_len; ucmd->user_cmd.parse_cmd.sn = cmd->tgt_sn; - ucmd->user_cmd.parse_cmd.cdb_len = cmd->cdb_len; ucmd->user_cmd.parse_cmd.op_flags = cmd->op_flags; ucmd->state = UCMD_STATE_PARSING; dev_user_add_to_ready(ucmd); @@ -1452,9 +1451,14 @@ static int dev_user_process_reply_exec(struct scst_user_cmd *ucmd, cmd->may_need_dma_sync = 1; scst_set_resp_data_len(cmd, ereply->resp_data_len); } else if (cmd->resp_data_len != ereply->resp_data_len) { - if (ucmd->ubuff == 0) + if (ucmd->ubuff == 0) { + /* + * We have an empty SG, so can't call + * scst_set_resp_data_len() + */ cmd->resp_data_len = ereply->resp_data_len; - else + cmd->resid_possible = 1; + } else scst_set_resp_data_len(cmd, ereply->resp_data_len); } diff --git a/scst/src/scst_lib.c b/scst/src/scst_lib.c index afcfbdf92..e87dafb3e 100644 --- a/scst/src/scst_lib.c +++ b/scst/src/scst_lib.c @@ -4923,16 +4923,11 @@ out: return; } -static const int SCST_CDB_LENGTH[8] = { 6, 10, 10, -1, 16, 12, -1, -1 }; +static const int SCST_CDB_LENGTH[8] = { 6, 10, 10, 0, 16, 12, 0, 0 }; #define SCST_CDB_GROUP(opcode) ((opcode >> 5) & 0x7) #define SCST_GET_CDB_LEN(opcode) SCST_CDB_LENGTH[SCST_CDB_GROUP(opcode)] -int scst_get_cdb_len(const uint8_t *cdb) -{ - return SCST_GET_CDB_LEN(cdb[0]); -} - /* get_trans_len_x extract x bytes from cdb as length starting from off */ static int get_trans_cdb_len_10(struct scst_cmd *cmd, uint8_t off) diff --git a/scst/src/scst_priv.h b/scst/src/scst_priv.h index 0cce2f334..cbd49b287 100644 --- a/scst/src/scst_priv.h +++ b/scst/src/scst_priv.h @@ -555,8 +555,6 @@ void scst_acg_sysfs_put(struct scst_acg *acg); #endif /* CONFIG_SCST_PROC */ -int scst_get_cdb_len(const uint8_t *cdb); - void __scst_dev_check_set_UA(struct scst_device *dev, struct scst_cmd *exclude, const uint8_t *sense, int sense_len); static inline void scst_dev_check_set_UA(struct scst_device *dev, diff --git a/scst/src/scst_targ.c b/scst/src/scst_targ.c index fe15462e1..f394fdefc 100644 --- a/scst/src/scst_targ.c +++ b/scst/src/scst_targ.c @@ -108,8 +108,8 @@ static inline void scst_schedule_tasklet(struct scst_cmd *cmd) * same session. */ struct scst_cmd *scst_rx_cmd(struct scst_session *sess, - const uint8_t *lun, int lun_len, - const uint8_t *cdb, int cdb_len, int atomic) + const uint8_t *lun, int lun_len, const uint8_t *cdb, + unsigned int cdb_len, int atomic) { struct scst_cmd *cmd; @@ -431,8 +431,6 @@ static int scst_pre_parse(struct scst_cmd *cmd) EXTRACHECKS_BUG_ON(cmd->op_flags & SCST_INFO_VALID); - cmd->cdb_len = scst_get_cdb_len(cmd->cdb); - TRACE(TRACE_MINOR, "Unknown opcode 0x%02x for %s. " "Should you update scst_scsi_op_table?", cmd->cdb[0], dev->handler->name); @@ -573,7 +571,7 @@ static int scst_parse_cmd(struct scst_cmd *cmd) #endif } - if (unlikely(cmd->cdb_len == -1)) { + if (unlikely(cmd->cdb_len == 0)) { PRINT_ERROR("Unable to get CDB length for " "opcode 0x%02x. Returning INVALID " "OPCODE", cmd->cdb[0]); diff --git a/usr/fileio/common.c b/usr/fileio/common.c index 51c5db5ab..787f71eb7 100644 --- a/usr/fileio/common.c +++ b/usr/fileio/common.c @@ -778,6 +778,7 @@ void *main_loop(void *arg) PRINT_ERROR("SCST_USER_REPLY_AND_GET_CMD failed: " "%s (%d)", strerror(res), res); #if 1 + cmd.preply = 0; continue; #else goto out_close;