Commit Graph

497 Commits

Author SHA1 Message Date
Linus Torvalds
99dbb1632f Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jikos/trivial
Pull the trivial tree from Jiri Kosina:
 "Tiny usual fixes all over the place"

* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jikos/trivial: (34 commits)
  doc: fix old config name of kprobetrace
  fs/fs-writeback.c: cleanup riteback_sb_inodes kerneldoc
  btrfs: fix the commment for the action flags in delayed-ref.h
  btrfs: fix trivial typo for the comment of BTRFS_FREE_INO_OBJECTID
  vfs: fix kerneldoc for generic_fh_to_parent()
  treewide: fix comment/printk/variable typos
  ipr: fix small coding style issues
  doc: fix broken utf8 encoding
  nfs: comment fix
  platform/x86: fix asus_laptop.wled_type module parameter
  mfd: printk/comment fixes
  doc: getdelays.c: remember to close() socket on error in create_nl_socket()
  doc: aliasing-test: close fd on write error
  mmc: fix comment typos
  dma: fix comments
  spi: fix comment/printk typos in spi
  Coccinelle: fix typo in memdup_user.cocci
  tmiofb: missing NULL pointer checks
  tools: perf: Fix typo in tools/perf
  tools/testing: fix comment / output typos
  ...
2012-10-01 09:06:36 -07:00
Paolo Bonzini
6abbdf3836 target: go through normal processing for zero-length REQUEST_SENSE
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: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-09-07 11:32:54 -07:00
Paolo Bonzini
32a8811ff1 target: support zero allocation length in REQUEST SENSE
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: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-09-07 11:14:21 -07:00
Paolo Bonzini
3717ef0c63 target: support zero-size allocation lengths in transport_kmap_data_sg
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: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-09-07 11:10:32 -07:00
Paolo Bonzini
9b16b9edb4 target: fail REPORT LUNS with less than 16 bytes of payload
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: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-09-07 11:09:14 -07:00
Paolo Bonzini
0d7f1299ca target: report too-small parameter lists everywhere
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: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-09-07 11:09:08 -07:00
Paolo Bonzini
306c11b28d target: go through normal processing for zero-length PSCSI commands
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: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-09-07 11:04:08 -07:00
Paolo Bonzini
d5829eac5f target: fix use-after-free with PSCSI sense data
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: Paolo Bonzini <pbonzini@redhat.com>
Cc: stable@vger.kernel.org
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-09-05 17:20:28 -07:00
Paolo Bonzini
27a2709912 target: simplify code around transport_get_sense_data
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: Paolo Bonzini <pbonzini@redhat.com>
Cc: stable@vger.kernel.org
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-09-05 17:20:19 -07:00
Paolo Bonzini
6138ed2ab8 target: move transport_get_sense_data
We will be calling it from transport_complete_cmd, avoid forward
declarations.  No semantic change.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Cc: stable@vger.kernel.org
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-09-05 17:19:46 -07:00
Masanari Iida
59e13d4833 scsi: fix various printk and comment typos
Correct spelling typo within drivers/scsi

Signed-off-by: Masanari Iida <standby24x7@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
2012-09-01 08:24:35 -07:00
Benjamin Wang
13b5533a38 target: Check idr_get_new return value in iscsi_login_zero_tsih_s1
This patch updates iscsi_login_zero_tsih_s1() usage for generating
iscsi_session->session_index to properly check the return value from
idr_get_new(), and reject the iSCSI login attempt with exception
status ISCSI_LOGIN_STATUS_NO_RESOURCES in the event of a failure.

Signed-off-by: Benjamin Wang <cpwang2009@gmail.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-08-26 23:27:08 -07:00
Nicholas Bellinger
4c054ba63a target: Fix ->data_length re-assignment bug with SCSI overflow
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: Roland 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: Nicholas Bellinger <nab@linux-iscsi.org>
2012-08-23 15:26:52 -07:00
Roland Dreier
af74115eed target: Remove unused se_cmd.cmd_spdtl
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: Roland Dreier <roland@purestorage.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-08-21 14:27:35 -07:00
Denis Efremov
08a16208c8 tcm_fc: rcu_deref outside rcu lock/unlock section
Use rcu_dereference_protected in order to prevent lockdep
complaint. Sequel of the patch 863555be

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Denis Efremov <yefremov.denis@gmail.com>
Acked-by: Mark D. Rustad <mark.d.rustad@intel.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-08-20 14:52:15 -07:00
Nicholas Bellinger
74f4cf2909 target: Fix regression bug with handling of zero-length data CDBs
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 4101f0a89d
  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 91ec1d3535
  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: Nicholas Bellinger <nab@linux-iscsi.org>
2012-08-17 17:24:11 -07:00
Nicholas Bellinger
1d2a2cd95e target/pscsi: Fix bug with REPORT_LUNs handling for SCSI passthrough
This patch fixes a regression bug in pscsi_transport_complete() callback
code where *pt was being NULL dereferenced during REPORT_LUNS handling,
that was introduced with the spc/sbc refactoring in:

commit 1fd032ee10
Author: Christoph Hellwig <hch@infradead.org>
Date:   Sun May 20 11:59:15 2012 -0400

    target: move code for CDB emulation

As this is a special case for pscsi_parse_cdb() to call spc_parse_cdb() to
allow TCM to handle REPORT_LUN emulation, pscsi_plugin_task will have not
been allocated..

So now in pscsi_transport_complete() just check for existence of *pt and
return for this special case.

Reported-by: Alex Elsayed <eternaleye+usenet@gmail.com>
Cc: Alex Elsayed <eternaleye+usenet@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-08-16 23:36:55 -07:00
Yi Zou
d0e27c88d7 target: fix NULL pointer dereference bug alloc_page() fails to get memory
I am hitting this bug when the target is low in memory that fails the
alloc_page() for the newly submitted command. This is a sort of off-by-one
bug causing NULL pointer dereference in __free_page() since 'i' here is
really the counter of total pages that have been successfully allocated here.

Signed-off-by: Yi Zou <yi.zou@intel.com>
Cc: Andy Grover <agrover@redhat.com>
Cc: Nicholas Bellinger <nab@linux-iscsi.org>
Cc: Open-FCoE.org <devel@open-fcoe.org>
Cc: stable@vger.kernel.org
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-08-16 17:33:46 -07:00
Mark Rustad
1fa8f45041 tcm_fc: Avoid debug overhead when not debugging
Stop doing a pile of work related to debugging messages when
the ft_debug_logging flag is not set. Use unlikely to add the
check in a way that the check can be inlined without inlining the
whole thing.

Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-08-16 17:33:46 -07:00
Linus Torvalds
a0e881b7c1 Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs
Pull second vfs pile from Al Viro:
 "The stuff in there: fsfreeze deadlock fixes by Jan (essentially, the
  deadlock reproduced by xfstests 068), symlink and hardlink restriction
  patches, plus assorted cleanups and fixes.

  Note that another fsfreeze deadlock (emergency thaw one) is *not*
  dealt with - the series by Fernando conflicts a lot with Jan's, breaks
  userland ABI (FIFREEZE semantics gets changed) and trades the deadlock
  for massive vfsmount leak; this is going to be handled next cycle.
  There probably will be another pull request, but that stuff won't be
  in it."

Fix up trivial conflicts due to unrelated changes next to each other in
drivers/{staging/gdm72xx/usb_boot.c, usb/gadget/storage_common.c}

* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs: (54 commits)
  delousing target_core_file a bit
  Documentation: Correct s_umount state for freeze_fs/unfreeze_fs
  fs: Remove old freezing mechanism
  ext2: Implement freezing
  btrfs: Convert to new freezing mechanism
  nilfs2: Convert to new freezing mechanism
  ntfs: Convert to new freezing mechanism
  fuse: Convert to new freezing mechanism
  gfs2: Convert to new freezing mechanism
  ocfs2: Convert to new freezing mechanism
  xfs: Convert to new freezing code
  ext4: Convert to new freezing mechanism
  fs: Protect write paths by sb_start_write - sb_end_write
  fs: Skip atime update on frozen filesystem
  fs: Add freezing handling to mnt_want_write() / mnt_drop_write()
  fs: Improve filesystem freezing handling
  switch the protection of percpu_counter list to spinlock
  nfsd: Push mnt_want_write() outside of i_mutex
  btrfs: Push mnt_want_write() outside of i_mutex
  fat: Push mnt_want_write() outside of i_mutex
  ...
2012-08-01 10:26:23 -07:00
Al Viro
dbc6e0222d delousing target_core_file a bit
* 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: Al Viro <viro@zeniv.linux.org.uk>
2012-08-01 16:53:16 +04:00
Al Viro
bf6932f44a iscsi-target: Drop bogus struct file usage for iSCSI/SCTP
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: Al 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: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-21 02:44:13 -07:00
Dan Carpenter
2962846d14 target: NULL dereference on error path
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 af8772926f
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: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-20 11:34:21 -07:00
Roland Dreier
d6dfc868bc target: Allow for target_submit_cmd() returning errors
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: Roland Dreier <roland@purestorage.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-17 17:05:05 -07:00
Roland Dreier
7409a6657a target: Check number of unmap descriptors against our limit
Fail UNMAP commands that have more than our reported limit on unmap
descriptors.

Signed-off-by: Roland Dreier <roland@purestorage.com>
Cc: stable@vger.kernel.org
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:36 -07:00
Roland Dreier
b7fc7f3777 target: Fix possible integer underflow in UNMAP emulation
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: Roland Dreier <roland@purestorage.com>
Cc: stable@vger.kernel.org
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:36 -07:00
Roland Dreier
1a5fa4576e target: Fix reading of data length fields for UNMAP commands
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: Roland Dreier <roland@purestorage.com>
Cc: stable@vger.kernel.org
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:36 -07:00
Roland Dreier
2594e29865 target: Add range checking to UNMAP emulation
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: Roland Dreier <roland@purestorage.com>
Cc: stable@vger.kernel.org
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:35 -07:00
Roland Dreier
e2397c7044 target: Add generation of LOGICAL BLOCK ADDRESS OUT OF RANGE
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: Roland Dreier <roland@purestorage.com>
Cc: stable@vger.kernel.org
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:35 -07:00
Roland Dreier
c8045372dd target: Make unnecessarily global se_dev_align_max_sectors() static
Signed-off-by: Roland Dreier <roland@purestorage.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:35 -07:00
Roland Dreier
1c7b13fe65 target: Remove se_session.sess_wait_list
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: Roland Dreier <roland@purestorage.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:27 -07:00
Roland Dreier
bc187ea6c3 target: Check sess_tearing_down in target_get_sess_cmd()
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: Roland Dreier <roland@purestorage.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:26 -07:00
Roland Dreier
7c78b8de26 sbp-target: Consolidate duplicated error path code in sbp_handle_command()
Cc: Chris Boot <bootc@bootc.net>
Cc: Stefan Richter <stefanr@s5r6.in-berlin.de>
Signed-off-by: Roland Dreier <roland@purestorage.com>
2012-07-16 17:35:26 -07:00
Roland Dreier
669ab62c9d target: Un-export target_get_sess_cmd()
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: Roland Dreier <roland@purestorage.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:25 -07:00
Nicholas Bellinger
77d4c74506 target: Make core_disable_device_list_for_node use pre-refactoring lock ordering
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: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:24 -07:00
Andy Grover
e80ac6c4cc target: refactor core_update_device_list_for_node()
Code was almost entirely divided based on value of bool param "enable".

Split it into two functions.

Signed-off-by: Andy Grover <agrover@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:24 -07:00
Andy Grover
cdf88a2fe3 target: Eliminate else using boolean logic
Signed-off-by: Andy Grover <agrover@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:23 -07:00
Andy Grover
617a0c2eb3 target: Misc retval cleanups
Bubble-up retval from iscsi_update_param_value() and
iscsit_ta_authentication().

Other very small retval cleanups.

Signed-off-by: Andy Grover <agrover@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:23 -07:00
Andy Grover
2dca673b46 target: Remove hba param from core_dev_add_lun
Only used in a debugprint, and function signature is cleaner now.

Signed-off-by: Andy Grover <agrover@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:22 -07:00
Andy Grover
ee1b1b9c3d target: Remove unneeded double parentheses
Signed-off-by: Andy Grover <agrover@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:22 -07:00
Christoph Hellwig
af8772926f target: replace the processing thread with a TMR work queue
The last functionality of the target processing thread is offloading possibly
long running task management requests from the submitter context.  To keep
TMR semantics the same we need a single threaded ordered queue, which can
be provided by a per-device workqueue with the right flags.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:21 -07:00
Christoph Hellwig
f314643751 target: remove transport_generic_handle_cdb_map
Remove this command submission path which is not used by any in-tree driver.
This also removes the now unused new_cmd_map fabtric method, which a few
drivers implemented despite never calling transport_generic_handle_cdb_map.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:21 -07:00
Christoph Hellwig
7a6f0a1ea5 target: simply fabric driver queue full processing
There is no need to schedule the delayed processing in a workqueue that
offloads it to the target processing thread.  Instead execute it directly
from the workqueue.  There will be a lot of future work in this area,
which I'd likfe to defer for now as it is not nessecary for getting rid
of the target processing thread.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:20 -07:00
Christoph Hellwig
1389533ef9 target: remove transport_generic_handle_data
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:20 -07:00
Christoph Hellwig
b8b22533fe tcm_fc: Offload WRITE I/O backend submission to tpg workqueue
Defer the write processing to the internal to be able to use
target_execute_cmd.  I'm not even entirely sure the calling code requires
this due to the convoluted structure in libfc, but let's be safe for now.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Cc: Mark Rustad <mark.d.rustad@intel.com>
Cc: Kiran Patil <Kiran.patil@intel.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:19 -07:00
Christoph Hellwig
67441b68fa iscsit: use target_execute_cmd for WRITEs
All three callers of transport_generic_handle_data are from user context
and can use target_execute_cmd directly to handle the backend I/O submission
of WRITE I/O.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Cc: Andy Grover <agrover@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:18 -07:00
Christoph Hellwig
c3196f0cf0 target: merge transport_generic_write_pending into transport_generic_new_cmd
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:17 -07:00
Christoph Hellwig
d59a02b4eb target: call transport_check_aborted_status from target_execute_cmd
When we call target_execute_cmd for write commands the command has been
on the state list before an abort might have come in before
target_execute_cmd.  Call transport_check_aborted_status to deal with
this case.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:16 -07:00
Christoph Hellwig
70baf0ab3b target: remove transport_generic_process_write
Just call target_execute_cmd directly.  Also, convert loopback, sbp,
usb-gadget to use the newly exported target_execute_cmd().

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:16 -07:00
Christoph Hellwig
f7113a47e2 target: split transport_cmd_check_stop
Inline the transport_off == 0 case into target_execute_cmd to simplify
the function for the remaining cases.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
2012-07-16 17:35:15 -07:00