Merge git://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending
authorLinus Torvalds <torvalds@linux-foundation.org>
Sun, 16 Sep 2012 20:00:36 +0000 (13:00 -0700)
committerLinus Torvalds <torvalds@linux-foundation.org>
Sun, 16 Sep 2012 20:00:36 +0000 (13:00 -0700)
Pull scsi target fixes from Nicholas Bellinger:
 "Here is the current set of target-pending fixes headed for v3.6-final

  The main parts of this series include bug-fixes from Paolo Bonzini to
  address an use-after-free bug in pSCSI sense exception handling, along
  with addressing some long-standing bugs wrt the handling of zero-
  length SCSI CDB payloads also specific to pSCSI pass-through device
  backends."

* git://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending:
  target: go through normal processing for zero-length REQUEST_SENSE
  target: support zero allocation length in REQUEST SENSE
  target: support zero-size allocation lengths in transport_kmap_data_sg
  target: fail REPORT LUNS with less than 16 bytes of payload
  target: report too-small parameter lists everywhere
  target: go through normal processing for zero-length PSCSI commands
  target: fix use-after-free with PSCSI sense data
  target: simplify code around transport_get_sense_data
  target: move transport_get_sense_data
  target: Check idr_get_new return value in iscsi_login_zero_tsih_s1
  target: Fix ->data_length re-assignment bug with SCSI overflow

drivers/target/iscsi/iscsi_target_login.c
drivers/target/target_core_alua.c
drivers/target/target_core_device.c
drivers/target/target_core_iblock.c
drivers/target/target_core_pr.c
drivers/target/target_core_pscsi.c
drivers/target/target_core_spc.c
drivers/target/target_core_transport.c
include/target/target_core_backend.h
include/target/target_core_base.h

