Re: [PATCH 2/3] target: Add TARGET_SCF_LOOKUP_LUN_FROM_TAG support for ABORT_TASK

From: Bart Van Assche
Date: Mon Jun 05 2017 - 11:57:37 EST


On Sat, 2017-06-03 at 22:10 +0000, Nicholas A. Bellinger wrote:
> +static bool target_lookup_lun_from_tag(struct se_session *se_sess, u64 tag,
> + u64 *unpacked_lun)
> +{
> + struct se_cmd *se_cmd;
> + unsigned long flags;
> + bool ret = false;
> +
> + spin_lock_irqsave(&se_sess->sess_cmd_lock, flags);
> + list_for_each_entry(se_cmd, &se_sess->sess_cmd_list, se_cmd_list) {
> + if (se_cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)
> + continue;
> +
> + if (se_cmd->tag == tag) {
> + *unpacked_lun = se_cmd->orig_fe_lun;
> + ret = true;
> + break;
> + }
> + }
> + spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
> +
> + return ret;
> +}
> +
> /**
> * target_submit_tmr - lookup unpacked lun and submit uninitialized se_cmd
> * for TMR CDBs
> @@ -1639,19 +1662,31 @@ int target_submit_tmr(struct se_cmd *se_cmd, struct se_session *se_sess,
> core_tmr_release_req(se_cmd->se_tmr_req);
> return ret;
> }
> + /*
> + * If this is ABORT_TASK with no explicit fabric provided LUN,
> + * go ahead and search active session tags for a match to figure
> + * out unpacked_lun for the original se_cmd.
> + */
> + if (tm_type == TMR_ABORT_TASK && (flags & TARGET_SCF_LOOKUP_LUN_FROM_TAG)) {
> + if (!target_lookup_lun_from_tag(se_sess, tag, &unpacked_lun))
> + goto failure;
> + }
>
> ret = transport_lookup_tmr_lun(se_cmd, unpacked_lun);
> - if (ret) {
> - /*
> - * For callback during failure handling, push this work off
> - * to process context with TMR_LUN_DOES_NOT_EXIST status.
> - */
> - INIT_WORK(&se_cmd->work, target_complete_tmr_failure);
> - schedule_work(&se_cmd->work);
> - return 0;
> - }
> + if (ret)
> + goto failure;
> +
> transport_generic_handle_tmr(se_cmd);
> return 0;

Hello Nic,

So after LUN lookup has finished sess_cmd_lock lock is dropped, a context
switch occurs due to the queue_work() call in transport_generic_handle_tmr()
and next core_tmr_abort_task() reacquires that lock? Sorry but I'm afraid
that if after that lock is dropped and before it is reacquired a command
finishes and the initiator reuses the same tag for another command for the
same LUN that this may result in the wrong command being aborted.

Bart.