[PATCH 0/2] scsi: target: alua: Skip grace periods on LUN add and remove

[PATCH 0/2] scsi: target: alua: Skip grace periods on LUN add and remove

[Qiliang Yuan]

Creating or removing a LUN through configfs attaches it to, or detaches
it from, its ALUA target port group, and both helpers wait for an RCU
grace period afterwards.

Neither wait protects anything, yet each costs one grace period per
LUN, which adds up to seconds when exporting or tearing down many LUNs,
e.g. when targetcli restores a saved configuration.

Drop the wait on attach (patch 1) and on detach (patch 2).

Adding and removing 256 LUNs of an iSCSI TPG through configfs, five
rounds alternating with and without the series, takes (median, min-max
in parentheses):

  256 LUNs  before (ms)       after (ms)        change
  add       3144 (3104-3175)  1379 (1277-1393)    -56%
  remove    5222 (5165-5244)  3551 (3436-3810)    -32%

Signed-off-by: Qiliang Yuan <qiliang.yuan@linux.dev>
---
Qiliang Yuan (2):
      scsi: target: alua: Don't wait for a grace period when attaching a LUN
      scsi: target: alua: Drop the grace period wait from LUN detach

 drivers/target/target_core_alua.c | 2 --
 1 file changed, 2 deletions(-)
---
base-commit: b3dfbde62cf1981c4638939c159dfd087b7a72e9
change-id: 20261110-bug-target-alua-lun-gp-b410ef8d1144

Best regards,
-- 
Qiliang Yuan <qiliang.yuan@linux.dev>

[PATCH 1/2] scsi: target: alua: Don't wait for a grace period when attaching a LUN

[Qiliang Yuan]

core_tpg_add_lun() attaches a new LUN to the default ALUA target port
group before adding it to the device and TPG lists, and
target_attach_tg_pt_gp() then waits for an RCU grace period.

The wait protects nothing: lun->lun_tg_pt_gp goes from NULL to a live
group, nothing is freed, and no reader can reach the LUN yet. It still
costs a grace period for every LUN created through configfs, which
dominates the time spent exporting many LUNs.

Remove the synchronize_rcu() call. Group changes through
core_alua_store_tg_pt_gp_info() keep their own grace period.

Adding 256 LUNs to an iSCSI TPG through configfs, five rounds
alternating with and without this patch, takes (median, min-max in
parentheses):

  LUNs  before (ms)       after (ms)        change
   256  3144 (3104-3175)  1379 (1277-1393)    -56%

Signed-off-by: Qiliang Yuan <qiliang.yuan@linux.dev>
---
 drivers/target/target_core_alua.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
index 140154d93c430..35d36cd36db6c 100644
--- a/drivers/target/target_core_alua.c
+++ b/drivers/target/target_core_alua.c
@@ -1833,7 +1833,6 @@ void target_attach_tg_pt_gp(struct se_lun *lun,
 	spin_lock(&lun->lun_tg_pt_gp_lock);
 	__target_attach_tg_pt_gp(lun, tg_pt_gp);
 	spin_unlock(&lun->lun_tg_pt_gp_lock);
-	synchronize_rcu();
 }
 
 static void __target_detach_tg_pt_gp(struct se_lun *lun,

-- 
2.43.0

[PATCH 2/2] scsi: target: alua: Drop the grace period wait from LUN detach

[Qiliang Yuan]

core_tpg_remove_lun() detaches a LUN from its ALUA target port group
once transport_clear_lun_ref() has drained its I/O, and
target_detach_tg_pt_gp() then waits for an RCU grace period.

Nothing that wait could protect is freed here: the group lives on until
core_alua_free_tg_pt_gp(), which waits for a grace period of its own
before freeing it, and the se_lun is freed with call_rcu(). Every LUN
removed through configfs still pays a grace period for it.

Remove the synchronize_rcu() call.

Removing 256 LUNs from an iSCSI TPG through configfs, five rounds
alternating with and without this patch, takes (median, min-max in
parentheses):

  LUNs  before (ms)       after (ms)        change
   256  5222 (5165-5244)  3551 (3436-3810)    -32%

Signed-off-by: Qiliang Yuan <qiliang.yuan@linux.dev>
---
 drivers/target/target_core_alua.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
index 35d36cd36db6c..4e22f988bb83f 100644
--- a/drivers/target/target_core_alua.c
+++ b/drivers/target/target_core_alua.c
@@ -1858,7 +1858,6 @@ void target_detach_tg_pt_gp(struct se_lun *lun)
 		rcu_assign_pointer(lun->lun_tg_pt_gp, NULL);
 	}
 	spin_unlock(&lun->lun_tg_pt_gp_lock);