index 0694d9b1bce6a4e066a7bba1b1ef5f870b4b51d7..6aba4395e8d8ffd5aa950c4dfc745791c3f65fc2 100644 (file)
@@ -221,6 +221,7 @@ static int iscsi_login_zero_tsih_s1(
 {
        struct iscsi_session *sess = NULL;
        struct iscsi_login_req *pdu = (struct iscsi_login_req *)buf;
+       int ret;
 
        sess = kzalloc(sizeof(struct iscsi_session), GFP_KERNEL);
        if (!sess) {
@@ -257,9 +258,17 @@ static int iscsi_login_zero_tsih_s1(
                return -ENOMEM;
        }
        spin_lock(&sess_idr_lock);
-       idr_get_new(&sess_idr, NULL, &sess->session_index);
+       ret = idr_get_new(&sess_idr, NULL, &sess->session_index);
        spin_unlock(&sess_idr_lock);
 
+       if (ret < 0) {
+               pr_err("idr_get_new() for sess_idr failed\n");
+               iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,
+                               ISCSI_LOGIN_STATUS_NO_RESOURCES);
+               kfree(sess);
+               return -ENOMEM;
+       }
+
        sess->creation_time = get_jiffies_64();
        spin_lock_init(&sess->session_stats_lock);
        /*
index 91799973081a3d907cd260792df3f573d1dbec82..41641ba548286e9dbf33f45a02ece5b20eec0d14 100644 (file)
@@ -218,6 +218,13 @@ int target_emulate_set_target_port_groups(struct se_cmd *cmd)
                cmd->scsi_sense_reason = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
                return -EINVAL;
        }
+       if (cmd->data_length < 4) {
+               pr_warn("SET TARGET PORT GROUPS parameter list length %u too"
+                       " small\n", cmd->data_length);
+               cmd->scsi_sense_reason = TCM_INVALID_PARAMETER_LIST;
+               return -EINVAL;
+       }
+
        buf = transport_kmap_data_sg(cmd);
 
        /*
index cf2c66f3c11690c81ca23e9fd2ce286e76b56e62..9fc9a6006ca082076a6d235dfad676e1e1a11ea9 100644 (file)
@@ -669,6 +669,13 @@ int target_report_luns(struct se_cmd *se_cmd)
        unsigned char *buf;
        u32 lun_count = 0, offset = 8, i;
 
+       if (se_cmd->data_length < 16) {
+               pr_warn("REPORT LUNS allocation length %u too small\n",
+                       se_cmd->data_length);
+               se_cmd->scsi_sense_reason = TCM_INVALID_CDB_FIELD;
+               return -EINVAL;
+       }
+
        buf = transport_kmap_data_sg(se_cmd);
        if (!buf)
                return -ENOMEM;
index 76db75e836ede701c2aed6090a212fdf1a08ad10..9ba495477fd24f80bf6643cfae3707525b7c165e 100644 (file)
@@ -325,17 +325,30 @@ static int iblock_execute_unmap(struct se_cmd *cmd)
        struct iblock_dev *ibd = dev->dev_ptr;
        unsigned char *buf, *ptr = NULL;
        sector_t lba;
-       int size = cmd->data_length;
+       int size;
        u32 range;
        int ret = 0;
        int dl, bd_dl;
 
+       if (cmd->data_length < 8) {
+               pr_warn("UNMAP parameter list length %u too small\n",
+                       cmd->data_length);
+               cmd->scsi_sense_reason = TCM_INVALID_PARAMETER_LIST;
+               return -EINVAL;
+       }
+
        buf = transport_kmap_data_sg(cmd);
 
        dl = get_unaligned_be16(&buf[0]);
        bd_dl = get_unaligned_be16(&buf[2]);
 
-       size = min(size - 8, bd_dl);
+       size = cmd->data_length - 8;
+       if (bd_dl > size)
+               pr_warn("UNMAP parameter list length %u too small, ignoring bd_dl %u\n",
+                       cmd->data_length, bd_dl);
+       else
+               size = bd_dl;
+
        if (size / 16 > dev->se_sub_dev->se_dev_attrib.max_unmap_block_desc_count) {
                cmd->scsi_sense_reason = TCM_INVALID_PARAMETER_LIST;
                ret = -EINVAL;
index 1e946502c378886aa90bafc16239d2c34730b9fb..956c84c6b666498caabf7b60b7404413698c0b14 100644 (file)
@@ -1540,6 +1540,14 @@ static int core_scsi3_decode_spec_i_port(
        tidh_new->dest_local_nexus = 1;
        list_add_tail(&tidh_new->dest_list, &tid_dest_list);
 
+       if (cmd->data_length < 28) {
+               pr_warn("SPC-PR: Received PR OUT parameter list"
+                       " length too small: %u\n", cmd->data_length);
+               cmd->scsi_sense_reason = TCM_INVALID_PARAMETER_LIST;
+               ret = -EINVAL;
+               goto out;
+       }
+
        buf = transport_kmap_data_sg(cmd);
        /*
         * For a PERSISTENT RESERVE OUT specify initiator ports payload,
index 5552fa7426bc9b317da906dec2e395bbf854ae78..9d7ce3daa26275a7c08b831ae9b955382e0e9103 100644 (file)
@@ -667,7 +667,8 @@ static void pscsi_free_device(void *p)
        kfree(pdv);
 }
 
-static int pscsi_transport_complete(struct se_cmd *cmd, struct scatterlist *sg)
+static void pscsi_transport_complete(struct se_cmd *cmd, struct scatterlist *sg,
+                                    unsigned char *sense_buffer)
 {
        struct pscsi_dev_virt *pdv = cmd->se_dev->dev_ptr;
        struct scsi_device *sd = pdv->pdv_sd;
@@ -679,7 +680,7 @@ static int pscsi_transport_complete(struct se_cmd *cmd, struct scatterlist *sg)
         * not been allocated because TCM is handling the emulation directly.
         */
        if (!pt)
-               return 0;
+               return;
 
        cdb = &pt->pscsi_cdb[0];
        result = pt->pscsi_result;
@@ -687,11 +688,11 @@ static int pscsi_transport_complete(struct se_cmd *cmd, struct scatterlist *sg)
         * Hack to make sure that Write-Protect modepage is set if R/O mode is
         * forced.
         */
+       if (!cmd->se_deve || !cmd->data_length)
+               goto after_mode_sense;
+
        if (((cdb[0] == MODE_SENSE) || (cdb[0] == MODE_SENSE_10)) &&
             (status_byte(result) << 1) == SAM_STAT_GOOD) {
-               if (!cmd->se_deve)
-                       goto after_mode_sense;
-
                if (cmd->se_deve->lun_flags & TRANSPORT_LUNFLAGS_READ_ONLY) {
                        unsigned char *buf = transport_kmap_data_sg(cmd);
 
@@ -708,7 +709,7 @@ static int pscsi_transport_complete(struct se_cmd *cmd, struct scatterlist *sg)
        }
 after_mode_sense:
 
-       if (sd->type != TYPE_TAPE)
+       if (sd->type != TYPE_TAPE || !cmd->data_length)
                goto after_mode_select;
 
        /*
@@ -750,10 +751,10 @@ after_mode_sense:
        }
 after_mode_select:
 
-       if (status_byte(result) & CHECK_CONDITION)
-               return 1;
-
-       return 0;
+       if (sense_buffer && (status_byte(result) & CHECK_CONDITION)) {
+               memcpy(sense_buffer, pt->pscsi_sense, TRANSPORT_SENSE_BUFFER);
+               cmd->se_cmd_flags |= SCF_TRANSPORT_TASK_SENSE;
+       }
 }
 
 enum {
@@ -1184,13 +1185,6 @@ fail:
        return -ENOMEM;
 }
 
-static unsigned char *pscsi_get_sense_buffer(struct se_cmd *cmd)
-{
-       struct pscsi_plugin_task *pt = cmd->priv;
-
-       return pt->pscsi_sense;
-}
-
 /*     pscsi_get_device_rev():
  *
  *
@@ -1273,7 +1267,6 @@ static struct se_subsystem_api pscsi_template = {
        .check_configfs_dev_params = pscsi_check_configfs_dev_params,
        .set_configfs_dev_params = pscsi_set_configfs_dev_params,
        .show_configfs_dev_params = pscsi_show_configfs_dev_params,
-       .get_sense_buffer       = pscsi_get_sense_buffer,
        .get_device_rev         = pscsi_get_device_rev,
        .get_device_type        = pscsi_get_device_type,
        .get_blocks             = pscsi_get_blocks,
index 4c861de538c9ddb627356e8aa3ea992b164033cf..388a922c8f6de8f4f2a18f7bea85f5b3590b41a9 100644 (file)
@@ -877,9 +877,11 @@ static int spc_emulate_modesense(struct se_cmd *cmd)
 static int spc_emulate_request_sense(struct se_cmd *cmd)
 {
        unsigned char *cdb = cmd->t_task_cdb;
-       unsigned char *buf;
+       unsigned char *rbuf;
        u8 ua_asc = 0, ua_ascq = 0;
-       int err = 0;
+       unsigned char buf[SE_SENSE_BUF];
+
+       memset(buf, 0, SE_SENSE_BUF);
 
        if (cdb[1] & 0x01) {
                pr_err("REQUEST_SENSE description emulation not"
@@ -888,20 +890,21 @@ static int spc_emulate_request_sense(struct se_cmd *cmd)
                return -ENOSYS;
        }
 
-       buf = transport_kmap_data_sg(cmd);
-
-       if (!core_scsi3_ua_clear_for_request_sense(cmd, &ua_asc, &ua_ascq)) {
+       rbuf = transport_kmap_data_sg(cmd);
+       if (cmd->scsi_sense_reason != 0) {
+               /*
+                * Out of memory.  We will fail with CHECK CONDITION, so
+                * we must not clear the unit attention condition.
+                */
+               target_complete_cmd(cmd, CHECK_CONDITION);
+               return 0;
+       } else if (!core_scsi3_ua_clear_for_request_sense(cmd, &ua_asc, &ua_ascq)) {
                /*
                 * CURRENT ERROR, UNIT ATTENTION
                 */
                buf[0] = 0x70;
                buf[SPC_SENSE_KEY_OFFSET] = UNIT_ATTENTION;
 
-               if (cmd->data_length < 18) {
-                       buf[7] = 0x00;
-                       err = -EINVAL;
-                       goto end;
-               }
                /*
                 * The Additional Sense Code (ASC) from the UNIT ATTENTION
                 */
@@ -915,11 +918,6 @@ static int spc_emulate_request_sense(struct se_cmd *cmd)
                buf[0] = 0x70;
                buf[SPC_SENSE_KEY_OFFSET] = NO_SENSE;
 
-               if (cmd->data_length < 18) {
-                       buf[7] = 0x00;
-                       err = -EINVAL;
-                       goto end;
-               }
                /*
                 * NO ADDITIONAL SENSE INFORMATION
                 */
@@ -927,8 +925,11 @@ static int spc_emulate_request_sense(struct se_cmd *cmd)
                buf[7] = 0x0A;
        }
 
-end:
-       transport_kunmap_data_sg(cmd);
+       if (rbuf) {
+               memcpy(rbuf, buf, min_t(u32, sizeof(buf), cmd->data_length));
+               transport_kunmap_data_sg(cmd);
+       }
+
        target_complete_cmd(cmd, GOOD);
        return 0;
 }
index 4de3186dc44e99672d3666a24ec145e058183691..269f54488397bd2193bb80869bf9de8ac4c73bf2 100644 (file)
@@ -567,6 +567,34 @@ static void target_complete_failure_work(struct work_struct *work)
        transport_generic_request_failure(cmd);
 }
 
+/*
+ * Used when asking transport to copy Sense Data from the underlying
+ * Linux/SCSI struct scsi_cmnd
+ */
+static unsigned char *transport_get_sense_buffer(struct se_cmd *cmd)
+{
+       unsigned char *buffer = cmd->sense_buffer;
+       struct se_device *dev = cmd->se_dev;
+       u32 offset = 0;
+
+       WARN_ON(!cmd->se_lun);
+
+       if (!dev)
+               return NULL;
+
+       if (cmd->se_cmd_flags & SCF_SENT_CHECK_CONDITION)
+               return NULL;
+
+       offset = cmd->se_tfo->set_fabric_sense_len(cmd, TRANSPORT_SENSE_BUFFER);
+
+       /* Automatically padded */
+       cmd->scsi_sense_length = TRANSPORT_SENSE_BUFFER + offset;
+
+       pr_debug("HBA_[%u]_PLUG[%s]: Requesting sense for SAM STATUS: 0x%02x\n",
+               dev->se_hba->hba_id, dev->transport->name, cmd->scsi_status);
+       return &buffer[offset];
+}
+
 void target_complete_cmd(struct se_cmd *cmd, u8 scsi_status)
 {
        struct se_device *dev = cmd->se_dev;
@@ -580,11 +608,11 @@ void target_complete_cmd(struct se_cmd *cmd, u8 scsi_status)
        cmd->transport_state &= ~CMD_T_BUSY;
 
        if (dev && dev->transport->transport_complete) {
-               if (dev->transport->transport_complete(cmd,
-                               cmd->t_data_sg) != 0) {
-                       cmd->se_cmd_flags |= SCF_TRANSPORT_TASK_SENSE;
+               dev->transport->transport_complete(cmd,
+                               cmd->t_data_sg,
+                               transport_get_sense_buffer(cmd));
+               if (cmd->se_cmd_flags & SCF_TRANSPORT_TASK_SENSE)
                        success = 1;
-               }
        }
 
        /*
@@ -1181,15 +1209,20 @@ int target_cmd_size_check(struct se_cmd *cmd, unsigned int size)
                        /* Returns CHECK_CONDITION + INVALID_CDB_FIELD */
                        goto out_invalid_cdb_field;
                }
