From: Stefan Hajnoczi <stefanha@redhat.com>
To: target-devel@vger.kernel.org
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>,
linux-scsi@vger.kernel.org, Bart Van Assche <bvanassche@acm.org>,
linux-kernel@vger.kernel.org,
Stefan Hajnoczi <stefanha@redhat.com>,
Maurizio Lombardi <mlombard@redhat.com>,
Dmitry Bogdanov <d.bogdanov@yadro.com>
Subject: [PATCH] scsi: target: don't validate ignored fields in PROUT PREEMPT
Date: Thu, 2 Apr 2026 14:03:42 -0400 [thread overview]
Message-ID: <20260402180342.126583-1-stefanha@redhat.com> (raw)
The PERSISTENT RESERVE OUT command's PREEMPT service action provides two
different functions: 1. preempting persistent reservations and 2.
removing registrations. In the latter case the spec says:
b) ignore the contents of the SCOPE field and the TYPE field;
The code currently validates the SCOPE and TYPE fields even when PREEMPT
is called to remove registrations.
This patch achieves spec compliance by validating the SCOPE and TYPE
fields only when they will actually be used.
To confirm my interpretation of the specification I tested against HPE
3PAR storage and found the TYPE field is indeed ignored in this case.
Cc: Maurizio Lombardi <mlombard@redhat.com>
Cc: Dmitry Bogdanov <d.bogdanov@yadro.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
drivers/target/target_core_pr.c | 59 ++++++++++++++++++---------------
1 file changed, 32 insertions(+), 27 deletions(-)
diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index f88e63aefcd84..11790f2c5d80f 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -2809,7 +2809,7 @@ static void core_scsi3_release_preempt_and_abort(
}
static sense_reason_t
-core_scsi3_pro_preempt(struct se_cmd *cmd, int type, int scope, u64 res_key,
+core_scsi3_emulate_pro_preempt(struct se_cmd *cmd, int type, int scope, u64 res_key,
u64 sa_res_key, enum preempt_type preempt_type)
{
struct se_device *dev = cmd->se_dev;
@@ -2838,11 +2838,6 @@ core_scsi3_pro_preempt(struct se_cmd *cmd, int type, int scope, u64 res_key,
core_scsi3_put_pr_reg(pr_reg_n);
return TCM_RESERVATION_CONFLICT;
}
- if (scope != PR_SCOPE_LU_SCOPE) {
- pr_err("SPC-3 PR: Illegal SCOPE: 0x%02x\n", scope);
- core_scsi3_put_pr_reg(pr_reg_n);
- return TCM_INVALID_PARAMETER_LIST;
- }
spin_lock(&dev->dev_reservation_lock);
pr_res_holder = dev->dev_pr_res_holder;
@@ -2856,6 +2851,37 @@ core_scsi3_pro_preempt(struct se_cmd *cmd, int type, int scope, u64 res_key,
core_scsi3_put_pr_reg(pr_reg_n);
return TCM_INVALID_PARAMETER_LIST;
}
+
+ /* Validate TYPE and SCOPE fields if they will be used */
+ if (pr_res_holder &&
+ (pr_res_holder->pr_res_key == sa_res_key ||
+ (all_reg && !sa_res_key))) {
+ switch (type) {
+ case PR_TYPE_WRITE_EXCLUSIVE:
+ case PR_TYPE_EXCLUSIVE_ACCESS:
+ case PR_TYPE_WRITE_EXCLUSIVE_REGONLY:
+ case PR_TYPE_EXCLUSIVE_ACCESS_REGONLY:
+ case PR_TYPE_WRITE_EXCLUSIVE_ALLREG:
+ case PR_TYPE_EXCLUSIVE_ACCESS_ALLREG:
+ break;
+ default:
+ pr_err("SPC-3 PR: Unknown Service Action PREEMPT%s"
+ " Type: 0x%02x\n",
+ (preempt_type == PREEMPT_AND_ABORT) ?
+ "_AND_ABORT" : "", type);
+ spin_unlock(&dev->dev_reservation_lock);
+ core_scsi3_put_pr_reg(pr_reg_n);
+ return TCM_INVALID_CDB_FIELD;
+ }
+
+ if (scope != PR_SCOPE_LU_SCOPE) {
+ pr_err("SPC-3 PR: Illegal SCOPE: 0x%02x\n", scope);
+ spin_unlock(&dev->dev_reservation_lock);
+ core_scsi3_put_pr_reg(pr_reg_n);
+ return TCM_INVALID_PARAMETER_LIST;
+ }
+ }
+
/*
* From spc4r17, section 5.7.11.4.4 Removing Registrations:
*
@@ -3118,27 +3144,6 @@ core_scsi3_pro_preempt(struct se_cmd *cmd, int type, int scope, u64 res_key,
return 0;
}
-static sense_reason_t
-core_scsi3_emulate_pro_preempt(struct se_cmd *cmd, int type, int scope,
- u64 res_key, u64 sa_res_key, enum preempt_type preempt_type)
-{
- switch (type) {
- case PR_TYPE_WRITE_EXCLUSIVE:
- case PR_TYPE_EXCLUSIVE_ACCESS:
- case PR_TYPE_WRITE_EXCLUSIVE_REGONLY:
- case PR_TYPE_EXCLUSIVE_ACCESS_REGONLY:
- case PR_TYPE_WRITE_EXCLUSIVE_ALLREG:
- case PR_TYPE_EXCLUSIVE_ACCESS_ALLREG:
- return core_scsi3_pro_preempt(cmd, type, scope, res_key,
- sa_res_key, preempt_type);
- default:
- pr_err("SPC-3 PR: Unknown Service Action PREEMPT%s"
- " Type: 0x%02x\n", (preempt_type == PREEMPT_AND_ABORT) ? "_AND_ABORT" : "", type);
- return TCM_INVALID_CDB_FIELD;
- }
-}
-
-
static sense_reason_t
core_scsi3_emulate_pro_register_and_move(struct se_cmd *cmd, u64 res_key,
u64 sa_res_key, int aptpl, int unreg)
--
2.53.0
next reply other threads:[~2026-04-02 18:04 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-02 18:03 Stefan Hajnoczi [this message]
2026-04-09 2:39 ` [PATCH] scsi: target: don't validate ignored fields in PROUT PREEMPT Martin K. Petersen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260402180342.126583-1-stefanha@redhat.com \
--to=stefanha@redhat.com \
--cc=bvanassche@acm.org \
--cc=d.bogdanov@yadro.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=mlombard@redhat.com \
--cc=target-devel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox