1. 07 Jul, 2017 22 commits
  2. 09 Jun, 2017 10 commits
    • Nicholas Bellinger's avatar
      iscsi-target: Avoid holding ->tpg_state_lock during param update · eceb4459
      Nicholas Bellinger authored
      As originally reported by Jia-Ju, iscsit_tpg_enable_portal_group()
      holds iscsi_portal_group->tpg_state_lock while updating AUTHMETHOD
      via iscsi_update_param_value(), which performs a GFP_KERNEL
      allocation.
      
      However, since iscsit_tpg_enable_portal_group() is already protected
      by iscsit_get_tpg() -> iscsi_portal_group->tpg_access_lock in it's
      parent caller, ->tpg_state_lock only needs to be held when setting
      TPG_STATE_ACTIVE.
      Reported-by: default avatarJia-Ju Bai <baijiaju1990@163.com>
      Reviewed-by: default avatarJia-Ju Bai <baijiaju1990@163.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      eceb4459
    • Nicholas Bellinger's avatar
      target/configfs: Kill se_lun->lun_link_magic · 9ae0e9ad
      Nicholas Bellinger authored
      Instead of using a hardcoded magic value in se_lun when verifying
      a target config_item symlink source during target_fabric_mappedlun_link(),
      go ahead and use target_fabric_port_item_ops directly instead.
      Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
      Cc: Mike Christie <mchristi@redhat.com>
      Cc: Hannes Reinecke <hare@suse.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      9ae0e9ad
    • Nicholas Bellinger's avatar
      target/configfs: Kill se_device->dev_link_magic · c17cd249
      Nicholas Bellinger authored
      Instead of using a hardcoded magic value in se_device when verifying
      a target config_item symlink source during target_fabric_port_link(),
      go ahead and use target_core_dev_item_ops directly instead.
      Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
      Cc: Mike Christie <mchristi@redhat.com>
      Cc: Hannes Reinecke <hare@suse.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      c17cd249
    • Nicholas Bellinger's avatar
      target/iblock: Convert WRITE_SAME to blkdev_issue_zeroout · 2237498f
      Nicholas Bellinger authored
      The people who are actively using iblock_execute_write_same_direct() are
      doing so in the context of ESX VAAI BlockZero, together with
      EXTENDED_COPY and COMPARE_AND_WRITE primitives.
      
      In practice though I've not seen any users of IBLOCK WRITE_SAME for
      anything other than VAAI BlockZero, so just using blkdev_issue_zeroout()
      when available, and falling back to iblock_execute_write_same() if the
      WRITE_SAME buffer contains anything other than zeros should be OK.
      
      (Hook up max_write_zeroes_sectors to signal LBPRZ feature bit in
       target_configure_unmap_from_queue - nab)
      Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
      Reviewed-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      Cc: Christoph Hellwig <hch@lst.de>
      Cc: Mike Christie <mchristi@redhat.com>
      Cc: Hannes Reinecke <hare@suse.com>
      Cc: Jens Axboe <axboe@fb.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      2237498f
    • Michael Cyr's avatar
      ibmvscsis: Enable Logical Partition Migration Support · 464fd641
      Michael Cyr authored
      Changes to support a new mechanism from phyp to better synchronize the
      logical partition migration (LPM) of the client partition.
      This includes a new VIOCTL to register that we support this new
      functionality, and 2 new Transport Event types, and finally another
      new VIOCTL to let phyp know once we're ready for the Suspend.
      Signed-off-by: default avatarMichael Cyr <mikecyr@us.ibm.com>
      Signed-off-by: default avatarBryant G. Ly <bryantly@linux.vnet.ibm.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      464fd641
    • Byungchul Park's avatar
      vhost/scsi: Don't reinvent the wheel but use existing llist API · 12bdcbd5
      Byungchul Park authored
      Although llist provides proper APIs, they are not used. Make them used.
      Signed-off-by: default avatarByungchul Park <byungchul.park@lge.com>
      Acked-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      12bdcbd5
    • Gustavo A. R. Silva's avatar
      target: remove dead code · fb418240
      Gustavo A. R. Silva authored
      Local variable _ret_ is assigned to a constant value and it is never
      updated again. Remove this variable and the dead code it guards.
      
      Addresses-Coverity-ID: 140761
      Signed-off-by: default avatarGustavo A. R. Silva <garsilva@embeddedor.com>
      Reviewed-by: default avatarTyrel Datwyler <tyreld@linux.vnet.ibm.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      fb418240
    • Nicholas Bellinger's avatar
      iscsi-target: Reject immediate data underflow larger than SCSI transfer length · abb85a9b
      Nicholas Bellinger authored
      When iscsi WRITE underflow occurs there are two different scenarios
      that can happen.
      
      Normally in practice, when an EDTL vs. SCSI CDB TRANSFER LENGTH
      underflow is detected, the iscsi immediate data payload is the
      smaller SCSI CDB TRANSFER LENGTH.
      
      That is, when a host fabric LLD is using a fixed size EDTL for
      a specific control CDB, the SCSI CDB TRANSFER LENGTH and actual
      SCSI payload ends up being smaller than EDTL.  In iscsi, this
      means the received iscsi immediate data payload matches the
      smaller SCSI CDB TRANSFER LENGTH, because there is no more
      SCSI payload to accept beyond SCSI CDB TRANSFER LENGTH.
      
      However, it's possible for a malicous host to send a WRITE
      underflow where EDTL is larger than SCSI CDB TRANSFER LENGTH,
      but incoming iscsi immediate data actually matches EDTL.
      
      In the wild, we've never had a iscsi host environment actually
      try to do this.
      
      For this special case, it's wrong to truncate part of the
      control CDB payload and continue to process the command during
      underflow when immediate data payload received was larger than
      SCSI CDB TRANSFER LENGTH, so go ahead and reject and drop the
      bogus payload as a defensive action.
      
      Note this potential bug was originally relaxed by the following
      for allowing WRITE underflow in MSFT FCP host environments:
      
         commit c72c5250
         Author: Roland Dreier <roland@purestorage.com>
         Date:   Wed Jul 22 15:08:18 2015 -0700
      
            target: allow underflow/overflow for PR OUT etc. commands
      
      Cc: Roland Dreier <roland@purestorage.com>
      Cc: Mike Christie <mchristi@redhat.com>
      Cc: Hannes Reinecke <hare@suse.de>
      Cc: Martin K. Petersen <martin.petersen@oracle.com>
      Cc: <stable@vger.kernel.org> # v4.3+
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      abb85a9b
    • Nicholas Bellinger's avatar
      iscsi-target: Fix delayed logout processing greater than SECONDS_FOR_LOGOUT_COMP · 105fa2f4
      Nicholas Bellinger authored
      This patch fixes a BUG() in iscsit_close_session() that could be
      triggered when iscsit_logout_post_handler() execution from within
      tx thread context was not run for more than SECONDS_FOR_LOGOUT_COMP
      (15 seconds), and the TCP connection didn't already close before
      then forcing tx thread context to automatically exit.
      
      This would manifest itself during explicit logout as:
      
      [33206.974254] 1 connection(s) still exist for iSCSI session to iqn.1993-08.org.debian:01:3f5523242179
      [33206.980184] INFO: NMI handler (kgdb_nmi_handler) took too long to run: 2100.772 msecs
      [33209.078643] ------------[ cut here ]------------
      [33209.078646] kernel BUG at drivers/target/iscsi/iscsi_target.c:4346!
      
      Normally when explicit logout attempt fails, the tx thread context
      exits and iscsit_close_connection() from rx thread context does the
      extra cleanup once it detects conn->conn_logout_remove has not been
      cleared by the logout type specific post handlers.
      
      To address this special case, if the logout post handler in tx thread
      context detects conn->tx_thread_active has already been cleared, simply
      return and exit in order for existing iscsit_close_connection()
      logic from rx thread context do failed logout cleanup.
      Reported-by: default avatarBart Van Assche <bart.vanassche@sandisk.com>
      Tested-by: default avatarBart Van Assche <bart.vanassche@sandisk.com>
      Cc: Mike Christie <mchristi@redhat.com>
      Cc: Hannes Reinecke <hare@suse.de>
      Cc: Sagi Grimberg <sagig@mellanox.com>
      Cc: stable@vger.kernel.org # 3.14+
      Tested-by: default avatarGary Guo <ghg@datera.io>
      Tested-by: default avatarChu Yuan Lin <cyl@datera.io>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      105fa2f4
    • Nicholas Bellinger's avatar
      target: Fix kref->refcount underflow in transport_cmd_finish_abort · 73d4e580
      Nicholas Bellinger authored
      This patch fixes a se_cmd->cmd_kref underflow during CMD_T_ABORTED
      when a fabric driver drops it's second reference from below the
      target_core_tmr.c based callers of transport_cmd_finish_abort().
      
      Recently with the conversion of kref to refcount_t, this bug was
      manifesting itself as:
      
      [705519.601034] refcount_t: underflow; use-after-free.
      [705519.604034] INFO: NMI handler (kgdb_nmi_handler) took too long to run: 20116.512 msecs
      [705539.719111] ------------[ cut here ]------------
      [705539.719117] WARNING: CPU: 3 PID: 26510 at lib/refcount.c:184 refcount_sub_and_test+0x33/0x51
      
      Since the original kref atomic_t based kref_put() didn't check for
      underflow and only invoked the final callback when zero was reached,
      this bug did not manifest in practice since all se_cmd memory is
      using preallocated tags.
      
      To address this, go ahead and propigate the existing return from
      transport_put_cmd() up via transport_cmd_finish_abort(), and
      change transport_cmd_finish_abort() + core_tmr_handle_tas_abort()
      callers to only do their local target_put_sess_cmd() if necessary.
      Reported-by: default avatarBart Van Assche <bart.vanassche@sandisk.com>
      Tested-by: default avatarBart Van Assche <bart.vanassche@sandisk.com>
      Cc: Mike Christie <mchristi@redhat.com>
      Cc: Hannes Reinecke <hare@suse.de>
      Cc: Christoph Hellwig <hch@lst.de>
      Cc: Himanshu Madhani <himanshu.madhani@qlogic.com>
      Cc: Sagi Grimberg <sagig@mellanox.com>
      Cc: stable@vger.kernel.org # 3.14+
      Tested-by: default avatarGary Guo <ghg@datera.io>
      Tested-by: default avatarChu Yuan Lin <cyl@datera.io>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      73d4e580
  3. 31 May, 2017 2 commits
    • Jiang Yi's avatar
      iscsi-target: Always wait for kthread_should_stop() before kthread exit · 5e0cf5e6
      Jiang Yi authored
      There are three timing problems in the kthread usages of iscsi_target_mod:
      
       - np_thread of struct iscsi_np
       - rx_thread and tx_thread of struct iscsi_conn
      
      In iscsit_close_connection(), it calls
      
       send_sig(SIGINT, conn->tx_thread, 1);
       kthread_stop(conn->tx_thread);
      
      In conn->tx_thread, which is iscsi_target_tx_thread(), when it receive
      SIGINT the kthread will exit without checking the return value of
      kthread_should_stop().
      
      So if iscsi_target_tx_thread() exit right between send_sig(SIGINT...)
      and kthread_stop(...), the kthread_stop() will try to stop an already
      stopped kthread.
      
      This is invalid according to the documentation of kthread_stop().
      
      (Fix -ECONNRESET logout handling in iscsi_target_tx_thread and
       early iscsi_target_rx_thread failure case - nab)
      Signed-off-by: default avatarJiang Yi <jiangyilism@gmail.com>
      Cc: <stable@vger.kernel.org> # v3.12+
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      5e0cf5e6
    • Nicholas Bellinger's avatar
      iscsi-target: Fix initial login PDU asynchronous socket close OOPs · 25cdda95
      Nicholas Bellinger authored
      This patch fixes a OOPs originally introduced by:
      
         commit bb048357
         Author: Nicholas Bellinger <nab@linux-iscsi.org>
         Date:   Thu Sep 5 14:54:04 2013 -0700
      
         iscsi-target: Add sk->sk_state_change to cleanup after TCP failure
      
      which would trigger a NULL pointer dereference when a TCP connection
      was closed asynchronously via iscsi_target_sk_state_change(), but only
      when the initial PDU processing in iscsi_target_do_login() from iscsi_np
      process context was blocked waiting for backend I/O to complete.
      
      To address this issue, this patch makes the following changes.
      
      First, it introduces some common helper functions used for checking
      socket closing state, checking login_flags, and atomically checking
      socket closing state + setting login_flags.
      
      Second, it introduces a LOGIN_FLAGS_INITIAL_PDU bit to know when a TCP
      connection has dropped via iscsi_target_sk_state_change(), but the
      initial PDU processing within iscsi_target_do_login() in iscsi_np
      context is still running.  For this case, it sets LOGIN_FLAGS_CLOSED,
      but doesn't invoke schedule_delayed_work().
      
      The original NULL pointer dereference case reported by MNC is now handled
      by iscsi_target_do_login() doing a iscsi_target_sk_check_close() before
      transitioning to FFP to determine when the socket has already closed,
      or iscsi_target_start_negotiation() if the login needs to exchange
      more PDUs (eg: iscsi_target_do_login returned 0) but the socket has
      closed.  For both of these cases, the cleanup up of remaining connection
      resources will occur in iscsi_target_start_negotiation() from iscsi_np
      process context once the failure is detected.
      
      Finally, to handle to case where iscsi_target_sk_state_change() is
      called after the initial PDU procesing is complete, it now invokes
      conn->login_work -> iscsi_target_do_login_rx() to perform cleanup once
      existing iscsi_target_sk_check_close() checks detect connection failure.
      For this case, the cleanup of remaining connection resources will occur
      in iscsi_target_do_login_rx() from delayed workqueue process context
      once the failure is detected.
      Reported-by: default avatarMike Christie <mchristi@redhat.com>
      Reviewed-by: default avatarMike Christie <mchristi@redhat.com>
      Tested-by: default avatarMike Christie <mchristi@redhat.com>
      Cc: Mike Christie <mchristi@redhat.com>
      Reported-by: default avatarHannes Reinecke <hare@suse.com>
      Cc: Hannes Reinecke <hare@suse.com>
      Cc: Sagi Grimberg <sagi@grimberg.me>
      Cc: Varun Prakash <varun@chelsio.com>
      Cc: <stable@vger.kernel.org> # v3.12+
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      25cdda95
  4. 24 May, 2017 1 commit
    • Mike Christie's avatar
      tcmu: fix crash during device removal · f3cdbe39
      Mike Christie authored
      We currently do
      
      tcmu_free_device ->tcmu_netlink_event(TCMU_CMD_REMOVED_DEVICE) ->
      uio_unregister_device -> kfree(tcmu_dev).
      
      The problem is that the kernel does not wait for userspace to
      do the close() on the uio device before freeing the tcmu_dev.
      We can then hit a race where the kernel frees the tcmu_dev before
      userspace does close() and so when close() -> release -> tcmu_release
      is done, we try to access a freed tcmu_dev.
      
      This patch made over the target-pending master branch moves the freeing
      of the tcmu_dev to when the last reference has been dropped.
      
      This also fixes a leak where if tcmu_configure_device was not called on a
      device we did not free udev->name which was allocated at tcmu_alloc_device time.
      Signed-off-by: default avatarMike Christie <mchristi@redhat.com>
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      f3cdbe39
  5. 16 May, 2017 3 commits
    • Nicholas Bellinger's avatar
      target: Re-add check to reject control WRITEs with overflow data · 4ff83daa
      Nicholas Bellinger authored
      During v4.3 when the overflow/underflow check was relaxed by
      commit c72c5250:
      
        commit c72c5250
        Author: Roland Dreier <roland@purestorage.com>
        Date:   Wed Jul 22 15:08:18 2015 -0700
      
             target: allow underflow/overflow for PR OUT etc. commands
      
      to allow underflow/overflow for Windows compliance + FCP, a
      consequence was to allow control CDBs to process overflow
      data for iscsi-target with immediate data as well.
      
      As per Roland's original change, continue to allow underflow
      cases for control CDBs to make Windows compliance + FCP happy,
      but until overflow for control CDBs is supported tree-wide,
      explicitly reject all control WRITEs with overflow following
      pre v4.3.y logic.
      Reported-by: default avatarBart Van Assche <bart.vanassche@sandisk.com>
      Cc: Roland Dreier <roland@purestorage.com>
      Cc: <stable@vger.kernel.org> # v4.3+
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      4ff83daa
    • Bryant G. Ly's avatar
      ibmvscsis: Fix the incorrect req_lim_delta · 75dbf2d3
      Bryant G. Ly authored
      The current code is not correctly calculating the req_lim_delta.
      
      We want to make sure vscsi->credit is always incremented when
      we do not send a response for the scsi op. Thus for the case where
      there is a successfully aborted task we need to make sure the
      vscsi->credit is incremented.
      
      v2 - Moves the original location of the vscsi->credit increment
      to a better spot. Since if we increment credit, the next command
      we send back will have increased req_lim_delta. But we probably
      shouldn't be doing that until the aborted cmd is actually released.
      Otherwise the client will think that it can send a new command, and
      we could find ourselves short of command elements. Not likely, but could
      happen.
      
      This patch depends on both:
      commit 25e78531 ("ibmvscsis: Do not send aborted task response")
      commit 98883f1b ("ibmvscsis: Clear left-over abort_cmd pointers")
      Signed-off-by: default avatarBryant G. Ly <bryantly@linux.vnet.ibm.com>
      Reviewed-by: default avatarMichael Cyr <mikecyr@linux.vnet.ibm.com>
      Cc: <stable@vger.kernel.org> # v4.8+
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      75dbf2d3
    • Bryant G. Ly's avatar
      ibmvscsis: Clear left-over abort_cmd pointers · 98883f1b
      Bryant G. Ly authored
      With the addition of ibmvscsis->abort_cmd pointer within
      commit 25e78531 ("ibmvscsis: Do not send aborted task response"),
      make sure to explicitly NULL these pointers when clearing
      DELAY_SEND flag.
      
      Do this for two cases, when getting the new new ibmvscsis
      descriptor in ibmvscsis_get_free_cmd() and before posting
      the response completion in ibmvscsis_send_messages().
      Signed-off-by: default avatarBryant G. Ly <bryantly@linux.vnet.ibm.com>
      Reviewed-by: default avatarMichael Cyr <mikecyr@linux.vnet.ibm.com>
      Cc: <stable@vger.kernel.org> # v4.8+
      Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
      98883f1b
  6. 13 May, 2017 2 commits