It is possible for one thread to to take se_sess->sess_cmd_lock in
core_tmr_abort_task() before taking a reference count on
se_cmd->cmd_kref, while another thread in target_put_sess_cmd() drops
se_cmd->cmd_kref before taking se_sess->sess_cmd_lock.

This introduces kref_put_spinlock_irqsave() and uses it in
target_put_sess_cmd() to close the race window.
Signed-off-by: NJoern Engel <joern@logfs.org>
Acked-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: <stable@vger.kernel.org>
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

Some were incremented, but never used anywhere from what I could tell.
Signed-off-by: NAndy Grover <agrover@redhat.com>
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

Export target_get_sess_cmd() symbol so that it can be used by
iscsi-target.
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

As the comment sais, this allows Solaris initiators to survive
intermittent errors.  The comment from someone reading the Solaris
sources seem to imply that multipathing would be broken without this
patch as well.
Signed-off-by: NJoern Engel <joern@logfs.org>
Cc: Brian Bunker <brian@purestorage.com>
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

This patch fixes a regression introduced in v3.8-rc1 code where a failed
target_check_reservation() check in target_setup_cmd_from_cdb() was causing
an incorrect SAM_STAT_GOOD status to be returned during a WRITE operation
performed by an unregistered / unreserved iscsi initiator port.

This regression is only effecting iscsi-target due to a special case check
for TCM_RESERVATION_CONFLICT within iscsi_target_erl1.c:iscsit_execute_cmd(),
and was still correctly disallowing WRITE commands from backend submission
for unregistered / unreserved initiator ports, while returning the incorrect
SAM_STAT_GOOD status due to the missing SAM_STAT_RESERVATION_CONFLICT
assignment.

This regression was first introduced with:

commit de103c93
Author: Christoph Hellwig <hch@lst.de>
Date:   Tue Nov 6 12:24:09 2012 -0800

    target: pass sense_reason as a return value

Go ahead and re-add the missing SAM_STAT_RESERVATION_CONFLICT assignment
during a target_check_reservation() failure, so that iscsi-target code
sends the correct SCSI status.

All other fabrics using target_submit_cmd_*() with a RESERVATION_CONFLICT
call to transport_generic_request_failure() are not effected by this bug.
Reported-by: NJeff Leung <jleung@curriegrad2004.ca>
Cc: Christoph Hellwig <hch@lst.de>
Cc: <stable@vger.kernel.org>
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

SBC-3 (revision 35) says:

    The PARAMETER LIST LENGTH field specifies the length in bytes of the
    UNMAP parameter list that is available to be transferred from the
    Data-Out Buffer. If the parameter list length is greater than zero
    and less than 0008h (i.e., eight), then the device server shall
    terminate the command with CHECK CONDITION status with the sense key
    set to ILLEGAL REQUEST and the additional sense code set to
    PARAMETER LIST LENGTH ERROR. A PARAMETER LIST LENGTH set to zero
    specifies that no data shall be sent.

so our sense code for too-short descriptors was wrong, and we were
incorrectly failing commands that didn't transfer any descriptors.

While we're at it, also handle the UNMAP check:

    If the ANCHOR bit is set to one, and the ANC_SUP bit in the Logical
    Block Provisioning VPD page (see 6.6.4) is set to zero, then the
    device server shall terminate the command with CHECK CONDITION
    status with the sense key set to ILLEGAL REQUEST and the additional
    sense code set to INVALID FIELD IN CDB.

(chris boot: Fix wrong cut+paste comment in transport_send_check_condition_and_sense)
Signed-off-by: NRoland Dreier <roland@purestorage.com>
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

"buf" is 128 characters and "vpd->device_identifier" is 256.  It makes
the static checkers complain.

Also bump VPD_TMP_BUF_SIZE to match INQUIRY_VPD_DEVICE_IDENTIFIER_LEN.
Signed-off-by: NDan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

When transport_lookup_tmr_lun() fails and we return a task management
response from target_complete_tmr_failure(), we need to call
transport_cmd_check_stop_to_fabric() to release the last ref to the
cmd after calling se_tfo->queue_tm_rsp(), or else we will never remove
the failed TMR from the session command list (and we'll end up waiting
forever when trying to tear down the session).

