aboutsummaryrefslogtreecommitdiff
path: root/drivers/target
AgeCommit message (Collapse)Author
2013-06-19iscsi-target: fix heap buffer overflow on errorKees Cook
commit cea4dcfdad926a27a18e188720efe0f2c9403456 upstream. If a key was larger than 64 bytes, as checked by iscsi_check_key(), the error response packet, generated by iscsi_add_notunderstood_response(), would still attempt to copy the entire key into the packet, overflowing the structure on the heap. Remote preauthentication kernel memory corruption was possible if a target was configured and listening on the network. CVE-2013-2850 Signed-off-by: Kees Cook <keescook@chromium.org> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2013-05-30iscsi-target: Fix processing of OOO commandsShlomo Pongratz
commit 3eccfdb01da58fbd0f789ae6ca61cee3769e26de upstream. Fix two issues in OOO commands processing done at iscsit_attach_ooo_cmdsn. Handle command serial numbers wrap around by using iscsi_sna_lt and not regular comparisson. The routine iterates until it finds an entry whose serial number is greater than the serial number of the new one, thus the new entry should be inserted before that entry and not after. Signed-off-by: Shlomo Pongratz <shlomop@mellanox.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2013-04-25target: Fix incorrect fallthrough of ALUA Standby/Offline/Transition CDBsNicholas Bellinger
commit 30f359a6f9da65a66de8cadf959f0f4a0d498bba upstream. This patch fixes a bug where a handful of informational / control CDBs that should be allowed during ALUA access state Standby/Offline/Transition where incorrectly returning CHECK_CONDITION + ASCQ_04H_ALUA_TG_PT_*. This includes INQUIRY + REPORT_LUNS, which would end up preventing LUN registration when LUN scanning occured during these ALUA access states. Cc: Hannes Reinecke <hare@suse.de> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2013-04-25target: Fix MAINTENANCE_IN service action CDB checks to use lower 5 bitsNicholas Bellinger
commit ba539743b70cd160c84bab1c82910d0789b820f8 upstream. This patch fixes the MAINTENANCE_IN service action type checks to only look at the proper lower 5 bits of cdb byte 1. This addresses the case where MI_REPORT_TARGET_PGS w/ extended header using the upper three bits of cdb byte 1 was not processed correctly in transport_generic_cmd_sequencer, as well as the three cases for standby, unavailable, and transition ALUA primary access state checks. Also add MAINTENANCE_IN to the excluded list in transport_generic_prepare_cdb() to prevent the PARAMETER DATA FORMAT bits from being cleared. Cc: Hannes Reinecke <hare@suse.de> Cc: Rob Evers <revers@redhat.com> Cc: Christoph Hellwig <hch@lst.de> Cc: Roland Dreier <roland@purestorage.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2013-03-06target: Add missing mapped_lun bounds checking during make_mappedlun setupNicholas Bellinger
commit fbbf8555a986ed31e54f006b6cc637ea4ff1425b upstream. This patch adds missing bounds checking for the configfs provided mapped_lun value during target_fabric_make_mappedlun() setup ahead of se_lun_acl initialization. This addresses a potential OOPs when using a mapped_lun value that exceeds the hardcoded TRANSPORT_MAX_LUNS_PER_TPG-1 value within se_node_acl->device_list[]. Reported-by: Jan Engelhardt <jengelh@inai.de> Cc: Jan Engelhardt <jengelh@inai.de> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2013-03-06target: Fix lookup of dynamic NodeACLs during cached demo-mode operationNicholas Bellinger
commit fcf29481fb8e106daad6688f2e898226ee928992 upstream. This patch fixes a bug in core_tpg_check_initiator_node_acl() -> core_tpg_get_initiator_node_acl() where a dynamically created se_node_acl generated during session login would be skipped during subsequent lookup due to the '!acl->dynamic_node_acl' check, causing a new se_node_acl to be created with a duplicate ->initiatorname. This would occur when a fabric endpoint was configured with TFO->tpg_check_demo_mode()=1 + TPF->tpg_check_demo_mode_cache()=1 preventing the release of an existing se_node_acl during se_session shutdown. Also, drop the unnecessary usage of core_tpg_get_initiator_node_acl() within core_dev_init_initiator_node_lun_acl() that originally required the extra '!acl->dynamic_node_acl' check, and just pass the configfs provided se_node_acl pointer instead. Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> [bwh: Backported to 3.2: adjust context, filename of header] Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2013-02-06tcm_fc: Do not report target role when target is not definedMark Rustad
commit edec8dfefa1f372b2dd8197da555352e76a10c03 upstream. Clear the target role when no target is provided for the node performing a PRLI. Signed-off-by: Mark Rustad <mark.d.rustad@intel.com> Reviewed-by: Bhanu Prakash Gollapudi <bprakash@broadcom.com> Acked by Robert Love <robert.w.love@intel.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2013-02-06tcm_fc: Do not indicate retry capability to initiatorsMark Rustad
commit f2eeba214bcd0215b7f558cab6420e5fd153042b upstream. When generating a PRLI response to an initiator, clear the FCP_SPPF_RETRY bit in the response. Signed-off-by: Mark Rustad <mark.d.rustad@intel.com> Reviewed-by: Bhanu Prakash Gollapudi <bprakash@broadcom.com> Acked by Robert Love <robert.w.love@intel.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2013-02-06target: use correct sense code for LUN communication failureHannes Reinecke
commit 18a9df42d53fabfa43b78be1104838cc8b9762e1 upstream. The ASC/ASCQ code for 'Logical Unit Communication failure' is 0x08/0x00; 0x80/0x00 is vendor specific. Signed-off-by: Hannes Reinecke <hare@suse.de> Cc: Nicholas Bellinger <nab@risingtidesystems.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> [bwh: Backported to 3.2: add offset to buffer index] Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2013-01-03target/tcm_fc: fix the lockdep warning due to inconsistent lock stateYi Zou
commit 9f4ad44b264f8bb61ffdd607148215566568430d upstream. The lockdep warning below is in theory correct but it will be in really weird rare situation that ends up that deadlock since the tcm fc session is hashed based the rport id. Nonetheless, the complaining below is about rcu callback that does the transport_deregister_session() is happening in softirq, where transport_register_session() that happens earlier is not. This triggers the lockdep warning below. So, just fix this to make lockdep happy by disabling the soft irq before calling transport_register_session() in ft_prli. BTW, this was found in FCoE VN2VN over two VMs, couple of create and destroy would get this triggered. v1: was enforcing register to be in softirq context which was not righ. See, http://www.spinics.net/lists/target-devel/msg03614.html v2: following comments from Roland&Nick (thanks), it seems we don't have to do transport_deregister_session() in rcu callback, so move it into ft_sess_free() but still do kfree() of the corresponding ft_sess struct in rcu callback to make sure the ft_sess is not freed till the rcu callback. ... [ 1328.370592] scsi2 : FCoE Driver [ 1328.383429] fcoe: No FDMI support. [ 1328.384509] host2: libfc: Link up on port (000000) [ 1328.934229] host2: Assigned Port ID 00a292 [ 1357.232132] host2: rport 00a393: Remove port [ 1357.232568] host2: rport 00a393: Port sending LOGO from Ready state [ 1357.233692] host2: rport 00a393: Delete port [ 1357.234472] host2: rport 00a393: work event 3 [ 1357.234969] host2: rport 00a393: callback ev 3 [ 1357.235979] host2: rport 00a393: Received a LOGO response closed [ 1357.236706] host2: rport 00a393: work delete [ 1357.237481] [ 1357.237631] ================================= [ 1357.238064] [ INFO: inconsistent lock state ] [ 1357.238450] 3.7.0-rc7-yikvm+ #3 Tainted: G O [ 1357.238450] --------------------------------- [ 1357.238450] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage. [ 1357.238450] ksoftirqd/0/3 [HC0[0]:SC1[1]:HE0:SE0] takes: [ 1357.238450] (&(&se_tpg->session_lock)->rlock){+.?...}, at: [<ffffffffa01eacd4>] transport_deregister_session+0x41/0x148 [target_core_mod] [ 1357.238450] {SOFTIRQ-ON-W} state was registered at: [ 1357.238450] [<ffffffff810834f5>] mark_held_locks+0x6d/0x95 [ 1357.238450] [<ffffffff8108364a>] trace_hardirqs_on_caller+0x12d/0x197 [ 1357.238450] [<ffffffff810836c1>] trace_hardirqs_on+0xd/0xf [ 1357.238450] [<ffffffff8149caba>] _raw_spin_unlock_irq+0x2d/0x45 [ 1357.238450] [<ffffffffa01e8d10>] __transport_register_session+0xb8/0x122 [target_core_mod] [ 1357.238450] [<ffffffffa01e8dbe>] transport_register_session+0x44/0x5a [target_core_mod] [ 1357.238450] [<ffffffffa018e32c>] ft_prli+0x1e3/0x275 [tcm_fc] [ 1357.238450] [<ffffffffa0160e8d>] fc_rport_recv_req+0x95e/0xdc5 [libfc] [ 1357.238450] [<ffffffffa015be88>] fc_lport_recv_els_req+0xc4/0xd5 [libfc] [ 1357.238450] [<ffffffffa015c778>] fc_lport_recv_req+0x12f/0x18f [libfc] [ 1357.238450] [<ffffffffa015a6d7>] fc_exch_recv+0x8ba/0x981 [libfc] [ 1357.238450] [<ffffffffa0176d7a>] fcoe_percpu_receive_thread+0x47a/0x4e2 [fcoe] [ 1357.238450] [<ffffffff810549f1>] kthread+0xb1/0xb9 [ 1357.238450] [<ffffffff814a40ec>] ret_from_fork+0x7c/0xb0 [ 1357.238450] irq event stamp: 275411 [ 1357.238450] hardirqs last enabled at (275410): [<ffffffff810bb6a0>] rcu_process_callbacks+0x229/0x42a [ 1357.238450] hardirqs last disabled at (275411): [<ffffffff8149c2f7>] _raw_spin_lock_irqsave+0x22/0x8e [ 1357.238450] softirqs last enabled at (275394): [<ffffffff8103d669>] __do_softirq+0x246/0x26f [ 1357.238450] softirqs last disabled at (275399): [<ffffffff8103d6bb>] run_ksoftirqd+0x29/0x62 [ 1357.238450] [ 1357.238450] other info that might help us debug this: [ 1357.238450] Possible unsafe locking scenario: [ 1357.238450] [ 1357.238450] CPU0 [ 1357.238450] ---- [ 1357.238450] lock(&(&se_tpg->session_lock)->rlock); [ 1357.238450] <Interrupt> [ 1357.238450] lock(&(&se_tpg->session_lock)->rlock); [ 1357.238450] [ 1357.238450] *** DEADLOCK *** [ 1357.238450] [ 1357.238450] no locks held by ksoftirqd/0/3. [ 1357.238450] [ 1357.238450] stack backtrace: [ 1357.238450] Pid: 3, comm: ksoftirqd/0 Tainted: G O 3.7.0-rc7-yikvm+ #3 [ 1357.238450] Call Trace: [ 1357.238450] [<ffffffff8149399a>] print_usage_bug+0x1f5/0x206 [ 1357.238450] [<ffffffff8100da59>] ? save_stack_trace+0x2c/0x49 [ 1357.238450] [<ffffffff81082aae>] ? print_irq_inversion_bug.part.14+0x1ae/0x1ae [ 1357.238450] [<ffffffff81083336>] mark_lock+0x106/0x258 [ 1357.238450] [<ffffffff81084e34>] __lock_acquire+0x2e7/0xe53 [ 1357.238450] [<ffffffff8102903d>] ? pvclock_clocksource_read+0x48/0xb4 [ 1357.238450] [<ffffffff810ba6a3>] ? rcu_process_gp_end+0xc0/0xc9 [ 1357.238450] [<ffffffffa01eacd4>] ? transport_deregister_session+0x41/0x148 [target_core_mod] [ 1357.238450] [<ffffffff81085ef1>] lock_acquire+0x119/0x143 [ 1357.238450] [<ffffffffa01eacd4>] ? transport_deregister_session+0x41/0x148 [target_core_mod] [ 1357.238450] [<ffffffff8149c329>] _raw_spin_lock_irqsave+0x54/0x8e [ 1357.238450] [<ffffffffa01eacd4>] ? transport_deregister_session+0x41/0x148 [target_core_mod] [ 1357.238450] [<ffffffffa01eacd4>] transport_deregister_session+0x41/0x148 [target_core_mod] [ 1357.238450] [<ffffffff810bb6a0>] ? rcu_process_callbacks+0x229/0x42a [ 1357.238450] [<ffffffffa018ddc5>] ft_sess_rcu_free+0x17/0x24 [tcm_fc] [ 1357.238450] [<ffffffffa018ddae>] ? ft_sess_free+0x1b/0x1b [tcm_fc] [ 1357.238450] [<ffffffff810bb6d7>] rcu_process_callbacks+0x260/0x42a [ 1357.238450] [<ffffffff8103d55d>] __do_softirq+0x13a/0x26f [ 1357.238450] [<ffffffff8149b34e>] ? __schedule+0x65f/0x68e [ 1357.238450] [<ffffffff8103d6bb>] run_ksoftirqd+0x29/0x62 [ 1357.238450] [<ffffffff8105c83c>] smpboot_thread_fn+0x1a5/0x1aa [ 1357.238450] [<ffffffff8105c697>] ? smpboot_unregister_percpu_thread+0x47/0x47 [ 1357.238450] [<ffffffff810549f1>] kthread+0xb1/0xb9 [ 1357.238450] [<ffffffff8149b49d>] ? wait_for_common+0xbb/0x10a [ 1357.238450] [<ffffffff81054940>] ? __init_kthread_worker+0x59/0x59 [ 1357.238450] [<ffffffff814a40ec>] ret_from_fork+0x7c/0xb0 [ 1357.238450] [<ffffffff81054940>] ? __init_kthread_worker+0x59/0x59 [ 1417.440099] rport-2:0-0: blocked FC remote port time out: removing rport Signed-off-by: Yi Zou <yi.zou@intel.com> Cc: Open-FCoE <devel@open-fcoe.org> Cc: Nicholas A. Bellinger <nab@risingtidesystems.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2013-01-03target/file: Fix 32-bit highmem breakage for SGL -> iovec mappingSebastian Andrzej Siewior
commit 40ff2c3b3da35dd3a00ac6722056a59b4b3f2caf upstream. This patch changes vectored file I/O to use kmap + kunmap when mapping incoming SGL memory -> struct iovec in order to properly support 32-bit highmem configurations. This is because an extra bounce buffer may be required when processing scatterlist pages allocated with GFP_KERNEL. Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> [bwh: Backported to 3.2: use task->task_sg{,_nents} for iteration] Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2013-01-03iscsit: use GFP_ATOMIC under spin lockWei Yongjun
commit 3c989d7603872bf878840f7ce3ea49b73bea4c6c upstream. The function iscsit_build_conn_drop_async_message() is called from iscsit_close_connection() with spin lock 'sess->conn_lock' held, so we should use GFP_ATOMIC instead of GFP_KERNEL. Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2013-01-03iscsi-target: Always send a response before terminating iSCSI connectionRoland Dreier
commit 1c5c12c666fda27c7c494b34934a0a0631a48130 upstream. There are some cases, for example when the initiator sends an out-of-bounds ErrorRecoveryLevel value, where the iSCSI target terminates the connection without sending back any error. Audit the login path and add appropriate iscsit_tx_login_rsp() calls to make sure this doesn't happen. Signed-off-by: Roland Dreier <roland@purestorage.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> [bwh: Backported to 3.2: adjust context] Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-11-16iscsi-target: Fix missed wakeup race in TX threadRoland Dreier
commit d5627acba9ae584cf4928af19f7ddf5f6837de32 upstream. The sleeping code in iscsi_target_tx_thread() is susceptible to the classic missed wakeup race: - TX thread finishes handle_immediate_queue() and handle_response_queue(), thinks both queues are empty. - Another thread adds a queue entry and does wake_up_process(), which does nothing because the TX thread is still awake. - TX thread does schedule_timeout() and sleeps forever. In practice this can kill an iSCSI connection if for example an initiator does single-threaded writes and the target misses the wakeup window when queueing an R2T; in this case the connection will be stuck until the initiator loses patience and does some task management operation (or kills the connection entirely). Fix this by converting to wait_event_interruptible(), which does not suffer from this sort of race. Signed-off-by: Roland Dreier <roland@purestorage.com> Cc: Andy Grover <agrover@redhat.com> Cc: Hannes Reinecke <hare@suse.de> Cc: Christoph Hellwig <hch@lst.de> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-11-16target: Avoid integer overflow in se_dev_align_max_sectors()Roland Dreier
commit 3e03989b5868acf69a391a424dc71fcd6cc48167 upstream. The expression (max_sectors * block_size) might overflow a u32 (indeed, since iblock sets max_hw_sectors to UINT_MAX, it is guaranteed to overflow and end up with a much-too-small result in many common cases). Fix this by doing an equivalent calculation that doesn't require multiplication. While we're touching this code, avoid splitting a printk format across two lines and use pr_info(...) instead of printk(KERN_INFO ...). Signed-off-by: Roland Dreier <roland@purestorage.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-11-16target: Don't return success from module_init() if setup failsRoland Dreier
commit 0d0f9dfb31e0a6c92063e235417b42df185b3275 upstream. If the call to core_dev_release_virtual_lun0() fails, then nothing sets ret to anything other than 0, so even though everything is torn down and freed, target_core_init_configfs() will seem to succeed and the module will be loaded. Fix this by passing the return value on up the chain. Signed-off-by: Roland Dreier <roland@purestorage.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-10-17iscsi-target: Bump defaults for nopin_timeout + nopin_response_timeout valuesNicholas Bellinger
commit cf0eb28d3ba60098865bf7dbcbfdd6b1cc483e3b upstream. This patch increases the default for nopin_timeout to 15 seconds (wait between sending a new NopIN ping) and nopin_response_timeout to 30 seconds (wait for NopOUT response before failing the connection) in order to avoid false positives by iSCSI Initiators who are not always able (under load) to respond to NopIN echo PING requests within the current 5 second window. False positives have been observed recently using Open-iSCSI code on v3.3.x with heavy large-block READ workloads over small MTU 1 Gb/sec ports, and increasing these values to more reasonable defaults significantly reduces the possibility of false positive NopIN response timeout events under this specific workload. Historically these have been set low to initiate connection recovery as soon as possible if we don't hear a ping back, but for modern v3.x code on 1 -> 10 Gb/sec ports these new defaults make alot more sense. Cc: Christoph Hellwig <hch@lst.de> Cc: Andy Grover <agrover@redhat.com> Cc: Mike Christie <michaelc@cs.wisc.edu> Cc: Hannes Reinecke <hare@suse.de> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-10-17iscsit: remove incorrect unlock in iscsit_build_sendtargets_respChristoph Hellwig
commit 904753da183566c71211d23c169a80184648c121 upstream. Fix a potential multiple spin-unlock -> deadlock scenario during the overflow check within iscsit_build_sendtargets_resp() as found by sparse static checking. Signed-off-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-10-17iscsi-target: Add explicit set of cache_dynamic_acls=1 for TPG demo-modeNicholas Bellinger
commit 38b11bae6ba02da352340aff12ee25755977b222 upstream. We've had reports in the past about this specific case, so it's time to go ahead and explicitly set cache_dynamic_acls=1 for generate_node_acls=1 (TPG demo-mode) operation. During normal generate_node_acls=0 operation with explicit NodeACLs -> se_node_acl memory is persistent to the configfs group located at /sys/kernel/config/target/$TARGETNAME/$TPGT/acls/$INITIATORNAME, so in the generate_node_acls=1 case we want the reservation logic to reference existing per initiator IQN se_node_acl memory (not to generate a new se_node_acl), so go ahead and always set cache_dynamic_acls=1 when TPG demo-mode is enabled. Reported-by: Ronnie Sahlberg <ronniesahlberg@gmail.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-10-17target/file: Re-enable optional fd_buffered_io=1 operationNicholas Bellinger
commit b32f4c7ed85c5cee2a21a55c9f59ebc9d57a2463 upstream. This patch re-adds the ability to optionally run in buffered FILEIO mode (eg: w/o O_DSYNC) for device backends in order to once again use the Linux buffered cache as a write-back storage mechanism. This logic was originally dropped with mainline v3.5-rc commit: commit a4dff3043c231d57f982af635c9d2192ee40e5ae Author: Nicholas Bellinger <nab@linux-iscsi.org> Date: Wed May 30 16:25:41 2012 -0700 target/file: Use O_DSYNC by default for FILEIO backends This difference with this patch is that fd_create_virtdevice() now forces the explicit setting of emulate_write_cache=1 when buffered FILEIO operation has been enabled. (v2: Switch to FDBD_HAS_BUFFERED_IO_WCE + add more detailed comment as requested by hch) Reported-by: Ferry <iscsitmp@bananateam.nl> Cc: Christoph Hellwig <hch@lst.de> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-10-17iscsi-target: Correctly set 0xffffffff field within ISCSI_OP_REJECT PDUNicholas Bellinger
commit f25590f39d543272f7ae7b00d533359c8d7ff331 upstream. This patch adds a missing iscsi_reject->ffffffff assignment within iscsit_send_reject() code to properly follow RFC-3720 Section 10.17 Bytes 16 -> 19 for the PDU format definition of ISCSI_OP_REJECT. We've not seen any initiators care about this bytes in practice, but as Ronnie reported this was causing trouble with wireshark packet decoding lets go ahead and fix this up now. Reported-by: Ronnie Sahlberg <ronniesahlberg@gmail.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-10-17target: fix return code in target_core_init_configfs error pathPeter Senna Tschudin
commit 37bb7899ca366dc212b71b150e78566d04808cc0 upstream. This patch fixes error cases within target_core_init_configfs() to properly set ret = -ENOMEM before jumping to the out_global exception path. This was originally discovered with the following Coccinelle semantic match information: Convert a nonnegative error return code to a negative one, as returned elsewhere in the function. A simplified version of the semantic match that finds this problem is as follows: (http://coccinelle.lip6.fr/) // <smpl> ( if@p1 (\(ret < 0\|ret != 0\)) { ... return ret; } | ret@p1 = 0 ) ... when != ret = e1 when != &ret *if(...) { ... when != ret = e2 when forall return ret; } // </smpl> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-10-10target: Fix ->data_length re-assignment bug with SCSI overflowNicholas Bellinger
commit 4c054ba63ad47ef244cfcfa1cea38134620a5bae upstream. 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> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-09-12target: fix NULL pointer dereference bug alloc_page() fails to get memoryYi Zou
commit d0e27c88d795fb9647153063ec48051fd84e1731 upstream. 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> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-08-02iscsi-target: Drop bogus struct file usage for iSCSI/SCTPAl Viro
commit bf6932f44a7b3fa7e2246a8b18a44670e5eab6c2 upstream. 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> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> [bwh: Backported to 3.2: adjust context] Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-08-02target: Check number of unmap descriptors against our limitRoland Dreier
commit 7409a6657aebf8be74c21d0eded80709b27275cb upstream. Fail UNMAP commands that have more than our reported limit on unmap descriptors. Signed-off-by: Roland Dreier <roland@purestorage.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> [bwh: Backported to 3.2: adjust filename] Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-08-02target: Fix possible integer underflow in UNMAP emulationRoland Dreier
commit b7fc7f3777582dea85156a821d78a522a0c083aa upstream. 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> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> [bwh: Backported to 3.2: adjust filename, context] Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-08-02target: Fix reading of data length fields for UNMAP commandsRoland Dreier
commit 1a5fa4576ec8a462313c7516b31d7453481ddbe8 upstream. 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> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> [bwh: Backported to 3.2: adjust filename, context] Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-08-02target: Add range checking to UNMAP emulationRoland Dreier
commit 2594e29865c291db162313187612cd9f14538f33 upstream. 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> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> [bwh: Backported to 3.2: adjust filename, context] Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-08-02target: Add generation of LOGICAL BLOCK ADDRESS OUT OF RANGERoland Dreier
commit e2397c704429025bc6b331a970f699e52f34283e upstream. 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> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-07-25target: Fix range calculation in WRITE SAME emulation when num blocks == 0Roland Dreier
commit 1765fe5edcb83f53fc67edeb559fcf4bc82c6460 upstream. When NUMBER OF LOGICAL BLOCKS is 0, WRITE SAME is supposed to write all the blocks from the specified LBA through the end of the device. However, dev->transport->get_blocks(dev) (perhaps confusingly) returns the last valid LBA rather than the number of blocks, so the correct number of blocks to write starting with lba is dev->transport->get_blocks(dev) - lba + 1 (nab: Backport roland's for-3.6 patch to for-3.5) Signed-off-by: Roland Dreier <roland@purestorage.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-07-25target: Clean up returning errors in PR handling codeRoland Dreier
commit d35212f3ca3bf4fb49d15e37f530c9931e2d2183 upstream. - instead of (PTR_ERR(file) < 0) just use IS_ERR(file) - return -EINVAL instead of EINVAL - all other error returns in target_scsi3_emulate_pr_out() use "goto out" -- get rid of the one remaining straight "return." Signed-off-by: Roland Dreier <roland@purestorage.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-07-25tcm_fc: Fix crash seen with aborts and large readsMark Rustad
commit 3cc5d2a6b9a2fd1bf024aa5e52dd22961eecaf13 upstream. This patch fixes a crash seen when large reads have their exchange aborted by either timing out or being reset. Because the exchange abort results in the seq pointer being set to NULL, because the sequence is no longer valid, it must not be dereferenced. This patch changes the function ft_get_task_tag to return ~0 if it is unable to get the tag for this reason. Because the get_task_tag interface provides no means of returning an error, this seems like the best way to fix this issue at the moment. Signed-off-by: Mark Rustad <mark.d.rustad@intel.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-07-12tcm_fc: Resolve suspicious RCU usage warningsMark Rustad
commit 863555be0c81558b1af277addcf68acb8f778860 upstream. Use rcu_dereference_protected to tell rcu that the ft_lport_lock is held during ft_lport_create. This resolved "suspicious RCU usage" warnings when debugging options are turned on. Signed-off-by: Mark Rustad <mark.d.rustad@intel.com> Tested-by: Ross Brattain <ross.b.brattain@intel.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-06-19target: Return error to initiator if SET TARGET PORT GROUPS emulation failsRoland Dreier
commit 59e4f541baf728dbb426949bfa9f6862387ffd0e upstream. The error paths in target_emulate_set_target_port_groups() are all essentially "rc = -EINVAL; goto out;" but the code at "out:" ignores rc and always returns success. This means that even if eg explicit ALUA is turned off, the initiator will always see a good SCSI status for SET TARGET PORT GROUPS. Fix this by returning rc as is intended. It appears this bug was added by the following patch: commit 05d1c7c0d0db4cc25548d9aadebb416888a82327 Author: Andy Grover <agrover@redhat.com> Date: Wed Jul 20 19:13:28 2011 +0000 target: Make all control CDBs scatter-gather Signed-off-by: Roland Dreier <roland@purestorage.com> Cc: Andy Grover <agrover@redhat.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> [bwh: Backported to 3.2: we have transport_complete_task() and not target_complete_cmd()] Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-06-10target/file: Use O_DSYNC by default for FILEIO backendsNicholas Bellinger
commit a4dff3043c231d57f982af635c9d2192ee40e5ae upstream. Convert to use O_DSYNC for all cases at FILEIO backend creation time to avoid the extra syncing of pure timestamp updates with legacy O_SYNC during default operation as recommended by hch. Continue to do this independently of Write Cache Enable (WCE) bit, as WCE=0 is currently the default for all backend devices and enabled by user on per device basis via attrib/emulate_write_cache. This patch drops the now unnecessary fd_buffered_io= token usage that was originally signalling when to explictly disable O_SYNC at backend creation time for buffered I/O operation. This can end up being dangerous for a number of reasons during physical node failure, so go ahead and drop this option for now when O_DSYNC is used as the default. Also allow explict FUA WRITEs -> vfs_fsync_range() call to function in fd_execute_cmd() independently of WCE bit setting. Reported-by: Christoph Hellwig <hch@lst.de> Cc: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk> [bwh: Backported to 3.2: - We have fd_do_task() and not fd_execute_cmd() - Various fields are in struct se_task rather than struct se_cmd - fd_create_virtdevice() flags initialisation hasn't been cleaned up]
2012-05-20target: Fix bug in handling of FILEIO + block_device resize opsNicholas Bellinger
commit cd9323fd68aee3c1c6b5b21e5746c9d1b586fb58 upstream. This patch fixes a bug in the handling of FILEIO w/ underlying block_device resize operations where the original fd_dev->fd_dev_size was incorrectly being used in fd_get_blocks() for READ_CAPACITY response payloads. This patch avoids using fd_dev->fd_dev_size for FILEIO devices with an underlying block_device, and instead changes fd_get_blocks() to get the sector count directly from i_size_read() as recommended by hch. Reported-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-05-20target: Fix SPC-2 RELEASE bug for multi-session iSCSI client setupsBernhard Kohl
commit edc318d9fea6854df752ec8c645b960b0d5a1d23 upstream. This patch addresses a bug in a special case for target core SPC-2 RELEASE logic where the same physical client (eg: iSCSI InitiatorName) with differing iSCSI session identifiers (ISID) is allowed to incorrectly release the same client's SPC-2 reservation from the non reservation holding path. Note this bug is specific to iscsi-target w/ SPC-2 reservations, and with the default enforce_pr_isids=1 device attr setting in target-core controls if a InitiatorName + different ISID reservations are handled the same as a single iSCSI client entity. Signed-off-by: Bernhard Kohl <bernhard.kohl@gmx.net> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-05-20target: Drop incorrect se_lun_acl release for dynamic -> explict ACL conversionNicholas Bellinger
commit cfebf8f42f47327fa54cf05c19b98f4bd5236a26 upstream. This patch removes some potentially problematic legacy code within core_clear_initiator_node_from_tpg() that was originally intended to release left over se_lun_acl setup during dynamic NodeACL+MappedLUN generate when running with TPG demo-mode operation. Since we now only ever expect to allocate and release se_lun_acl from within target_core_fabric_configfs.c:target_fabric_make_mappedlun() and target_fabric_drop_mappedlun() context respectively, this code for demo-mode release is incorrect and needs to be removed. Cc: Christoph Hellwig <hch@lst.de> Cc: Andy Grover <agrover@redhat.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
2012-04-13tcm_fc: Do not free tpg structure during wq allocation failureMark Rustad
commit 06383f10c49f507220594a455c6491ca6f8c94ab upstream. Avoid freeing a registered tpg structure if an alloc_workqueue call fails. This fixes a bug where the failure was leaking memory associated with se_portal_group setup during the original core_tpg_register() call. Signed-off-by: Mark Rustad <mark.d.rustad@intel.com> Acked-by: Kiran Patil <Kiran.patil@intel.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-04-13tcm_fc: Add abort flag for gracefully handling exchange timeoutMark Rustad
commit e1c4038282c7586c3544542b37872c434669d3ac upstream. Add abort flag and use it to terminate processing when an exchange is timed out or is reset. The abort flag is used in place of the transport_generic_free_cmd function call in the reset and timeout cases, because calling that function in that context would free memory that was in use. The aborted flag allows the lifetime to be managed in a more normal way, while truncating the processing. This change eliminates a source of memory corruption which manifested in a variety of ugly ways. (nab: Drop unused struct fc_exch *ep in ft_recv_seq) Signed-off-by: Mark Rustad <mark.d.rustad@intel.com> Acked-by: Kiran Patil <Kiran.patil@intel.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-04-13target: Fix unsupported WRITE_SAME sense payloadMartin Svec
commit 67236c44741e250199ccd77f1115568e68cf8848 upstream. This patch fixes a bug in target-core where unsupported WRITE_SAME ops from a target_check_write_same_discard() failure was incorrectly returning CHECK_CONDITION w/ TCM_INVALID_CDB_FIELD sense data. This was causing some clients to not properly fall back, so go ahead and use the correct TCM_UNSUPPORTED_SCSI_OPCODE sense for this case. Reported-by: Martin Svec <martin.svec@zoner.cz> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-04-02target: Fix 16-bit target ports for SET TARGET PORT GROUPS emulationRoland Dreier
commit 33395fb8a13731c7ef7b175dbf5a4d8a6738fe6c upstream. The old code did (MSB << 8) & 0xff, which always evaluates to 0. Just use get_unaligned_be16() so we don't have to worry about whether our open-coded version is correct or not. Signed-off-by: Roland Dreier <roland@purestorage.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-04-02target: prevent NULL pointer dereference in target_report_lunsJörn Engel
commit 47f1b8803e1e358ebbf4f82bfdb98971c912a2c3 upstream. transport_kmap_data_sg can return NULL. I never saw this trigger, but returning -ENOMEM seems better than a crash. Also removes a pointless case while at it. Signed-off-by: Joern Engel <joern@logfs.org> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-04-02target: fix use after free in target_report_lunsJörn Engel
commit 382436f8804fe1cb20b9a2a811a10eb2d8554721 upstream. Fix possible NULL pointer dereference in target_report_luns failure path. Signed-off-by: Joern Engel <joern@logfs.org> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-04-02target: Don't set WBUS16 or SYNC bits in INQUIRY responseRoland Dreier
commit effc6cc8828257c32c37635e737f14fd6e19ecd7 upstream. SPC-4 says about the WBUS16 and SYNC bits: The meanings of these fields are specific to SPI-5 (see 6.4.3). For SCSI transport protocols other than the SCSI Parallel Interface, these fields are reserved. We don't have a SPI fabric module, so we should never set these bits. (The comment was misleading, since it only mentioned Sync but the actual code set WBUS16 too). Signed-off-by: Roland Dreier <roland@purestorage.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-04-02tcm_fc: Fix fc_exch memory leak in ft_send_resp_statusNicholas Bellinger
commit 031ed4d565b31880a4136bb7366bc89f5b1dba7d upstream. This patch fixes a bug in tcm_fc where fc_exch memory from fc_exch_mgr->ep_pool is currently being leaked by ft_send_resp_status() usage. Following current code in ft_queue_status() response path, using lport->tt.seq_send() needs to be followed by a lport->tt.exch_done() in order to release fc_exch memory back into libfc_em kmem_cache. ft_send_resp_status() code is currently used in pre submit se_cmd ft_send_work() error exceptions, TM request setup exceptions, and main TM response callback path in ft_queue_tm_resp(). This bugfix addresses the leak in these cases. Cc: Mark D Rustad <mark.d.rustad@intel.com> Cc: Kiran Patil <kiran.patil@intel.com> Cc: Robert Love <robert.w.love@intel.com> Cc: Andy Grover <agrover@redhat.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-04-02iscsi-target: Fix dynamic -> explict NodeACL pointer referenceNicholas Bellinger
commit d06283341aee9e48eff1b068779d340785c635ce upstream. This patch fixes a free after use in lio_target_make_nodeacl() where iscsi_node_acl was referenced from the original se_nacl_new allocation, instead of from core_tpg_add_initiator_node_acl() in the case of dynamic -> explict NodeACL conversion. Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-04-02iscsi-target: Fix iscsit_alloc_buffs() failure casesNicholas Bellinger
commit d335e6054db616bce3f040e659fa38440518ad1d upstream. Make iscsit_alloc_buffs() failure case for page_alloc_failed use correct __free_page() SGL pointer, and return -ENOMEM for iscsit_allocate_iovecs failure to push se_cmd->t_mem_sg release into iscsit_release_cmd() callback during iscsit_add_reject_from_cmd() connection reset. Also drop cmd->t_mem_sg = NULL assignment from page_alloc_failed failure case. Reported-by: Roland Dreier <roland@purestorage.com> Cc: Andy Grover <agrover@redhat.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2012-04-02tcm_loop: Set residual field for SCSI commandsRoland Dreier
commit 6cf3fa6918baab0c447f1206f1cef9166ad04864 upstream. If the target core signals an over- or under-run, tcm_loop should call scsi_set_resid() to tell the SCSI midlayer about the residual data length. The difference can be seen by doing something like strace -eioctl sg_raw -r 1024 /dev/sda 8 0 0 0 1 0 > /dev/null and looking at the "resid=" part of the SG_IO ioctl -- after this patch, the field is correctly reported as 512. Signed-off-by: Roland Dreier <roland@purestorage.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>