1. 18 9月, 2012 6 次提交
  2. 08 9月, 2012 6 次提交
    • P
      target: go through normal processing for zero-length REQUEST_SENSE · 6abbdf38
      Paolo Bonzini 提交于
      Now that spc_emulate_request_sense has been taught to process zero-length
      REQUEST SENSE correctly, drop the special handling of unit attention
      conditions from transport_generic_new_cmd.  However, for now REQUEST SENSE
      will be the only command that goes through emulation for zero lengths.
      
      (nab: Fix up zero-length check in transport_generic_new_cmd)
      Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
      Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>
      6abbdf38
    • P
      target: support zero allocation length in REQUEST SENSE · 32a8811f
      Paolo Bonzini 提交于
      Similar to INQUIRY and MODE SENSE, construct the sense data in a
      buffer and later copy it to the scatterlist.  Do not do anything,
      but still clear a pending unit attention condition, if the allocation
      length is zero.
      
      However, SPC tells us that "If a REQUEST SENSE command is terminated with
      CHECK CONDITION status [and] the REQUEST SENSE command was received on
      an I_T nexus with a pending unit attention condition (i.e., before the
      device server reports CHECK CONDITION status), then the device server
      shall not clear the pending unit attention condition."  Do the
      transport_kmap_data_sg early to detect this case.
      
      It also tells us "Device servers shall not adjust the additional sense
      length to reflect truncation if the allocation length is less than the
      sense data available", so do not do that!  Note that the err variable
      is write-only.
      Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
      Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>
      32a8811f
    • P
      target: support zero-size allocation lengths in transport_kmap_data_sg · 3717ef0c
      Paolo Bonzini 提交于
      In order to support zero-size allocation lengths, do not assert
      that we have a scatterlist until after checking cmd->data_length.
      
      But once we do this, we can have two cases of transport_kmap_data_sg
      returning NULL: a zero-size allocation length, or an out-of-memory
      condition.  Report the latter using sense codes, so that the SCSI
      command that triggered it will fail.
      Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
      Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>
      3717ef0c
    • P
      target: fail REPORT LUNS with less than 16 bytes of payload · 9b16b9ed
      Paolo Bonzini 提交于
      SPC says:
      
      "The ALLOCATION LENGTH field is defined in 4.3.5.6. The allocation length
      should be at least 16.  Device servers compliant with SPC return CHECK
      CONDITION status, with the sense key set to ILLEGAL REQUEST, and the
      additional sense code set to INVALID FIELD IN CDB when the allocation
      length is less than 16 bytes".
      
      Testcase: sg_raw -r8 /dev/sdb a0 00 00 00 00 00 00 00 00 08 00 00
          should fail with ILLEGAL REQUEST / INVALID FIELD IN CDB sense
          does not fail without the patch
          fails correctly with the patch
      Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
      Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>
      9b16b9ed
    • P
      target: report too-small parameter lists everywhere · 0d7f1299
      Paolo Bonzini 提交于
      Several places were not checking that the parameter list length
      was large enough, and thus accessing invalid memory.  Zero-length
      parameter lists are just a special case of this.
      Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
      Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>
      0d7f1299
    • P
      target: go through normal processing for zero-length PSCSI commands · 306c11b2
      Paolo Bonzini 提交于
      Right now, commands with a zero-size payload are skipped completely.
      This is wrong; such commands should be passed down to the device and
      processed normally.
      
      For physical backends, this ignores completely things such as START
      STOP UNIT.  For virtual backends, we have a hack in place to clear a
      unit attention state on a zero-size REQUEST SENSE, but we still do
      not report errors properly on zero-length commands---out-of-bounds
      0-block reads and writes, too small parameter list lengths, etc.
      
      This patch fixes this for PSCSI.  Uses of transport_kmap_data_sg are
      guarded with a check for non-zero cmd->data_length; for all other
      commands a zero length is handled properly in pscsi_execute_cmd.
      The sole exception will be for now REPORT LUNS, which is handled
      through the normal SPC emulation.
      Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
      Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>
      306c11b2
  3. 06 9月, 2012 3 次提交
  4. 27 8月, 2012 1 次提交
  5. 24 8月, 2012 1 次提交
    • N
      target: Fix ->data_length re-assignment bug with SCSI overflow · 4c054ba6
      Nicholas Bellinger 提交于
      This patch fixes a long-standing bug with SCSI overflow handling
      where se_cmd->data_length was incorrectly being re-assigned to
      the larger CDB extracted allocation length, resulting in a number
      of fabric level errors that would end up causing a session reset
      in most cases.  So instead now:
      
       - Only re-assign se_cmd->data_length durining UNDERFLOW (to use the
         smaller value)
       - Use existing se_cmd->data_length for OVERFLOW (to use the smaller
         value)
      
      This fix has been tested with the following CDB to generate an
      SCSI overflow:
      
        sg_raw -r512 /dev/sdc 28 0 0 0 0 0 0 0 9 0
      
      Tested using iscsi-target, tcm_qla2xxx, loopback and tcm_vhost fabric
      ports.  Here is a bit more detail on each case:
      
       - iscsi-target: Bug with open-iscsi with overflow, sg_raw returns
                       -3584 bytes of data.
       - tcm_qla2xxx: Working as expected, returnins 512 bytes of data
       - loopback: sg_raw returns CHECK_CONDITION, from overflow rejection
                   in transport_generic_map_mem_to_cmd()
       - tcm_vhost: Same as loopback
      Reported-by: NRoland Dreier <roland@purestorage.com>
      Cc: Roland Dreier <roland@purestorage.com>
      Cc: Christoph Hellwig <hch@lst.de>
      Cc: Boaz Harrosh <bharrosh@panasas.com>
      Cc: <stable@vger.kernel.org>
      Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>
      4c054ba6
  6. 22 8月, 2012 1 次提交
  7. 21 8月, 2012 1 次提交
  8. 18 8月, 2012 1 次提交
    • N
      target: Fix regression bug with handling of zero-length data CDBs · 74f4cf29
      Nicholas Bellinger 提交于
      This patch fixes a regression bug with the handling of zero-length
      data CDBs within transport_generic_new_cmd() code.  The bug was introduced
      with the following commit as part of the single task conversion work:
      
        commit 4101f0a8
        Author: Christoph Hellwig <hch@infradead.org>
        Date:   Tue Apr 24 00:25:03 2012 -0400
      
            target: always allocate a single task
      
      where the zero-length check for SCF_SCSI_DATA_SG_IO_CDB was incorrectly
      changed to SCF_SCSI_CONTROL_SG_IO_CDB because of the seperate comment
      in transport_generic_new_cmd() wrt to control CDBs zero-length handling
      introduced in:
      
        commit 91ec1d35
        Author: Nicholas Bellinger <nab@linux-iscsi.org>
        Date:   Fri Jan 13 12:01:34 2012 -0800
      
            target: Add workaround for zero-length control CDB handling
      
      So go ahead and change transport_generic_new_cmd() to handle control+data
      zero-length CDBs in the same manner for this special case.
      
      Tested with iscsi-target + loopback fabric port LUNs on 3.6-rc0 code.
      
      This patch will also need to be picked up for 3.5-stable.
      
      (hch: Add proper comment in transport_generic_new_cmd)
      
      Cc: Christoph Hellwig <hch@lst.de>
      Cc: Roland Dreier <roland@purestorage.com>
      Cc: Andy Grover <agrover@redhat.com>
      Cc: stable@vger.kernel.org
      Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>
      74f4cf29
  9. 17 8月, 2012 3 次提交
  10. 01 8月, 2012 1 次提交
    • A
      delousing target_core_file a bit · dbc6e022
      Al Viro 提交于
      * set_fs(KERNEL_DS) + getname() is probably the weirdest implementation
      of strdup() I've seen.  Especially since they don't to copy it at all...
      * filp_open() never returns NULL; it's ERR_PTR(-E...) on failure.
      * file->f_dentry is never going to be NULL, TYVM.
      * match_strdup() + snprintf() + kfree() is a bloody weird way to spell
      match_strlcpy().
      
      Pox on cargo-cult programmers...
      Signed-off-by: NAl Viro <viro@zeniv.linux.org.uk>
      dbc6e022
  11. 21 7月, 2012 2 次提交
    • A
      iscsi-target: Drop bogus struct file usage for iSCSI/SCTP · bf6932f4
      Al Viro 提交于
      From Al Viro:
      
      	BTW, speaking of struct file treatment related to sockets -
              there's this piece of code in iscsi:
              /*
               * The SCTP stack needs struct socket->file.
               */
              if ((np->np_network_transport == ISCSI_SCTP_TCP) ||
                  (np->np_network_transport == ISCSI_SCTP_UDP)) {
                      if (!new_sock->file) {
                              new_sock->file = kzalloc(
                                              sizeof(struct file), GFP_KERNEL);
      
      For one thing, as far as I can see it'not true - sctp does *not* depend on
      socket->file being non-NULL; it does, in one place, check socket->file->f_flags
      for O_NONBLOCK, but there it treats NULL socket->file as "flag not set".
      Which is the case here anyway - the fake struct file created in
      __iscsi_target_login_thread() (and in iscsi_target_setup_login_socket(), with
      the same excuse) do *not* get that flag set.
      
      Moreover, it's a bloody serious violation of a bunch of asserts in VFS;
      all struct file instances should come from filp_cachep, via get_empty_filp()
      (or alloc_file(), which is a wrapper for it).  FWIW, I'm very tempted to
      do this and be done with the entire mess:
      Signed-off-by: NAl Viro <viro@zeniv.linux.org.uk>
      Cc: Andy Grover <agrover@redhat.com>
      Cc: Hannes Reinecke <hare@suse.de>
      Cc: Christoph Hellwig <hch@lst.de>
      Cc: stable@vger.kernel.org
      Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>
      bf6932f4
    • D
      target: NULL dereference on error path · 2962846d
      Dan Carpenter 提交于
      During a failure in transport_add_device_to_core_hba() code, we called
      destroy_workqueue(dev->tmr_wq) before ->tmr_wq was allocated which leads
      to an oops.
      
      This fixes a regression introduced in with:
      
      commit af877292
      Author: Christoph Hellwig <hch@infradead.org>
      Date:   Sun Jul 8 15:58:49 2012 -0400
      
          target: replace the processing thread with a TMR work queue
      Signed-off-by: NDan Carpenter <dan.carpenter@oracle.com>
      Cc: Christoph Hellwig <hch@lst.de>
      Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>
      2962846d
  12. 18 7月, 2012 1 次提交
    • R
      target: Allow for target_submit_cmd() returning errors · d6dfc868
      Roland Dreier 提交于
      We want it to be possible for target_submit_cmd() to return errors up
      to its fabric module callers.  For now just update the prototype to
      return an int, and update all callers to handle non-zero return values
      as an error.
      
      This is immediately useful for tcm_qla2xxx to fix a long-standing active
      I/O session shutdown race, but tcm_fc, usb-gadget, and sbp-target the
      fabric maintainers need to check + ACK that handling a target_submit_cmd()
      failure due to session shutdown does not introduce regressions
      
      (nab: Respin against for-next after initial NACK + update docbook comment +
            fix double se_cmd init in exception path for usb-gadget)
      
      Cc: Chad Dupuis <chad.dupuis@qlogic.com>
      Cc: Arun Easi <arun.easi@qlogic.com>
      Cc: Chris Boot <bootc@bootc.net>
      Cc: Stefan Richter <stefanr@s5r6.in-berlin.de>
      Cc: Mark Rustad <mark.d.rustad@intel.com>
      Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
      Cc: Felipe Balbi <balbi@ti.com>
      Cc: Andy Grover <agrover@redhat.com>
      Signed-off-by: NRoland Dreier <roland@purestorage.com>
      Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>
      d6dfc868
  13. 17 7月, 2012 13 次提交
    • R
      target: Check number of unmap descriptors against our limit · 7409a665
      Roland Dreier 提交于
      Fail UNMAP commands that have more than our reported limit on unmap
      descriptors.
      Signed-off-by: NRoland Dreier <roland@purestorage.com>
      Cc: stable@vger.kernel.org
      Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>
      7409a665
    • R
      target: Fix possible integer underflow in UNMAP emulation · b7fc7f37
      Roland Dreier 提交于
      It's possible for an initiator to send us an UNMAP command with a
      descriptor that is less than 8 bytes; in that case it's really bad for
      us to set an unsigned int to that value, subtract 8 from it, and then
      use that as a limit for our loop (since the value will wrap around to
      a huge positive value).
      
      Fix this by making size be signed and only looping if size >= 16 (ie
      if we have at least a full descriptor available).
      
      Also remove offset as an obfuscated name for the constant 8.
      Signed-off-by: NRoland Dreier <roland@purestorage.com>
      Cc: stable@vger.kernel.org
      Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>
      b7fc7f37
    • R
      target: Fix reading of data length fields for UNMAP commands · 1a5fa457
      Roland Dreier 提交于
      The UNMAP DATA LENGTH and UNMAP BLOCK DESCRIPTOR DATA LENGTH fields
      are in the unmap descriptor (the payload transferred to our data out
      buffer), not in the CDB itself.  Read them from the correct place in
      target_emulated_unmap.
      Signed-off-by: NRoland Dreier <roland@purestorage.com>
      Cc: stable@vger.kernel.org
      Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>
      1a5fa457
    • R
      target: Add range checking to UNMAP emulation · 2594e298
      Roland Dreier 提交于
      When processing an UNMAP command, we need to make sure that the number
      of blocks we're asked to UNMAP does not exceed our reported maximum
      number of blocks per UNMAP, and that the range of blocks we're
      unmapping doesn't go past the end of the device.
      Signed-off-by: NRoland Dreier <roland@purestorage.com>
      Cc: stable@vger.kernel.org
      Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>
      2594e298
    • R
      target: Add generation of LOGICAL BLOCK ADDRESS OUT OF RANGE · e2397c70
      Roland Dreier 提交于
      Many SCSI commands are defined to return a CHECK CONDITION / ILLEGAL
      REQUEST with ASC set to LOGICAL BLOCK ADDRESS OUT OF RANGE if the
      initiator sends a command that accesses a too-big LBA.  Add an enum
      value and case entries so that target code can return this status.
      Signed-off-by: NRoland Dreier <roland@purestorage.com>
      Cc: stable@vger.kernel.org
      Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>
      e2397c70
    • R
    • R
      target: Remove se_session.sess_wait_list · 1c7b13fe
      Roland Dreier 提交于
      Since we set se_session.sess_tearing_down and stop new commands from
      being added to se_session.sess_cmd_list before we wait for commands to
      finish when freeing a session, there's no need for a separate
      sess_wait_list -- if we let new commands be added to sess_cmd_list
      after setting sess_tearing_down, that would be a bug that breaks the
      logic of waiting in-flight commands.
      
      Also rename target_splice_sess_cmd_list() to
      target_sess_cmd_list_set_waiting(), since we are no longer splicing
      onto a separate list.
      Signed-off-by: NRoland Dreier <roland@purestorage.com>
      Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>
      1c7b13fe
    • R
      target: Check sess_tearing_down in target_get_sess_cmd() · bc187ea6
      Roland Dreier 提交于
      Target core code assumes that target_splice_sess_cmd_list() has set
      sess_tearing_down and moved the list of pending commands to
      sess_wait_list, no more commands will be added to the session; if any
      are added, nothing keeps the se_session from being freed while the
      command is still in flight, which e.g. leads to use-after-free of
      se_cmd->se_sess in target_release_cmd_kref().
      
      To enforce this invariant, put a check of sess_tearing_down inside of
      sess_cmd_lock in target_get_sess_cmd(); any checks before this are
      racy and can lead to the use-after-free described above.  For example,
      the qla_target check in qlt_do_work() checks sess_tearing_down from
      work thread context but then drops all locks before calling
      target_submit_cmd() (as it must, since that is a sleeping function).
      
      However, since no locks are held, anything can happen with respect to
      the session it has looked up -- although it does correctly get
      sess_kref within its lock, so the memory won't be freed while
      target_submit_cmd() is actually running, nothing stops eg an ACL from
      being dropped and calling ->shutdown_session() (which calls into
      target_splice_sess_cmd_list()) before we get to target_get_sess_cmd().
      Once this happens, the se_session memory can be freed as soon as
      target_submit_cmd() returns and qlt_do_work() drops its reference,
      even though we've just added a command to sess_cmd_list.
      
      To prevent this use-after-free, check sess_tearing_down inside of
      sess_cmd_lock right before target_get_sess_cmd() adds a command to
      sess_cmd_list; this is synchronized with target_splice_sess_cmd_list()
      so that every command is either waited for or not added to the queue.
      
      (nab: Keep target_submit_cmd() returning void for now..)
      Signed-off-by: NRoland Dreier <roland@purestorage.com>
      Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>
      bc187ea6
    • R
      sbp-target: Consolidate duplicated error path code in sbp_handle_command() · 7c78b8de
      Roland Dreier 提交于
      Cc: Chris Boot <bootc@bootc.net>
      Cc: Stefan Richter <stefanr@s5r6.in-berlin.de>
      Signed-off-by: NRoland Dreier <roland@purestorage.com>
      7c78b8de
    • R
      target: Un-export target_get_sess_cmd() · 669ab62c
      Roland Dreier 提交于
      There are no in-tree users of target_get_sess_cmd() outside of
      target_core_transport.c.  Any new code should use the higher-level
      target_submit_cmd() interface.  So let's un-export target_get_sess_cmd()
      and make it static to the one file where it's actually used.
      
      (nab: Fix up minor fuzz to for-next)
      Signed-off-by: NRoland Dreier <roland@purestorage.com>
      Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>
      669ab62c
    • N
      target: Make core_disable_device_list_for_node use pre-refactoring lock ordering · 77d4c745
      Nicholas Bellinger 提交于
      So after kicking around commit 547ac4c9c90 around a bit more, a tcm_qla2xxx LUN
      unlink OP has generated the following warning:
      
      [   50.386625] qla2xxx [0000:07:00.0]-00af:0: Performing ISP error recovery - ha=ffff880263774000.
      [   70.572988] qla2xxx [0000:07:00.0]-8038:0: Cable is unplugged...
      [  126.527531] ------------[ cut here ]------------
      [  126.532677] WARNING: at kernel/softirq.c:159 local_bh_enable_ip+0x41/0x8c()
      [  126.540433] Hardware name: S5520HC
      [  126.544248] Modules linked in: tcm_vhost ib_srpt ib_cm ib_sa ib_mad ib_core tcm_qla2xxx tcm_loop tcm_fc libfc iscsi_target_mod target_core_pscsi target_core_file target_core_iblock target_core_mod configfs ipv6 iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi dm_round_robin dm_multipath scsi_dh loop i2c_i801 kvm_intel kvm crc32c_intel i2c_core microcode joydev button iomemory_vsl(O) pcspkr ext3 jbd uhci_hcd lpfc ata_piix libata ehci_hcd qla2xxx mlx4_core scsi_transport_fc scsi_tgt igb [last unloaded: scsi_wait_scan]
      [  126.595567] Pid: 3283, comm: unlink Tainted: G           O 3.5.0-rc2+ #33
      [  126.603128] Call Trace:
      [  126.605853]  [<ffffffff81026b91>] ? warn_slowpath_common+0x78/0x8c
      [  126.612737]  [<ffffffff8102c342>] ? local_bh_enable_ip+0x41/0x8c
      [  126.619433]  [<ffffffffa03582a2>] ? core_disable_device_list_for_node+0x70/0xe3 [target_core_mod]
      [  126.629323]  [<ffffffffa035849f>] ? core_clear_lun_from_tpg+0x88/0xeb [target_core_mod]
      [  126.638244]  [<ffffffffa0362ec1>] ? core_tpg_post_dellun+0x17/0x48 [target_core_mod]
      [  126.646873]  [<ffffffffa03575ee>] ? core_dev_del_lun+0x26/0x8c [target_core_mod]
      [  126.655114]  [<ffffffff810bcbd1>] ? dput+0x27/0x154
      [  126.660549]  [<ffffffffa0359aa0>] ? target_fabric_port_unlink+0x3b/0x41 [target_core_mod]
      [  126.669661]  [<ffffffffa034a698>] ? configfs_unlink+0xfc/0x14a [configfs]
      [  126.677224]  [<ffffffff810b5979>] ? vfs_unlink+0x58/0xb7
      [  126.683141]  [<ffffffff810b6ef3>] ? do_unlinkat+0xbb/0x142
      [  126.689253]  [<ffffffff81330c75>] ? page_fault+0x25/0x30
      [  126.695170]  [<ffffffff81335df9>] ? system_call_fastpath+0x16/0x1b
      [  126.702053] ---[ end trace 2f8e5b0a9ec797ef ]---
      [  126.756336] qla2xxx [0000:07:00.0]-00af:0: Performing ISP error recovery - ha=ffff880263774000.
      [  146.942414] qla2xxx [0000:07:00.0]-8038:0: Cable is unplugged...
      
      So this warning triggered because device_list disable logic is now
      holding nacl->device_list_lock w/ spin_lock_irqsave before obtaining
      port->sep_alua_lock with only spin_lock_bh..
      
      The original disable logic obtains *deve ahead of dropping the entry
      from deve->alua_port_list and then obtains ->device_list_lock to do the
      remaining work.  Also, I'm pretty sure this particular warning is being
      generated by a demo-mode session in tcm_qla2xxx, and not by explicit
      NodeACL MappedLUNs.  The Initiator MappedLUNs are already protected by a
      seperate configfs symlink reference back se_lun->lun_group, and the
      demo-mode se_node_acl (and associated ->device_list[]) is released
      during se_portal_group->tpg_group shutdown.
      
      The following patch drops the extra functional change to disable logic
      in commit 547ac4c9c90
      
      Cc: Andy Grover <agrover@redhat.com>
      Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>
      77d4c745
    • A
      target: refactor core_update_device_list_for_node() · e80ac6c4
      Andy Grover 提交于
      Code was almost entirely divided based on value of bool param "enable".
      
      Split it into two functions.
      Signed-off-by: NAndy Grover <agrover@redhat.com>
      Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>
      e80ac6c4
    • A
      target: Eliminate else using boolean logic · cdf88a2f
      Andy Grover 提交于
      Signed-off-by: NAndy Grover <agrover@redhat.com>
      Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>
      cdf88a2f