From 634c1e69aa23465b96fc96ffbcaf773495b5096d Mon Sep 17 00:00:00 2001 From: Vladislav Bolkhovitin Date: Sat, 8 Feb 2014 01:52:03 +0000 Subject: [PATCH] Fix spurious BUG when parse_type != SCST_USER_PARSE_STANDARD Changeset 4224 introduced EXTRACHECKS for valid lba/data_len and state at the end of the parsing phase of command processing. However, the checks do not account for deferral of parsing to userland, as occurs when SCST_USER_PARSE_CALL or SCST_USER_PARSE_EXCEPTION are specified. In such cases the checks report errors on commands that userland has not yet had an opportunity to parse. NOTE: this includes a refactoring of the EXTRACHECKS to improve clarity. The rework is not exactly equivalent to the original code, but does conform to the comments describing the original code. Specifically, the original code would not trap an illegal command state unless there was also an illegal lba or data_len. Signed-off-by: Steven J. Magnani with some improvements git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@5275 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- scst/src/scst_targ.c | 45 +++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/scst/src/scst_targ.c b/scst/src/scst_targ.c index 3c0d2afe9..cc0d5a1b0 100644 --- a/scst/src/scst_targ.c +++ b/scst/src/scst_targ.c @@ -941,23 +941,34 @@ set_res: out: #ifdef CONFIG_SCST_EXTRACHECKS - /* - * At this point either both lba and data_len must be initialized to - * at least 0 for not data transfer commands, or cmd must be - * completed (with an error) and have correct state set. - */ - if (unlikely((((cmd->lba == SCST_DEF_LBA_DATA_LEN) && - !(cmd->op_flags & SCST_LBA_NOT_VALID)) || - (cmd->data_len == SCST_DEF_LBA_DATA_LEN)) && - (!cmd->completed || - (((cmd->state < SCST_CMD_STATE_PRE_XMIT_RESP) || - (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 %lld, " - "completed %d, state %d)", cmd, - (long long)cmd->data_len, cmd->completed, cmd->state); - sBUG(); + if (unlikely(cmd->completed)) { + /* Command completed with error */ + bool valid_state = (cmd->state == SCST_CMD_STATE_PREPROCESSING_DONE) || + ((cmd->state >= SCST_CMD_STATE_PRE_XMIT_RESP) && + (cmd->state < SCST_CMD_STATE_LAST_ACTIVE)); + + if (!valid_state) { + PRINT_CRIT_ERROR("Bad state for completed cmd " + "(cmd %p, state %d)", cmd, cmd->state); + sBUG(); + } + } else if (cmd->state != SCST_CMD_STATE_PARSE) { + /* + * Ready to execute. At this point both lba and data_len must + * be initialized or marked non-applicable. + */ + bool bad_lba = (cmd->lba == SCST_DEF_LBA_DATA_LEN) && + !(cmd->op_flags & SCST_LBA_NOT_VALID); + bool bad_data_len = (cmd->data_len == SCST_DEF_LBA_DATA_LEN); + + if (unlikely(bad_lba || bad_data_len)) { + PRINT_CRIT_ERROR("Uninitialized lba or data_len for " + "ready-to-execute command (cmd %p, lba %lld, " + "data_len %lld, state %d)", cmd, + (long long)cmd->lba, (long long)cmd->data_len, + cmd->state); + sBUG(); + } } #endif