Linux SCSI subsystem development
 help / color / mirror / Atom feed
* [PATCH] target/stat: implement scsiPortBusyStatuses counter
@ 2026-07-08  9:34 kensanya
  2026-07-08  9:57 ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: kensanya @ 2026-07-08  9:34 UTC (permalink / raw)
  To: martin.petersen; +Cc: linux-scsi, target-devel, TanZheng

From: TanZheng <tanzheng@kylinos.cn>

Implement RFC 4455 scsiPortBusyStatuses (ConfigFS busy_count) by adding a
per-CPU busy_statuses counter to scsi_port_stats, incrementing it when
the target sends SAM_STAT_BUSY, and summing all CPUs in the show handler.

Signed-off-by: TanZheng <tanzheng@kylinos.cn>
---
 drivers/target/target_core_internal.h  |  1 +
 drivers/target/target_core_stat.c      | 26 ++++++++++++++++++++++++--
 drivers/target/target_core_transport.c | 23 +++++++++++++++++------
 include/target/target_core_base.h      |  1 +
 4 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h
index 763e6d26e187..a91f873d1845 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -161,6 +161,7 @@ void	target_queued_submit_work(struct work_struct *work);
 void	target_stat_setup_dev_default_groups(struct se_device *);
 void	target_stat_setup_port_default_groups(struct se_lun *);
 void	target_stat_setup_mappedlun_default_groups(struct se_lun_acl *);
+void	target_stat_count_busy_status(struct se_cmd *cmd);
 
 /* target_core_xcopy.c */
 extern struct se_portal_group xcopy_pt_tpg;
diff --git a/drivers/target/target_core_stat.c b/drivers/target/target_core_stat.c
index 083205052be2..374aab290c07 100644
--- a/drivers/target/target_core_stat.c
+++ b/drivers/target/target_core_stat.c
@@ -21,12 +21,25 @@
 #include <linux/seq_file.h>
 #include <linux/configfs.h>
 
+#include <scsi/scsi_proto.h>
+
 #include <target/target_core_base.h>
 #include <target/target_core_backend.h>
 #include <target/target_core_fabric.h>
 
 #include "target_core_internal.h"
 
+void target_stat_count_busy_status(struct se_cmd *cmd)
+{
+	struct se_lun *lun = cmd->se_lun;
+
+	if (cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)
+		return;
+
+	if (cmd->scsi_status == SAM_STAT_BUSY && lun && lun->lun_stats)
+		this_cpu_inc(lun->lun_stats->busy_statuses);
+}
+
 #ifndef INITIAL_JIFFIES
 #define INITIAL_JIFFIES ((unsigned long)(unsigned int) (-300*HZ))
 #endif
