[PATCH 1/2] target: Fix compatible reservation handling (CRH=1) with legacy RESERVE/RELEASE

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx>

This patch addresses a bug with target_check_scsi2_reservation_conflict()
return checking in target_scsi2_reservation_[reserve,release]() that was
preventing CRH=1 operation from silently succeeding in the two special
cases defined by SPC-3, and not failing with reservation conflict status
when dealing with legacy RESERVE/RELEASE + active SPC-3 PR logic.

Also explictly set cmd->scsi_status = SAM_STAT_RESERVATION_CONFLICT during
the early non reservation holder failure from pr_ops->t10_seq_non_holder()
check in transport_generic_cmd_sequencer() for fabrics that already expect
it to be set.

This bug was originally introduced in lio-core commit:

commit eacac00ce5bfde8086cd0615fb53c986f7f970fe
Author: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date:   Thu Nov 3 17:50:40 2011 -0400

    target: split core_scsi2_emulate_crh

Reported-by: Martin Svec <martin.svec@xxxxxxxx>
Cc: Martin Svec <martin.svec@xxxxxxxx>
Cc: Christoph Hellwig <hch@xxxxxx>
Signed-off-by: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx>
---
 drivers/target/target_core_pr.c        |   34 ++++++++++++++++++++-----------
 drivers/target/target_core_transport.c |    1 +
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index 76d4143..e899e7d 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -114,7 +114,7 @@ static struct t10_pr_registration *core_scsi3_locate_pr_reg(struct se_device *,
 					struct se_node_acl *, struct se_session *);
 static void core_scsi3_put_pr_reg(struct t10_pr_registration *);
 
-static int target_check_scsi2_reservation_conflict(struct se_cmd *cmd, int *ret)
+static int target_check_scsi2_reservation_conflict(struct se_cmd *cmd)
 {
 	struct se_session *se_sess = cmd->se_sess;
 	struct se_subsystem_dev *su_dev = cmd->se_dev->se_sub_dev;
@@ -124,7 +124,7 @@ static int target_check_scsi2_reservation_conflict(struct se_cmd *cmd, int *ret)
 	int conflict = 0;
 
 	if (!crh)
-		return false;
+		return -EINVAL;
 
 	pr_reg = core_scsi3_locate_pr_reg(cmd->se_dev, se_sess->se_node_acl,
 			se_sess);
@@ -152,16 +152,14 @@ static int target_check_scsi2_reservation_conflict(struct se_cmd *cmd, int *ret)
 		 */
 		if (pr_reg->pr_res_holder) {
 			core_scsi3_put_pr_reg(pr_reg);
-			*ret = 0;
-			return false;
+			return 1;
 		}
 		if ((pr_reg->pr_res_type == PR_TYPE_WRITE_EXCLUSIVE_REGONLY) ||
 		    (pr_reg->pr_res_type == PR_TYPE_EXCLUSIVE_ACCESS_REGONLY) ||
 		    (pr_reg->pr_res_type == PR_TYPE_WRITE_EXCLUSIVE_ALLREG) ||
 		    (pr_reg->pr_res_type == PR_TYPE_EXCLUSIVE_ACCESS_ALLREG)) {
 			core_scsi3_put_pr_reg(pr_reg);
-			*ret = 0;
-			return true;
+			return 1;
 		}
 		core_scsi3_put_pr_reg(pr_reg);
 		conflict = 1;
@@ -186,10 +184,10 @@ static int target_check_scsi2_reservation_conflict(struct se_cmd *cmd, int *ret)
 			" while active SPC-3 registrations exist,"
 			" returning RESERVATION_CONFLICT\n");
 		cmd->scsi_sense_reason = TCM_RESERVATION_CONFLICT;
-		return true;
+		return -EBUSY;
 	}
 
-	return false;
+	return 0;
 }
 
 int target_scsi2_reservation_release(struct se_task *task)
@@ -198,10 +196,16 @@ int target_scsi2_reservation_release(struct se_task *task)
 	struct se_device *dev = cmd->se_dev;
 	struct se_session *sess = cmd->se_sess;
 	struct se_portal_group *tpg = sess->se_tpg;
-	int ret = 0;
+	int ret = 0, rc;
 
-	if (target_check_scsi2_reservation_conflict(cmd, &ret))
+	rc = target_check_scsi2_reservation_conflict(cmd);
+	if (rc == 1) 
+		goto out;
+	else if (rc < 0) {
+		cmd->scsi_sense_reason = TCM_RESERVATION_CONFLICT;
+		ret = -EINVAL;
 		goto out;
+	}
 
 	ret = 0;
 	spin_lock(&dev->dev_reservation_lock);
@@ -238,7 +242,7 @@ int target_scsi2_reservation_reserve(struct se_task *task)
 	struct se_device *dev = cmd->se_dev;
 	struct se_session *sess = cmd->se_sess;
 	struct se_portal_group *tpg = sess->se_tpg;
-	int ret = 0;
+	int ret = 0, rc;
 
 	if ((cmd->t_task_cdb[1] & 0x01) &&
 	    (cmd->t_task_cdb[1] & 0x02)) {
@@ -248,8 +252,14 @@ int target_scsi2_reservation_reserve(struct se_task *task)
 		ret = -EINVAL;
 		goto out;
 	}
-	if (target_check_scsi2_reservation_conflict(cmd, &ret))
+	rc = target_check_scsi2_reservation_conflict(cmd);
+	if (rc == 1)
 		goto out;
+	else if (rc < 0) {
+		cmd->scsi_sense_reason = TCM_RESERVATION_CONFLICT;
+		ret = -EINVAL;
+		goto out;
+	}
 
 	ret = 0;
 	spin_lock(&dev->dev_reservation_lock);
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 961d8b7..cfdfb80 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -2640,6 +2640,7 @@ static int transport_generic_cmd_sequencer(
 					cmd, cdb, pr_reg_type) != 0) {
 			cmd->se_cmd_flags |= SCF_SCSI_CDB_EXCEPTION;
 			cmd->se_cmd_flags |= SCF_SCSI_RESERVATION_CONFLICT;
+			cmd->scsi_status = SAM_STAT_RESERVATION_CONFLICT;
 			cmd->scsi_sense_reason = TCM_RESERVATION_CONFLICT;
 			return -EBUSY;
 		}
-- 
1.7.2.5

--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux SCSI]     [Kernel Newbies]     [Linux SCSI Target Infrastructure]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Device Mapper]

  Powered by Linux