-
+               /*
+                * For the overflow case keep the existing fabric provided
+                * ->data_length.  Otherwise for the underflow case, reset
+                * ->data_length to the smaller SCSI expected data transfer
+                * length.
+                */
                if (size > cmd->data_length) {
                        cmd->se_cmd_flags |= SCF_OVERFLOW_BIT;
                        cmd->residual_count = (size - cmd->data_length);
                } else {
                        cmd->se_cmd_flags |= SCF_UNDERFLOW_BIT;
                        cmd->residual_count = (cmd->data_length - size);
+                       cmd->data_length = size;
                }
-               cmd->data_length = size;
        }
 
        return 0;
@@ -1815,61 +1848,6 @@ execute:
 }
 EXPORT_SYMBOL(target_execute_cmd);
 
-/*
- * Used to obtain Sense Data from underlying Linux/SCSI struct scsi_cmnd
- */
-static int transport_get_sense_data(struct se_cmd *cmd)
-{
-       unsigned char *buffer = cmd->sense_buffer, *sense_buffer = NULL;
-       struct se_device *dev = cmd->se_dev;
-       unsigned long flags;
-       u32 offset = 0;
-
-       WARN_ON(!cmd->se_lun);
-
-       if (!dev)
-               return 0;
-
-       spin_lock_irqsave(&cmd->t_state_lock, flags);
-       if (cmd->se_cmd_flags & SCF_SENT_CHECK_CONDITION) {
-               spin_unlock_irqrestore(&cmd->t_state_lock, flags);
-               return 0;
-       }
-
-       if (!(cmd->se_cmd_flags & SCF_TRANSPORT_TASK_SENSE))
-               goto out;
-
-       if (!dev->transport->get_sense_buffer) {
-               pr_err("dev->transport->get_sense_buffer is NULL\n");
-               goto out;
-       }
-
-       sense_buffer = dev->transport->get_sense_buffer(cmd);
-       if (!sense_buffer) {
-               pr_err("ITT 0x%08x cmd %p: Unable to locate"
-                       " sense buffer for task with sense\n",
-                       cmd->se_tfo->get_task_tag(cmd), cmd);
-               goto out;
-       }
-
-       spin_unlock_irqrestore(&cmd->t_state_lock, flags);
-
-       offset = cmd->se_tfo->set_fabric_sense_len(cmd, TRANSPORT_SENSE_BUFFER);
-
-       memcpy(&buffer[offset], sense_buffer, TRANSPORT_SENSE_BUFFER);
-
-       /* Automatically padded */
-       cmd->scsi_sense_length = TRANSPORT_SENSE_BUFFER + offset;
-
-       pr_debug("HBA_[%u]_PLUG[%s]: Set SAM STATUS: 0x%02x and sense\n",
-               dev->se_hba->hba_id, dev->transport->name, cmd->scsi_status);
-       return 0;
-
-out:
-       spin_unlock_irqrestore(&cmd->t_state_lock, flags);
-       return -1;
-}
-
 /*
  * Process all commands up to the last received ORDERED task attribute which
  * requires another blocking boundary
@@ -1985,7 +1963,7 @@ static void transport_handle_queue_full(
 static void target_complete_ok_work(struct work_struct *work)
 {
        struct se_cmd *cmd = container_of(work, struct se_cmd, work);
-       int reason = 0, ret;
+       int ret;
 
        /*
         * Check if we need to move delayed/dormant tasks from cmds on the
@@ -2002,23 +1980,19 @@ static void target_complete_ok_work(struct work_struct *work)
                schedule_work(&cmd->se_dev->qf_work_queue);
 
        /*
-        * Check if we need to retrieve a sense buffer from
+        * Check if we need to send a sense buffer from
         * the struct se_cmd in question.
         */
        if (cmd->se_cmd_flags & SCF_TRANSPORT_TASK_SENSE) {
-               if (transport_get_sense_data(cmd) < 0)
-                       reason = TCM_NON_EXISTENT_LUN;
-
-               if (cmd->scsi_status) {
-                       ret = transport_send_check_condition_and_sense(
-                                       cmd, reason, 1);
-                       if (ret == -EAGAIN || ret == -ENOMEM)
-                               goto queue_full;
+               WARN_ON(!cmd->scsi_status);
+               ret = transport_send_check_condition_and_sense(
+                                       cmd, 0, 1);
+               if (ret == -EAGAIN || ret == -ENOMEM)
+                       goto queue_full;
 
-                       transport_lun_remove_cmd(cmd);
-                       transport_cmd_check_stop_to_fabric(cmd);
-                       return;
-               }
+               transport_lun_remove_cmd(cmd);
+               transport_cmd_check_stop_to_fabric(cmd);
+               return;
        }
        /*
         * Check for a callback, used by amongst other things
@@ -2216,7 +2190,6 @@ void *transport_kmap_data_sg(struct se_cmd *cmd)
        struct page **pages;
        int i;
 
-       BUG_ON(!sg);
        /*
         * We need to take into account a possible offset here for fabrics like
         * tcm_loop who may be using a contig buffer from the SCSI midlayer for
@@ -2224,13 +2197,17 @@ void *transport_kmap_data_sg(struct se_cmd *cmd)
         */
        if (!cmd->t_data_nents)
                return NULL;
-       else if (cmd->t_data_nents == 1)
+
+       BUG_ON(!sg);
+       if (cmd->t_data_nents == 1)
                return kmap(sg_page(sg)) + sg->offset;
 
        /* >1 page. use vmap */
        pages = kmalloc(sizeof(*pages) * cmd->t_data_nents, GFP_KERNEL);
-       if (!pages)
+       if (!pages) {
+               cmd->scsi_sense_reason = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
                return NULL;
+       }
 
        /* convert sg[] to pages[] */
        for_each_sg(cmd->t_data_sg, sg, cmd->t_data_nents, i) {
@@ -2239,8 +2216,10 @@ void *transport_kmap_data_sg(struct se_cmd *cmd)
 
        cmd->t_data_vmap = vmap(pages, cmd->t_data_nents,  VM_MAP, PAGE_KERNEL);
        kfree(pages);
-       if (!cmd->t_data_vmap)
+       if (!cmd->t_data_vmap) {
+               cmd->scsi_sense_reason = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
                return NULL;
+       }
 
        return cmd->t_data_vmap + cmd->t_data_sg[0].offset;
 }
@@ -2326,19 +2305,14 @@ int transport_generic_new_cmd(struct se_cmd *cmd)
         * into the fabric for data transfers, go ahead and complete it right
         * away.
         */