@@ -483,13 +496,22 @@ static ssize_t target_stat_port_busy_count_show(struct config_item *item,
 {
 	struct se_lun *lun = to_stat_port(item);
 	struct se_device *dev;
+	struct scsi_port_stats *stats;
+	unsigned int cpu;
+	u64 sum = 0;
 	ssize_t ret = -ENODEV;
 
 	rcu_read_lock();
 	dev = rcu_dereference(lun->lun_se_dev);
 	if (dev) {
-		/* FIXME: scsiPortBusyStatuses  */
-		ret = snprintf(page, PAGE_SIZE, "%u\n", 0);
+		/* scsiPortBusyStatuses */
+		if (lun->lun_stats) {
+			for_each_possible_cpu(cpu) {
+				stats = per_cpu_ptr(lun->lun_stats, cpu);
+				sum += stats->busy_statuses;
+			}
+		}
+		ret = snprintf(page, PAGE_SIZE, "%llu\n", sum);
 	}
 	rcu_read_unlock();
 	return ret;
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index fad03a15c969..bc3e683e8bdf 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -829,6 +829,17 @@ void transport_copy_sense_to_cmd(struct se_cmd *cmd, unsigned char *sense)
 }
 EXPORT_SYMBOL(transport_copy_sense_to_cmd);
 
+static int target_queue_status(struct se_cmd *cmd)
+{
+	int ret;
+
+	ret = cmd->se_tfo->queue_status(cmd);
+	if (!ret)
+		target_stat_count_busy_status(cmd);
+
+	return ret;
+}
+
 static void target_handle_abort(struct se_cmd *cmd)
 {
 	bool tas = cmd->transport_state & CMD_T_TAS;
@@ -843,7 +854,7 @@ static void target_handle_abort(struct se_cmd *cmd)
 			pr_debug("Setting SAM_STAT_TASK_ABORTED status for CDB: 0x%02x, ITT: 0x%08llx\n",
 				 cmd->t_task_cdb[0], cmd->tag);
 			trace_target_cmd_complete(cmd);
-			ret = cmd->se_tfo->queue_status(cmd);
+			ret = target_queue_status(cmd);
 			if (ret) {
 				transport_handle_queue_full(cmd, cmd->se_dev,
 							    ret, false);
@@ -2170,7 +2181,7 @@ void transport_generic_request_failure(struct se_cmd *cmd,
 
 queue_status:
 	trace_target_cmd_complete(cmd);
-	ret = cmd->se_tfo->queue_status(cmd);
+	ret = target_queue_status(cmd);
 	if (!ret)
 		goto check_stop;
 queue_full:
@@ -2487,7 +2498,7 @@ static void transport_complete_qf(struct se_cmd *cmd)
 	case DMA_NONE:
 queue_status:
 		trace_target_cmd_complete(cmd);
-		ret = cmd->se_tfo->queue_status(cmd);
+		ret = target_queue_status(cmd);
 		break;
 	default:
 		break;
@@ -2685,7 +2696,7 @@ static void target_complete_ok_work(struct work_struct *work)
 	case DMA_NONE:
 queue_status:
 		trace_target_cmd_complete(cmd);
-		ret = cmd->se_tfo->queue_status(cmd);
+		ret = target_queue_status(cmd);
 		if (ret)
 			goto queue_full;
 		break;
@@ -3599,7 +3610,7 @@ transport_send_check_condition_and_sense(struct se_cmd *cmd,
 		translate_sense_reason(cmd, reason);
 
 	trace_target_cmd_complete(cmd);
-	return cmd->se_tfo->queue_status(cmd);
+	return target_queue_status(cmd);
 }
 EXPORT_SYMBOL(transport_send_check_condition_and_sense);
 
@@ -3615,7 +3626,7 @@ int target_send_busy(struct se_cmd *cmd)
 
 	cmd->scsi_status = SAM_STAT_BUSY;
 	trace_target_cmd_complete(cmd);
-	return cmd->se_tfo->queue_status(cmd);
+	return target_queue_status(cmd);
 }
 EXPORT_SYMBOL(target_send_busy);
 
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 002b0fc57587..61a230331124 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -764,6 +764,7 @@ struct scsi_port_stats {
 	u64			cmd_pdus;
 	u64			tx_data_octets;
 	u64			rx_data_octets;
+	u64			busy_statuses;
 };
 
 struct se_lun {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] target/stat: implement scsiPortBusyStatuses counter
  2026-07-08  9:34 [PATCH] target/stat: implement scsiPortBusyStatuses counter kensanya
@ 2026-07-08  9:57 ` sashiko-bot
  2026-07-09  6:29   ` kensanya
  0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-07-08  9:57 UTC (permalink / raw)
  To: kensanya; +Cc: linux-scsi

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Accessing `cmd` after calling `queue_status()` can cause a Use-After-Free, as the command may be completed and freed asynchronously by the fabric driver.
- [Medium] The `scsiPortBusyStatuses` implementation only tracks `SAM_STAT_BUSY`, violating RFC 4455.
--

--- Patch [1]: [PATCH] target/stat: implement scsiPortBusyStatuses counter ---
commit 0e48192c7d342b22881ecb0dee446ae287e24607
Author: TanZheng <tanzheng@kylinos.cn>

target/stat: implement scsiPortBusyStatuses counter
    
Implement RFC 4455 scsiPortBusyStatuses (ConfigFS busy_count) by adding a
per-CPU busy_statuses counter to scsi_port_stats, incrementing it when
the target sends SAM_STAT_BUSY, and summing all CPUs in the show handler.

> diff --git a/drivers/target/target_core_stat.c b/drivers/target/target_core_stat.c
> --- a/drivers/target/target_core_stat.c
> +++ b/drivers/target/target_core_stat.c
[ ... ]
> +void target_stat_count_busy_status(struct se_cmd *cmd)
> +{
> +	struct se_lun *lun = cmd->se_lun;
> +
> +	if (cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)
> +		return;
> +
> +	if (cmd->scsi_status == SAM_STAT_BUSY && lun && lun->lun_stats)
> +		this_cpu_inc(lun->lun_stats->busy_statuses);

[Severity: Medium]
Does this code fully implement the RFC 4455 scsiPortBusyStatuses counter?

RFC 4455 states that scsiPortBusyStatuses must increment for BUSY, TASK SET
FULL, and ACA ACTIVE statuses. By checking only for SAM_STAT_BUSY, does this
undercount port busy statuses during queue full events (like
SAM_STAT_TASK_SET_FULL)?

> +}
[ ... ]
> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
> --- a/drivers/target/target_core_transport.c
> +++ b/drivers/target/target_core_transport.c
[ ... ]
> +static int target_queue_status(struct se_cmd *cmd)
> +{
> +	int ret;
> +
> +	ret = cmd->se_tfo->queue_status(cmd);
> +	if (!ret)
> +		target_stat_count_busy_status(cmd);

[Severity: High]
Can this result in a use-after-free on the cmd pointer?

If a fabric driver (such as ib_srpt) calls target_send_busy(cmd) during a
failure path, target_queue_status() will invoke the fabric driver's
queue_status() callback. The fabric driver might transmit the response and its
asynchronous completion handler could drop the final reference to the command,
freeing its memory.

When queue_status() returns, target_queue_status() calls
target_stat_count_busy_status(cmd) which accesses cmd->se_lun and
cmd->scsi_status. Is it safe to dereference cmd here after relinquishing
ownership to queue_status()?

> +
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708093433.838647-1-kensanya@163.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re:Re: [PATCH] target/stat: implement scsiPortBusyStatuses counter
  2026-07-08  9:57 ` sashiko-bot
@ 2026-07-09  6:29   ` kensanya
  0 siblings, 0 replies; 3+ messages in thread
From: kensanya @ 2026-07-09  6:29 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: linux-scsi


















At 2026-07-08 17:57:22, sashiko-bot@kernel.org wrote:
>Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>- [High] Accessing `cmd` after calling `queue_status()` can cause a Use-After-Free, as the command may be completed and freed asynchronously by the fabric driver.
>- [Medium] The `scsiPortBusyStatuses` implementation only tracks `SAM_STAT_BUSY`, violating RFC 4455.
>--
>
>--- Patch [1]: [PATCH] target/stat: implement scsiPortBusyStatuses counter ---
>commit 0e48192c7d342b22881ecb0dee446ae287e24607
>Author: TanZheng <tanzheng@kylinos.cn>
>
>target/stat: implement scsiPortBusyStatuses counter
>    
>Implement RFC 4455 scsiPortBusyStatuses (ConfigFS busy_count) by adding a
>per-CPU busy_statuses counter to scsi_port_stats, incrementing it when
>the target sends SAM_STAT_BUSY, and summing all CPUs in the show handler.
>
>> diff --git a/drivers/target/target_core_stat.c b/drivers/target/target_core_stat.c
>> --- a/drivers/target/target_core_stat.c
>> +++ b/drivers/target/target_core_stat.c
>[ ... ]
>> +void target_stat_count_busy_status(struct se_cmd *cmd)
>> +{
>> +	struct se_lun *lun = cmd->se_lun;
>> +
>> +	if (cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)
>> +		return;
>> +
>> +	if (cmd->scsi_status == SAM_STAT_BUSY && lun && lun->lun_stats)
>> +		this_cpu_inc(lun->lun_stats->busy_statuses);
>
>[Severity: Medium]
>Does this code fully implement the RFC 4455 scsiPortBusyStatuses counter?
>
>RFC 4455 states that scsiPortBusyStatuses must increment for BUSY, TASK SET
>FULL, and ACA ACTIVE statuses. By checking only for SAM_STAT_BUSY, does this
>undercount port busy statuses during queue full events (like
>SAM_STAT_TASK_SET_FULL)?
>
This patch currently only supports the statistics of scsiPortBusyStatuses.
>> +}
>[ ... ]
>> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
>> --- a/drivers/target/target_core_transport.c
>> +++ b/drivers/target/target_core_transport.c
>[ ... ]
>> +static int target_queue_status(struct se_cmd *cmd)
>> +{
>> +	int ret;
>> +
>> +	ret = cmd->se_tfo->queue_status(cmd);
>> +	if (!ret)
>> +		target_stat_count_busy_status(cmd);
>
>[Severity: High]
>Can this result in a use-after-free on the cmd pointer?
>
>If a fabric driver (such as ib_srpt) calls target_send_busy(cmd) during a
>failure path, target_queue_status() will invoke the fabric driver's
>queue_status() callback. The fabric driver might transmit the response and its
>asynchronous completion handler could drop the final reference to the command,
>freeing its memory.
>
>When queue_status() returns, target_queue_status() calls
>target_stat_count_busy_status(cmd) which accesses cmd->se_lun and
>cmd->scsi_status. Is it safe to dereference cmd here after relinquishing
>ownership to queue_status()?
>
Count after a successful queue_status() because RFC 4455 counts statuses actually sent 
by the port. Fabric may fail and retry via queue_full, and may even change the SCSI 
status on failure (e.g. tcm_fc BUSY → TASK_SET_FULL). Counting before send would
over-count and double-count on retries.

Regarding the UAF issue, I will release the second patch.
>> +
>> +	return ret;
>> +}
>
>-- 
>Sashiko AI review · https://sashiko.dev/#/patchset/20260708093433.838647-1-kensanya@163.com?part=1

Best regard!
TanZheng

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-09  6:30 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-08  9:34 [PATCH] target/stat: implement scsiPortBusyStatuses counter kensanya
2026-07-08  9:57 ` sashiko-bot
2026-07-09  6:29   ` kensanya

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox