Skip to content

Commit 55aac6e

Browse files
committed
Merge git://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending
Pull SCSI target fixes from Nicholas Bellinger: "This target series for v4.10 contains fixes which address a few long-standing bugs that DATERA's QA + automation teams have uncovered while putting v4.1.y target code into production usage. We've been running the top three in our nightly automated regression runs for the last two months, and the COMPARE_AND_WRITE fix Mr. Gary Guo has been manually verifying against a four node ESX cluster this past week. Note all of them have CC' stable tags. Summary: - Fix a bug with ESX EXTENDED_COPY + SAM_STAT_RESERVATION_CONFLICT status, where target_core_xcopy.c logic was incorrectly returning SAM_STAT_CHECK_CONDITION for all non SAM_STAT_GOOD cases (Nixon Vincent) - Fix a TMR LUN_RESET hung task bug while other in-flight TMRs are being aborted, before the new one had been dispatched into tmr_wq (Rob Millner) - Fix a long standing double free OOPs, where a dynamically generated 'demo-mode' NodeACL has multiple sessions associated with it, and the /sys/kernel/config/target/$FABRIC/$WWN/ subsequently disables demo-mode, but never converts the dynamic ACL into a explicit ACL (Rob Millner) - Fix a long standing reference leak with ESX VAAI COMPARE_AND_WRITE when the second phase WRITE COMMIT command fails, resulting in CHECK_CONDITION response never being sent and se_cmd->cmd_kref never reaching zero (Gary Guo) Beyond these items on v4.1.y we've reproduced, fixed, and run through our regression test suite using iscsi-target exports, there are two additional outstanding list items: - Remove a >= v4.2 RCU conversion BUG_ON that would trigger when dynamic node NodeACLs where being converted to explicit NodeACLs. The patch drops the BUG_ON to follow how pre RCU conversion worked for this special case (Benjamin Estrabaud) - Add ibmvscsis target_core_fabric_ops->max_data_sg_nent assignment to match what IBM's Virtual SCSI hypervisor is already enforcing at transport layer. (Bryant Ly + Steven Royer)" * git://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending: ibmvscsis: Add SGL limit target: Fix COMPARE_AND_WRITE ref leak for non GOOD status target: Fix multi-session dynamic se_node_acl double free OOPs target: Fix early transport_generic_handle_tmr abort scenario target: Use correct SCSI status during EXTENDED_COPY exception target: Don't BUG_ON during NodeACL dynamic -> explicit conversion
2 parents 2b36947 + b22bc27 commit 55aac6e

File tree

6 files changed

+76
-32
lines changed

6 files changed

+76
-32
lines changed

drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3816,6 +3816,7 @@ static struct configfs_attribute *ibmvscsis_tpg_attrs[] = {
38163816
static const struct target_core_fabric_ops ibmvscsis_ops = {
38173817
.module = THIS_MODULE,
38183818
.name = "ibmvscsis",
3819+
.max_data_sg_nents = MAX_TXU / PAGE_SIZE,
38193820
.get_fabric_name = ibmvscsis_get_fabric_name,
38203821
.tpg_get_wwn = ibmvscsis_get_fabric_wwn,
38213822
.tpg_get_tag = ibmvscsis_get_tag,

drivers/target/target_core_device.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,15 @@ int core_enable_device_list_for_node(
352352
kfree(new);
353353
return -EINVAL;
354354
}
355-
BUG_ON(orig->se_lun_acl != NULL);
355+
if (orig->se_lun_acl != NULL) {
356+
pr_warn_ratelimited("Detected existing explicit"
357+
" se_lun_acl->se_lun_group reference for %s"
358+
" mapped_lun: %llu, failing\n",
359+
nacl->initiatorname, mapped_lun);
360+
mutex_unlock(&nacl->lun_entry_mutex);
361+
kfree(new);
362+
return -EINVAL;
363+
}
356364

357365
rcu_assign_pointer(new->se_lun, lun);
358366
rcu_assign_pointer(new->se_lun_acl, lun_acl);

drivers/target/target_core_sbc.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -451,16 +451,20 @@ static sense_reason_t compare_and_write_post(struct se_cmd *cmd, bool success,
451451
int *post_ret)
452452
{
453453
struct se_device *dev = cmd->se_dev;
454+
sense_reason_t ret = TCM_NO_SENSE;
454455

455456
/*
456457
* Only set SCF_COMPARE_AND_WRITE_POST to force a response fall-through
457458
* within target_complete_ok_work() if the command was successfully
458459
* sent to the backend driver.
459460
*/
460461
spin_lock_irq(&cmd->t_state_lock);
461-
if ((cmd->transport_state & CMD_T_SENT) && !cmd->scsi_status) {
462+
if (cmd->transport_state & CMD_T_SENT) {
462463
cmd->se_cmd_flags |= SCF_COMPARE_AND_WRITE_POST;
463464
*post_ret = 1;
465+
466+
if (cmd->scsi_status == SAM_STAT_CHECK_CONDITION)
467+
ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
464468
}
465469
spin_unlock_irq(&cmd->t_state_lock);
466470

@@ -470,7 +474,7 @@ static sense_reason_t compare_and_write_post(struct se_cmd *cmd, bool success,
470474
*/
471475
up(&dev->caw_sem);
472476

473-
return TCM_NO_SENSE;
477+
return ret;
474478
}
475479

476480
static sense_reason_t compare_and_write_callback(struct se_cmd *cmd, bool success,

drivers/target/target_core_transport.c

Lines changed: 58 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -457,8 +457,20 @@ static void target_complete_nacl(struct kref *kref)
457457
{
458458
struct se_node_acl *nacl = container_of(kref,
459459
struct se_node_acl, acl_kref);
460+
struct se_portal_group *se_tpg = nacl->se_tpg;
460461

461-
complete(&nacl->acl_free_comp);
462+
if (!nacl->dynamic_stop) {
463+
complete(&nacl->acl_free_comp);
464+
return;
465+
}
466+
467+
mutex_lock(&se_tpg->acl_node_mutex);
468+
list_del(&nacl->acl_list);
469+
mutex_unlock(&se_tpg->acl_node_mutex);
470+
471+
core_tpg_wait_for_nacl_pr_ref(nacl);
472+
core_free_device_list_for_node(nacl, se_tpg);
473+
kfree(nacl);
462474
}
463475

464476
void target_put_nacl(struct se_node_acl *nacl)
@@ -499,12 +511,39 @@ EXPORT_SYMBOL(transport_deregister_session_configfs);
499511
void transport_free_session(struct se_session *se_sess)
500512
{
501513
struct se_node_acl *se_nacl = se_sess->se_node_acl;
514+
502515
/*
503516
* Drop the se_node_acl->nacl_kref obtained from within
504517
* core_tpg_get_initiator_node_acl().
505518
*/
506519
if (se_nacl) {
520+
struct se_portal_group *se_tpg = se_nacl->se_tpg;
521+
const struct target_core_fabric_ops *se_tfo = se_tpg->se_tpg_tfo;
522+
unsigned long flags;
523+
507524
se_sess->se_node_acl = NULL;
525+
526+
/*
527+
* Also determine if we need to drop the extra ->cmd_kref if
528+
* it had been previously dynamically generated, and
529+
* the endpoint is not caching dynamic ACLs.
530+
*/
531+
mutex_lock(&se_tpg->acl_node_mutex);
532+
if (se_nacl->dynamic_node_acl &&
533+
!se_tfo->tpg_check_demo_mode_cache(se_tpg)) {
534+
spin_lock_irqsave(&se_nacl->nacl_sess_lock, flags);
535+
if (list_empty(&se_nacl->acl_sess_list))
536+
se_nacl->dynamic_stop = true;
537+
spin_unlock_irqrestore(&se_nacl->nacl_sess_lock, flags);
538+
539+
if (se_nacl->dynamic_stop)
540+
list_del(&se_nacl->acl_list);
541+
}
542+
mutex_unlock(&se_tpg->acl_node_mutex);
543+
544+
if (se_nacl->dynamic_stop)
545+
target_put_nacl(se_nacl);
546+
508547
target_put_nacl(se_nacl);
509548
}
510549
if (se_sess->sess_cmd_map) {
@@ -518,50 +557,28 @@ EXPORT_SYMBOL(transport_free_session);
518557
void transport_deregister_session(struct se_session *se_sess)
519558
{
520559
struct se_portal_group *se_tpg = se_sess->se_tpg;
521-
const struct target_core_fabric_ops *se_tfo;
522-
struct se_node_acl *se_nacl;
523560
unsigned long flags;
524-
bool drop_nacl = false;
525561

526562
if (!se_tpg) {
527563
transport_free_session(se_sess);
528564
return;
529565
}
530-
se_tfo = se_tpg->se_tpg_tfo;
531566

532567
spin_lock_irqsave(&se_tpg->session_lock, flags);
533568
list_del(&se_sess->sess_list);
534569
se_sess->se_tpg = NULL;
535570
se_sess->fabric_sess_ptr = NULL;
536571
spin_unlock_irqrestore(&se_tpg->session_lock, flags);
537572

538-
/*
539-
* Determine if we need to do extra work for this initiator node's
540-
* struct se_node_acl if it had been previously dynamically generated.
541-
*/
542-
se_nacl = se_sess->se_node_acl;
543-
544-
mutex_lock(&se_tpg->acl_node_mutex);
545-
if (se_nacl && se_nacl->dynamic_node_acl) {
546-
if (!se_tfo->tpg_check_demo_mode_cache(se_tpg)) {
547-
list_del(&se_nacl->acl_list);
548-
drop_nacl = true;
549-
}
550-
}
551-
mutex_unlock(&se_tpg->acl_node_mutex);
552-
553-
if (drop_nacl) {
554-
core_tpg_wait_for_nacl_pr_ref(se_nacl);
555-
core_free_device_list_for_node(se_nacl, se_tpg);
556-
se_sess->se_node_acl = NULL;
557-
kfree(se_nacl);
558-
}
559573
pr_debug("TARGET_CORE[%s]: Deregistered fabric_sess\n",
560574
se_tpg->se_tpg_tfo->get_fabric_name());
561575
/*
562576
* If last kref is dropping now for an explicit NodeACL, awake sleeping
563577
* ->acl_free_comp caller to wakeup configfs se_node_acl->acl_group
564578
* removal context from within transport_free_session() code.
579+
*
580+
* For dynamic ACL, target_put_nacl() uses target_complete_nacl()
581+
* to release all remaining generate_node_acl=1 created ACL resources.
565582
*/
566583

567584
transport_free_session(se_sess);
@@ -3110,7 +3127,6 @@ static void target_tmr_work(struct work_struct *work)
31103127
spin_unlock_irqrestore(&cmd->t_state_lock, flags);
31113128
goto check_stop;
31123129
}
3113-
cmd->t_state = TRANSPORT_ISTATE_PROCESSING;
31143130
spin_unlock_irqrestore(&cmd->t_state_lock, flags);
31153131

31163132
cmd->se_tfo->queue_tm_rsp(cmd);
@@ -3123,11 +3139,25 @@ int transport_generic_handle_tmr(
31233139
struct se_cmd *cmd)
31243140
{
31253141
unsigned long flags;
3142+
bool aborted = false;
31263143

31273144
spin_lock_irqsave(&cmd->t_state_lock, flags);
3128-
cmd->transport_state |= CMD_T_ACTIVE;
3145+
if (cmd->transport_state & CMD_T_ABORTED) {
3146+
aborted = true;
3147+
} else {
3148+
cmd->t_state = TRANSPORT_ISTATE_PROCESSING;
3149+
cmd->transport_state |= CMD_T_ACTIVE;
3150+
}
31293151
spin_unlock_irqrestore(&cmd->t_state_lock, flags);
31303152

3153+
if (aborted) {
3154+
pr_warn_ratelimited("handle_tmr caught CMD_T_ABORTED TMR %d"
3155+
"ref_tag: %llu tag: %llu\n", cmd->se_tmr_req->function,
3156+
cmd->se_tmr_req->ref_task_tag, cmd->tag);
3157+
transport_cmd_check_stop_to_fabric(cmd);
3158+
return 0;
3159+
}
3160+
31313161
INIT_WORK(&cmd->work, target_tmr_work);
31323162
queue_work(cmd->se_dev->tmr_wq, &cmd->work);
31333163
return 0;

drivers/target/target_core_xcopy.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -864,7 +864,7 @@ static void target_xcopy_do_work(struct work_struct *work)
864864
" CHECK_CONDITION -> sending response\n", rc);
865865
ec_cmd->scsi_status = SAM_STAT_CHECK_CONDITION;
866866
}
867-
target_complete_cmd(ec_cmd, SAM_STAT_CHECK_CONDITION);
867+
target_complete_cmd(ec_cmd, ec_cmd->scsi_status);
868868
}
869869

870870
sense_reason_t target_do_xcopy(struct se_cmd *se_cmd)

include/target/target_core_base.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,7 @@ struct se_node_acl {
538538
char initiatorname[TRANSPORT_IQN_LEN];
539539
/* Used to signal demo mode created ACL, disabled by default */
540540
bool dynamic_node_acl;
541+
bool dynamic_stop;
541542
u32 queue_depth;
542543
u32 acl_index;
543544
enum target_prot_type saved_prot_type;

0 commit comments

Comments
 (0)