-       if (!cmd->data_length) {
+       if (!cmd->data_length &&
+           cmd->t_task_cdb[0] != REQUEST_SENSE &&
+           cmd->se_dev->transport->transport_type != TRANSPORT_PLUGIN_PHBA_PDEV) {
                spin_lock_irq(&cmd->t_state_lock);
                cmd->t_state = TRANSPORT_COMPLETE;
                cmd->transport_state |= CMD_T_ACTIVE;
                spin_unlock_irq(&cmd->t_state_lock);
 
-               if (cmd->t_task_cdb[0] == REQUEST_SENSE) {
-                       u8 ua_asc = 0, ua_ascq = 0;
-
-                       core_scsi3_ua_clear_for_request_sense(cmd,
-                                       &ua_asc, &ua_ascq);
-               }
-
                INIT_WORK(&cmd->work, target_complete_ok_work);
                queue_work(target_completion_wq, &cmd->work);
                return 0;
index f1405d335a968d093aadafee06ab4be105970f88..941c84bf1065f406cfbc3eeb5f4b09e4fdcd58ec 100644 (file)
@@ -23,7 +23,9 @@ struct se_subsystem_api {
        struct se_device *(*create_virtdevice)(struct se_hba *,
                                struct se_subsystem_dev *, void *);
        void (*free_device)(void *);
-       int (*transport_complete)(struct se_cmd *cmd, struct scatterlist *);
+       void (*transport_complete)(struct se_cmd *cmd,
+                                  struct scatterlist *,
+                                  unsigned char *);
 
        int (*parse_cdb)(struct se_cmd *cmd);
        ssize_t (*check_configfs_dev_params)(struct se_hba *,
index 015cea01ae39bedd1a1bf796e23a8f90f061e5e2..5be89373ceac659c92e671a463c80f146656c416 100644 (file)
 
 #define SE_INQUIRY_BUF                         512
 #define SE_MODE_PAGE_BUF                       512
+#define SE_SENSE_BUF                           96
 
 /* struct se_hba->hba_flags */
 enum hba_flags_table {