[PATCH] scsi: target: fix PRIN READ_FULL_STATUS buffer bounds check

[PATCH] scsi: target: fix PRIN READ_FULL_STATUS buffer bounds check

[Tianchu Chen]

From: Tianchu Chen <flynnnchen@tencent.com>

core_scsi3_pri_read_full_status() writes one Full Status descriptor
per registered I_T nexus into the PRIN response buffer: a fixed
24-byte descriptor header followed by the variable-length
TransportID.  The overflow check, however, compares

	exp_desc_len + add_len > cmd->data_length

where exp_desc_len covers only the TransportID and add_len
accumulates only after completed descriptors, so it undershoots the actual
write extent by a constant 32 bytes (the 8-byte PRIN header, since
off = add_len + 8, plus the 24-byte descriptor header), leading a
remotely-triggerable heap OOB-write.

The allocation length of the PERSISTENT RESERVE IN CDB is fully
controlled by the (possibly remote) initiator and truncates
cmd->data_length, so an initiator that registered itself can pick a
data_length in [desc_len, desc_len + 31] and make the first
descriptor write up to 32 bytes past the end of the command's data
buffer.

Base the check on the actual write offset instead, so the test
covers exactly what the loop is about to write (24-byte descriptor
header plus TransportID, matching add_desc_len = 24 + desc_len
below), as core_scsi3_pri_read_keys() already does. Same principle
as commit c58a252beb04c ("target: fix buffer offset in
core_scsi3_pri_read_full_status").

Discovered by Atuin - Automated Vulnerability Discovery Engine.

Fixes: c66ac9db8d4ad ("[SCSI] target: Add LIO target core v4.0.0-rc6")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
---
 drivers/target/target_core_pr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index 25b1bcacc0c8f..1d34887e9e845 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -3990,7 +3990,7 @@ core_scsi3_pri_read_full_status(struct se_cmd *cmd)
 		exp_desc_len = target_get_pr_transport_id_len(se_nacl, pr_reg,
 					&format_code);
 		if (exp_desc_len < 0 ||
-		    exp_desc_len + add_len > cmd->data_length) {
+		    off + 24 + exp_desc_len > cmd->data_length) {
 			pr_warn("SPC-3 PRIN READ_FULL_STATUS ran"
 				" out of buffer: %d\n", cmd->data_length);
 			spin_lock(&pr_tmpl->registration_lock);
-- 
2.51.0

[PATCH] scsi: target: fix use-after-free on self-preempt in PROUT PREEMPT

[Tianchu Chen]

From: Tianchu Chen <flynnnchen@tencent.com>

In core_scsi3_emulate_pro_preempt(), the !all_reg first path removes
every registration whose key matches the SA RESERVATION KEY field -
including the caller's own.  Unlike the two sibling loops, this one
does not skip the calling I_T nexus; instead it passes
dec_holders = calling_it_nexus, so __core_scsi3_free_registration()
drops the reference taken by core_scsi3_locate_pr_reg() and, for
plain PREEMPT, kmem_cache_free()s the registration right away.

The function tail then unconditionally calls
core_scsi3_put_pr_reg(pr_reg_n), decrementing pr_res_holders of the
already freed object.

A remote initiator can trigger this with two commands: REGISTER with
key K, then PREEMPT with SA RESERVATION KEY K while no reservation is
held (or it is held with a different key).  The dangling decrement
silently corrupts the freed slot, or underflows pr_res_holders of
whatever registration reuses the t10_pr_reg_cache slot, which can
later turn into an early free / double free of a live registration.

Track that the free path consumed the locate reference and skip the
final put in that case.

Discovered by Atuin - Automated Vulnerability Discovery Engine.

Fixes: c66ac9db8d4ad ("[SCSI] target: Add LIO target core v4.0.0-rc6")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
---
 drivers/target/target_core_pr.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index 1d34887e9e845..80e112efd6ead 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -2823,6 +2823,7 @@ core_scsi3_emulate_pro_preempt(struct se_cmd *cmd, int type, int scope, u64 res_
 	u64 pr_res_mapped_lun = 0;
 	int all_reg = 0, calling_it_nexus = 0;
 	bool sa_res_key_unmatched = sa_res_key != 0;
+	bool reg_n_released = false;
 	int prh_type = 0, prh_scope = 0;
 
 	if (!se_sess)
@@ -2926,6 +2927,13 @@ core_scsi3_emulate_pro_preempt(struct se_cmd *cmd, int type, int scope, u64 res_
 				sa_res_key_unmatched = false;
 
 				calling_it_nexus = (pr_reg_n == pr_reg) ? 1 : 0;
+				/*
+				 * dec_holders=1 makes the free path drop the
+				 * reference core_scsi3_locate_pr_reg() took on
+				 * pr_reg_n, so the tail must not put it again.
+				 */
+				if (calling_it_nexus)
+					reg_n_released = true;
 				pr_reg_nacl = pr_reg->pr_reg_nacl;
 				pr_res_mapped_lun = pr_reg->pr_res_mapped_lun;
 				__core_scsi3_free_registration(dev, pr_reg,
@@ -3012,7 +3020,8 @@ core_scsi3_emulate_pro_preempt(struct se_cmd *cmd, int type, int scope, u64 res_
 		if (pr_tmpl->pr_aptpl_active)
 			core_scsi3_update_and_write_aptpl(cmd->se_dev, true);
 
-		core_scsi3_put_pr_reg(pr_reg_n);
+		if (!reg_n_released)
+			core_scsi3_put_pr_reg(pr_reg_n);
 		core_scsi3_pr_generation(cmd->se_dev);
 		return 0;
 	}
-- 
2.51.0