-	synchronize_rcu();
 }
 
 static void target_swap_tg_pt_gp(struct se_lun *lun,

-- 
2.43.0

Re: [PATCH 1/2] scsi: target: alua: Don't wait for a grace period when attaching a LUN

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] target: out-of-bounds write in
  target_emulate_report_target_port_groups() [1]
--

commit 2f412f4b4c4324ed81aac0037066698113367ec3
Author: Qiliang Yuan <qiliang.yuan@linux.dev>

scsi: target: alua: Don't wait for a grace period when attaching a LUN

This commit removes the synchronize_rcu() call from target_attach_tg_pt_gp()
when attaching a newly created LUN to the default ALUA target port group.

> diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
> index 140154d93c430..35d36cd36db6c 100644
> --- a/drivers/target/target_core_alua.c
> +++ b/drivers/target/target_core_alua.c
> @@ -1833,7 +1833,6 @@ void target_attach_tg_pt_gp(struct se_lun *lun,
>  	spin_lock(&lun->lun_tg_pt_gp_lock);
>  	__target_attach_tg_pt_gp(lun, tg_pt_gp);
>  	spin_unlock(&lun->lun_tg_pt_gp_lock);
> -	synchronize_rcu();
>  }
>  
>  static void __target_detach_tg_pt_gp(struct se_lun *lun,

[Severity: Critical]
This problem wasn't introduced by this patch, but can concurrent calls to
target_attach_tg_pt_gp() and target_emulate_report_target_port_groups() cause
an out-of-bounds write to buf[] [1]?

drivers/target/target_core_alua.c:target_emulate_report_target_port_groups() {
    ...
		if ((off + 8 + (tg_pt_gp->tg_pt_gp_members * 4)) >
		     cmd->data_length) {
			rd_len += 8 + (tg_pt_gp->tg_pt_gp_members * 4);
			continue;
		}
    ...
		spin_lock(&tg_pt_gp->tg_pt_gp_lock);
		list_for_each_entry(lun, &tg_pt_gp->tg_pt_gp_lun_list,
				lun_tg_pt_gp_link) {
			off += 2; /* Skip over Obsolete */
			put_unaligned_be16(lun->lun_tpg->tpg_rtpi, &buf[off]);
			off += 2;
			rd_len += 4;
		}
		spin_unlock(&tg_pt_gp->tg_pt_gp_lock);
    ...
}

target_emulate_report_target_port_groups() checks whether the descriptors for
a port group fit within cmd->data_length using tg_pt_gp->tg_pt_gp_members
without holding tg_pt_gp->tg_pt_gp_lock:

CPU0                                  CPU1
target_emulate_report_target_port_groups()
  if ((off + 8 +
       (tg_pt_gp->tg_pt_gp_members * 4)) >
      cmd->data_length)
                                      target_attach_tg_pt_gp()
                                        __target_attach_tg_pt_gp()
                                          spin_lock(&tg_pt_gp->tg_pt_gp_lock)
                                          list_add_tail(...,
                                            &tg_pt_gp->tg_pt_gp_lun_list)
                                          tg_pt_gp->tg_pt_gp_members++
                                          spin_unlock(&tg_pt_gp->tg_pt_gp_lock)
  spin_lock(&tg_pt_gp->tg_pt_gp_lock)
  list_for_each_entry(lun, ...)
    put_unaligned_be16(..., &buf[off])  <-- out-of-bounds write

When a concurrent configfs operation attaches a LUN to the same port group
between the size check and acquiring tg_pt_gp->tg_pt_gp_lock,
tg_pt_gp_lun_list grows past the validated count and the loop writes past the
end of the mapped command buffer.

[1] https://sashiko.dev/#/bug/linux-9cff51b1-852e-4bef-94e2-6d7c000ddc21

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-bug-target-alua-lun-gp-v1-0-127a46efdae9@linux.dev?part=1