(nab: Fix minor compile breakage)
Signed-off-by: NRoland Dreier <roland@purestorage.com>
Cc: stable@vger.kernel.org
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

If a backend IO takes a really long then an initiator might abort a
command, and then when it gives up on the abort, send a LUN reset too,
all before we process any of the original command or the abort.  (The
abort will wait for the backend IO to complete too)

When the backend IO final completes (or fails), the abort handling
will proceed and queue up a "return aborted status" operation.  Then,
while that's still pending, the LUN reset might find the original
command still on the LUN's list of commands and try to return aborted
status again, which leads to a use-after free when the first
se_tfo->queue_status call frees the command and then the second
se_tfo->queue_status call runs.

Fix this by removing a command from the LUN state_list when we first
are about to queue aborted status; we shouldn't do anything
LUN-related after we've started returning status, so this seems like
the correct thing to do.
Signed-off-by: NRoland Dreier <roland@purestorage.com>
Cc: stable@vger.kernel.org
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

This patch fixes a regression bug introduced during v3.6.x code with
the following commit to drop transport_add_cmd_to_queue(), which
originally re-set CMD_T_ACTIVE during pending WRITE I/O submission:

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

The following sequence happens for write commands (or any other
commands with a data out phase):

 - The transport calls target_submit_cmd(), which sets CMD_T_ACTIVE in
   cmd->transport_state and sets cmd->t_state to TRANSPORT_NEW_CMD.
 - Things go on transport_generic_new_cmd(), which notices that the
   command needs to transfer data, so it sets cmd->t_state to
   TRANSPORT_WRITE_PENDING and calls transport_cmd_check_stop().
 - transport_cmd_check_stop() clears CMD_T_ACTIVE in cmd->transport_state
   and returns in the normal case.
 - Then we continue on to call ->se_tfo->write_pending().
 - The data comes back from the initiator, and the transport calls
   target_execute_cmd(), which sets cmd->t_state to TRANSPORT_PROCESSING
   and calls into the backend to actually write the data.

At this point, the backend might take a long time to complete the
command, since it has to do real IO.  If an abort request comes in for
this command at this point, it will not wait for the command to finish
since CMD_T_ACTIVE is not set.  Then when the command does finally
finish, we blow up with use-after-free.

Avoid this by setting CMD_T_ACTIVE in target_execute_cmd() so that
transport_wait_for_tasks() waits for the command to finish executing.
This matches the behavior from before commit 1389533e ("target:
remove transport_generic_handle_data"), when data was signaled via
transport_generic_handle_data(), which set CMD_T_ACTIVE because it
called transport_add_cmd_to_queue().
Signed-off-by: NRoland Dreier <roland@purestorage.com>
Reported-by: NMartin Svec <martin.svec@zoner.cz>
Cc: Christoph Hellwig <hch@lst.de>
Cc: stable@vger.kernel.org
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

Introduce TCM_NO_SENSE, mapping to sense code
'Not ready, no additional sense information'.
Signed-off-by: NHannes Reinecke <hare@suse.de>
Cc: Nicholas Bellinger <nab@risingtidesystems.com>
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

The ASC/ASCQ code for 'Logical Unit Communication failure' is
0x08/0x00; 0x80/0x00 is vendor specific.
Signed-off-by: NHannes Reinecke <hare@suse.de>
Cc: Nicholas Bellinger <nab@risingtidesystems.com>
Cc: stable@vger.kernel.org
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

Signed-off-by: NRoland Dreier <roland@purestorage.com>
Signed-off-by: NNicholas Bellinger <nab@risingtidesystems.com>

No need to have a goto where a return is clearer.
Signed-off-by: NRoland Dreier <roland@purestorage.com>
Signed-off-by: NNicholas Bellinger <nab@risingtidesystems.com>

v2: Use correct target_core_stat.c 2006 copyright year
v3: Drop extra unnessary legal verbage from header (hch)

Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

- If we stop processing an already-aborted command in
  target_execute_cmd(), then we need to complete t_transport_stop_comp
  to wake up the the TMR handling thread, or else it will end up
  waiting forever.

- If we've a already sent an "aborted" status for a command in
  transport_check_aborted_status() then we should bail out of
  transport_send_task_abort() to avoid freeing the command twice.
Signed-off-by: NRoland Dreier <roland@purestorage.com>
Cc: stable@vger.kernel.org
Signed-off-by: NNicholas Bellinger <nab@risingtidesystems.com>

This patch fixes a possible case in transport_generic_new_cmd() where a
failure from TFO->write_pending() from a fabric module return something
other than -EAGAIN or -ENOMEM would cause a failed WRITE to silently
succeed.

Go ahead and return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE for this
special case instead of only just making noise with WARN_ON().

(v2: Fix incorrect exception return for all cases)

Cc: Christoph Hellwig <hch@lst.de>
Cc: Roland Dreier <roland@purestorage.com>
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

Pass the sense reason as an explicit return value from the I/O submission
path instead of storing it in struct se_cmd and using negative return
values.  This cleans up a lot of the code pathes, and with the sparse
annotations for the new sense_reason_t type allows for much better
error checking.

(nab: Convert spc_emulate_modesense + spc_emulate_modeselect to use
      sense_reason_t with Roland's MODE SELECT changes)
Signed-off-by: NChristoph Hellwig <hch@lst.de>
Cc: Roland Dreier <roland@purestorage.com>
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

We always support ALUA for virtual backends, and never for physical ones.  Simplify
the code to just deal with these two cases and remove the superflous abstractions.
Signed-off-by: NChristoph Hellwig <hch@lst.de>
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

We do not support host-level reservations for the pscsi backend, and all
virtual backends are newere than SCSI-2, so just make the combined
SPC-3 + SCSI-2 support the only supported variant and kill the switches
for the different implementations, given that this code handles the no-op
version just fine.

(hch: Update DRF_SPC2_RESERVATIONS lock usage)
Signed-off-by: NChristoph Hellwig <hch@lst.de>

We can just key off ordered tag emulation of the transport_type field.
Signed-off-by: NChristoph Hellwig <hch@lst.de>
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

drivers/target/target_core_pscsi.c:464:5: sparse: symbol 'pscsi_configure_device'
was not declared. Should it be static?

FYI, there are new sparse warnings show up in

tree:   git://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending.git queue
head:   738b86ac5e56c645aa5b7bf49cb38e2a04c665f8
commit: 410aeee637c47bcf7e8dd7893347fe0811e07ab1 [47/51] target: kill struct se_subsystem_dev

vim +464 drivers/target/target_core_pscsi.c

410aeee6 Christoph Hellwig  2012-10-08 @464  int pscsi_configure_device(struct se_device *dev)
c66ac9db Nicholas Bellinger 2010-12-17  465  {
410aeee6 Christoph Hellwig  2012-10-08  466  	struct se_hba *hba = dev->se_hba;
410aeee6 Christoph Hellwig  2012-10-08  467  	struct pscsi_dev_virt *pdv = PSCSI_DEV(dev);
c66ac9db Nicholas Bellinger 2010-12-17  468  	struct scsi_device *sd;
410aeee6 Christoph Hellwig  2012-10-08  469  	struct pscsi_hba_virt *phv = dev->se_hba->hba_ptr;
c66ac9db Nicholas Bellinger 2010-12-17  470  	struct Scsi_Host *sh = phv->phv_lld_host;
c66ac9db Nicholas Bellinger 2010-12-17  471  	int legacy_mode_enable = 0;
410aeee6 Christoph Hellwig  2012-10-08  472  	int ret;

Please consider folding the attached diff :-)
Signed-off-by: NFengguang Wu <fengguang.wu@intel.com>
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

Simplify the code a lot by killing the superflous struct se_subsystem_dev.
Instead se_device is allocated early on by the backend driver, which allocates
it as part of its own per-device structure, borrowing the scheme that is for
example used for inode allocation.
Signed-off-by: NChristoph Hellwig <hch@lst.de>
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

Fabric drivers currently expect to internally release se_cmd in the event
of a TMR failure during target_submit_tmr(), which means the immediate call
to transport_generic_free_cmd() after TFO->queue_tm_rsp() from within
target_complete_tmr_failure() workqueue context is wrong.

This is done as some fabrics expect TMR operations to be acknowledged
before releasing the descriptor, so the assumption that core is releasing
se_cmd associated TMR memory is incorrect.  This fixes a OOPs where
transport_generic_free_cmd() was being called more than once.

This bug was originally observed with tcm_qla2xxx fabric ports.

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>

This patch carries forward a work-around from tcm_loop to target
core code to explicitly clear control CDB READ paylods in order to
avoid bugs in scsi-generic user-space code for INQUIRY that do not
explicitly zero CDB payload memory.

(v2: Drop TARGET_SCF_MAP_CLEAR_MEM, and perform the explicit zero
     of READ memory for all target_submit_cmd_map_sgls users)

Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

This patch adds a new target_submit_cmd_map_sgls() to pass pre-allocated
SGL memory using transport_generic_map_mem_to_cmd() logic into the generic
target submit I/O codepath.

It also adds a target_submit_cmd() wrapper around target_submit_cmd_map_sgls()
for existing fabric code that already assumes internal target-core SGL memory
allocation.

(v2: Rename to target_submit_cmd_map_sgls + drop TARGET_SCF_MAP_MEM flag
     in favor of non zero sgl_count check)
Reported-by: NChristoph Hellwig <hch@lst.de>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

Yay, all users of transport_kmap_data_sg now check for a zero-length
request and/or a too-small parameter list length.  We can thus go through
the normal emulation path even for such commands.

This means that out-of-bounds reads and writes are now reported correctly
even if they transfer 0 blocks.  Other errors are also reported correctly.

Testcase: sg_raw /dev/sdb 28 00 80 00 00 00 00 00 00 00
    should fail with ILLEGAL REQUEST / LBA OUT OF RANGE sense
    does not fail without the patch
    (still wrong with the patch, but better: the ASC is INVALID FIELD IN CDB)
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

Correct spelling typo in printk and comment within drivers/target.
Signed-off-by: NMasanari Iida <standby24x7@gmail.com>
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

Every fabric driver has to supply a se_tfo->set_fabric_sense_len()
method, just so iSCSI can return an offset of 2.  However, every fabric
driver is already allocating a sense buffer and passing it into the
target core, either via transport_init_se_cmd() or target_submit_cmd().

So instead of having iSCSI pass the start of its sense buffer into the
core and then later tell the core to skip the first 2 bytes, it seems
easier for iSCSI just to do the offset of 2 when it passes the sense
buffer into the core.  Then we can drop the se_tfo->set_fabric_sense_len()
everywhere, and just add a couple of lines of code to iSCSI to set the
sense data length to the beginning of the buffer right before it sends
it over the network.

(nab: Remove .set_fabric_sense_len usage from tcm_qla2xxx_npiv_ops +
      change transport_get_sense_buffer to follow v3.6-rc6 code w/o
      ->set_fabric_sense_len usage)
Signed-off-by: NRoland Dreier <roland@purestorage.com>
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

Move static into function body from file scope.

Remove extraneous return statement
Signed-off-by: NAndy Grover <agrover@redhat.com>
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

It is no longer a supported module.
Signed-off-by: NAndy Grover <agrover@redhat.com>
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

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>

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>

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>

The pointer to the sense buffer is fetched by transport_get_sense_data,
but this is called by target_complete_ok_work long after pscsi_req_done
has freed the struct that contains it.

Pass instead the fabric's sense buffer to transport_complete,
and copy the data to it directly in transport_complete.  Setting
SCF_TRANSPORT_TASK_SENSE also becomes a duty of transport_complete.
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
Cc: stable@vger.kernel.org
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

The error conditions in transport_get_sense_data are superfluous
and complicate the code unnecessarily:

* SCF_TRANSPORT_TASK_SENSE is checked in the caller;

* it's simply part of the invariants of dev->transport->get_sense_buffer
  that it must be there if transport_complete ever returns 1, and that
  it must not return NULL.  Besides, the entire callback will disappear
  with the next patch.

* similarly in the caller we can expect that sense data is only sent
  for non-zero cmd->scsi_status.
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
Cc: stable@vger.kernel.org
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

We will be calling it from transport_complete_cmd, avoid forward
declarations.  No semantic change.
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
Cc: stable@vger.kernel.org
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

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>

This was originally for helping fabrics to determine overflow/underflow
status, and has been superceeded by SCF_OVERFLOW_BIT + SCF_UNDERFLOW_BIT.
Signed-off-by: NRoland Dreier <roland@purestorage.com>
Signed-off-by: NNicholas Bellinger <nab@linux-iscsi.org>

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>