[PATCH v3] scsi: target: fix iSCSI ISID use-after-free in REGISTER AND MOVE

[PATCH v3] scsi: target: fix iSCSI ISID use-after-free in REGISTER AND MOVE

[Bryam Vargas]

core_scsi3_emulate_pro_register_and_move() maps the PERSISTENT RESERVE OUT
parameter list with transport_kmap_data_sg() and parses the destination
TransportID with target_parse_pr_out_transport_id(). For an iSCSI
TransportID (FORMAT CODE 01b), iscsi_parse_pr_out_transport_id() returns
the ISID in iport_ptr as a raw pointer into that mapped buffer.

The function then unmaps the buffer with transport_kunmap_data_sg() before
dereferencing iport_ptr in strcmp(), __core_scsi3_locate_pr_reg() and
core_scsi3_alloc_registration(). When the parameter list spans more than
one page (PARAMETER LIST LENGTH > 4096), transport_kmap_data_sg() uses
vmap() and transport_kunmap_data_sg() does vunmap(), so the kernel virtual
address backing iport_ptr is torn down and every subsequent dereference is
a use-after-free read of the unmapped region.

Keep the parameter list mapped until iport_ptr is no longer needed: drop
the early transport_kunmap_data_sg() and unmap once on the success path,
right before returning. The error paths already unmap through the existing
"if (buf) transport_kunmap_data_sg(cmd)" at the out: label, which now runs
on every post-map error exit because buf is no longer cleared early. Only
reads of the mapping happen while spinlocks are held; the map and unmap
calls remain outside any lock. The sibling caller
core_scsi3_decode_spec_i_port() already uses the buffer before unmapping it
and is left unchanged.

Fixes: 4949314c7283 ("target: Allow control CDBs with data > 1 page")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
v3 (review of v2 by John Garry and James Bottomley):
  - Drop the parser-ownership approach. Rather than copy the ISID into an
    allocation that both callers must kfree() (v2), keep the PR-OUT
    parameter list mapped across the iport_ptr dereferences and move the
    single unmap to the success path. No allocation, no kfree, and
    target_core_fabric_lib.c / core_scsi3_decode_spec_i_port() are
    unchanged. This is the form John Garry asked for ("keep the mapping in
    place for longer, until the out: label") and removes the multiple
    kfree() paths v2 added.
  - Re: holding the mapping while spinlocks are held (raised on v2): only
    reads of the mapping occur under dev_reservation_lock; the kmap/kunmap
    calls are all outside any lock, so there is no sleep-in-atomic concern.

v2: https://lore.kernel.org/linux-scsi/20260609005858.17504-1-hexlabsecurity@proton.me/
v1: https://lore.kernel.org/linux-scsi/20260606015359.181724-1-hexlabsecurity@proton.me/

Class / impact: CWE-416 use-after-free (use-after-vunmap) in the LIO SCSI
target, reachable by an authenticated iSCSI initiator that is a current
Persistent Reservation registrant on the LUN. It sends PERSISTENT RESERVE
OUT / REGISTER AND MOVE with an iSCSI (FORMAT CODE 01b) TransportID and a
PARAMETER LIST LENGTH > 4096 so the parameter list spans >1 page and is
mapped with vmap(). After transport_kunmap_data_sg() vunmap()s that region,
the retained iport_ptr is dereferenced -> kernel read of an unmapped
vmalloc address (oops / DoS). Primarily a remotely reachable authenticated
denial of service. Present in all maintained trees since 4949314c7283
(v3.3, 2012), which introduced the multi-page vmap() path. Verified at
mainline v7.1-rc6 and stable v6.12.92.

Reproducer (authenticated iSCSI initiator, current PR reservation holder):
  1. PERSISTENT RESERVE OUT / REGISTER a key from the iSCSI nexus.
  2. PERSISTENT RESERVE OUT / REGISTER AND MOVE, FORMAT CODE 01b TransportID
     (IQN + ",i,0x" + 12-char ISID), RELATIVE TARGET PORT IDENTIFIER of an
     existing target port, with PARAMETER LIST LENGTH = 8192 (two pages ->
     vmap()/vunmap()), the inner ADDITIONAL LENGTH set so tid_len + 24 ==
     data_length, the remainder zero padding.

A/B verification (CONFIG_KASAN_VMALLOC=y, kasan.fault=report, x86-64,
6.12.90; same kernel for every arm; 64-bit and 32-bit initiator):
  - Without this patch (8192-byte, two-page request):
      BUG: KASAN: vmalloc-out-of-bounds in strcmp+0xa7/0xb0
        strcmp
        core_scsi3_emulate_pro_register_and_move [target_core]
        ? remove_vm_area
        target_scsi3_emulate_pr_out [target_core]
        __target_execute_cmd / iscsit_execute_cmd / iscsi_target_rx_thread
      followed by "unable to handle page fault" (PTE 0); the command never
      completes (the iSCSI rx kthread dies).
  - Control (128-byte, single-page request): no report (kunmap is a no-op
    on 64-bit !HIGHMEM, no vunmap).
  - With this patch (same 8192-byte request, 64-bit and 32-bit initiator):
    no report, the command completes.

 drivers/target/target_core_pr.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index 11790f2c5d80..7c5bb7d67947 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -3293,9 +3293,6 @@ core_scsi3_emulate_pro_register_and_move(struct se_cmd *cmd, u64 res_key,
 		goto out;
 	}
 
-	transport_kunmap_data_sg(cmd);
-	buf = NULL;
-
 	pr_debug("SPC-3 PR [%s] Extracted initiator %s identifier: %s"
 		" %s\n", dest_tf_ops->fabric_name, (iport_ptr != NULL) ?
 		"port" : "device", initiator_str, (iport_ptr != NULL) ?
@@ -3532,6 +3529,11 @@ core_scsi3_emulate_pro_register_and_move(struct se_cmd *cmd, u64 res_key,
 	core_scsi3_update_and_write_aptpl(cmd->se_dev, aptpl);
 
 	core_scsi3_put_pr_reg(dest_pr_reg);
+	/*
+	 * iport_ptr aliases the PR-OUT parameter list mapped above, so the
+	 * buffer is unmapped only here on success (and at out: on error).
+	 */
+	transport_kunmap_data_sg(cmd);
 	return 0;
 out:
 	if (buf)
-- 
2.43.0

Re: [PATCH v3] scsi: target: fix iSCSI ISID use-after-free in REGISTER AND MOVE

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Missing length validation on the TransportID payload leads to out-of-bounds reads and potential kernel oops via `strstr()` and `get_unaligned_be64()`.
--

commit f95f1e5f1cdc7f67d3f1cb876ff0263d03032d49
Author: Bryam Vargas <hexlabsecurity@proton.me>

scsi: target: fix iSCSI ISID use-after-free in REGISTER AND MOVE

This commit fixes a use-after-free read of the iscsi isid pointer in
core_scsi3_emulate_pro_register_and_move(). The early unmap of the buffer
is dropped, keeping the parameter list mapped until the pointer is no
longer needed.

> diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
> index 11790f2c5d80f..7c5bb7d679477 100644
> --- a/drivers/target/target_core_pr.c
> +++ b/drivers/target/target_core_pr.c
> @@ -3293,9 +3293,6 @@ core_scsi3_emulate_pro_register_and_move(struct se_cmd *cmd, u64 res_key,
>  		goto out;
>  	}
>  
> -	transport_kunmap_data_sg(cmd);
> -	buf = NULL;
> -

[Severity: Critical]
This is a pre-existing issue, but does this code properly validate the
payload length before parsing the buffer?

Earlier in core_scsi3_emulate_pro_register_and_move(), proto_ident is read
from buf[24] without verifying that tid_len > 0. If a command is sent with
tid_len = 0 and data_length = 24, would reading buf[24] result in an
out-of-bounds read?

Furthermore, iscsi_parse_pr_out_transport_id() calls:

    p = strstr(&buf[4], ",i,0x");

If a payload length is exactly aligned to a multiple of the page size with no
null terminator, could strstr() scan past the vmap buffer and hit a guard
page, causing a kernel oops?

Finally, if the parsed isid string is found but terminates quickly near a page
boundary (for example, ",i,0x1\0"), __core_scsi3_do_alloc_registration()
executes:

    pr_reg->pr_reg_bin_isid = get_unaligned_be64(isid);

Since get_unaligned_be64() unconditionally reads 8 bytes from the potentially
short ascii string, could this read cross into a guard page and cause an oops?

>  	pr_debug("SPC-3 PR [%s] Extracted initiator %s identifier: %s"
>  		" %s\n", dest_tf_ops->fabric_name, (iport_ptr != NULL) ?
>  		"port" : "device", initiator_str, (iport_ptr != NULL) ?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610042245.35473-1-hexlabsecurity@proton.me?part=1

Re: [PATCH v3] scsi: target: fix iSCSI ISID use-after-free in REGISTER AND MOVE

[John Garry]

On 10/06/2026 05:22, Bryam Vargas wrote:
> core_scsi3_emulate_pro_register_and_move() maps the PERSISTENT RESERVE OUT
> parameter list with transport_kmap_data_sg() and parses the destination
> TransportID with target_parse_pr_out_transport_id(). For an iSCSI
> TransportID (FORMAT CODE 01b), iscsi_parse_pr_out_transport_id() returns
> the ISID in iport_ptr as a raw pointer into that mapped buffer.
> 
> The function then unmaps the buffer with transport_kunmap_data_sg() before
> dereferencing iport_ptr in strcmp(), __core_scsi3_locate_pr_reg() and
> core_scsi3_alloc_registration(). When the parameter list spans more than
> one page (PARAMETER LIST LENGTH > 4096), transport_kmap_data_sg() uses
> vmap() and transport_kunmap_data_sg() does vunmap(), so the kernel virtual
> address backing iport_ptr is torn down and every subsequent dereference is
> a use-after-free read of the unmapped region.
> 
> Keep the parameter list mapped until iport_ptr is no longer needed: drop
> the early transport_kunmap_data_sg() and unmap once on the success path,
> right before returning. The error paths already unmap through the existing
> "if (buf) transport_kunmap_data_sg(cmd)" at the out: label, which now runs
> on every post-map error exit because buf is no longer cleared early. Only
> reads of the mapping happen while spinlocks are held; the map and unmap
> calls remain outside any lock. The sibling caller
> core_scsi3_decode_spec_i_port() already uses the buffer before unmapping it
> and is left unchanged.
> 
> Fixes: 4949314c7283 ("target: Allow control CDBs with data > 1 page")
> Cc:stable@vger.kernel.org
> Signed-off-by: Bryam Vargas<hexlabsecurity@proton.me>

FWIW:
Reviewed-by: John Garry <john.g.garry@oracle.com>

Re: [PATCH v3] scsi: target: fix iSCSI ISID use-after-free in REGISTER AND MOVE

[David Disseldorp]

On Wed, 10 Jun 2026 04:22:48 +0000, Bryam Vargas wrote:

> core_scsi3_emulate_pro_register_and_move() maps the PERSISTENT RESERVE OUT
> parameter list with transport_kmap_data_sg() and parses the destination
> TransportID with target_parse_pr_out_transport_id(). For an iSCSI
> TransportID (FORMAT CODE 01b), iscsi_parse_pr_out_transport_id() returns
> the ISID in iport_ptr as a raw pointer into that mapped buffer.
> 
> The function then unmaps the buffer with transport_kunmap_data_sg() before
> dereferencing iport_ptr in strcmp(), __core_scsi3_locate_pr_reg() and
> core_scsi3_alloc_registration(). When the parameter list spans more than
> one page (PARAMETER LIST LENGTH > 4096), transport_kmap_data_sg() uses
> vmap() and transport_kunmap_data_sg() does vunmap(), so the kernel virtual
> address backing iport_ptr is torn down and every subsequent dereference is
> a use-after-free read of the unmapped region.
> 
> Keep the parameter list mapped until iport_ptr is no longer needed: drop
> the early transport_kunmap_data_sg() and unmap once on the success path,
> right before returning. The error paths already unmap through the existing
> "if (buf) transport_kunmap_data_sg(cmd)" at the out: label, which now runs
> on every post-map error exit because buf is no longer cleared early. Only
> reads of the mapping happen while spinlocks are held; the map and unmap
> calls remain outside any lock. The sibling caller
> core_scsi3_decode_spec_i_port() already uses the buffer before unmapping it
> and is left unchanged.
> 
> Fixes: 4949314c7283 ("target: Allow control CDBs with data > 1 page")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
> ---
> v3 (review of v2 by John Garry and James Bottomley):
>   - Drop the parser-ownership approach. Rather than copy the ISID into an
>     allocation that both callers must kfree() (v2), keep the PR-OUT
>     parameter list mapped across the iport_ptr dereferences and move the
>     single unmap to the success path. No allocation, no kfree, and
>     target_core_fabric_lib.c / core_scsi3_decode_spec_i_port() are
>     unchanged. This is the form John Garry asked for ("keep the mapping in
>     place for longer, until the out: label") and removes the multiple
>     kfree() paths v2 added.
>   - Re: holding the mapping while spinlocks are held (raised on v2): only
>     reads of the mapping occur under dev_reservation_lock; the kmap/kunmap
>     calls are all outside any lock, so there is no sleep-in-atomic concern.
> 
> v2: https://lore.kernel.org/linux-scsi/20260609005858.17504-1-hexlabsecurity@proton.me/
> v1: https://lore.kernel.org/linux-scsi/20260606015359.181724-1-hexlabsecurity@proton.me/
> 
> Class / impact: CWE-416 use-after-free (use-after-vunmap) in the LIO SCSI
> target, reachable by an authenticated iSCSI initiator that is a current
> Persistent Reservation registrant on the LUN. It sends PERSISTENT RESERVE
> OUT / REGISTER AND MOVE with an iSCSI (FORMAT CODE 01b) TransportID and a
> PARAMETER LIST LENGTH > 4096 so the parameter list spans >1 page and is
> mapped with vmap(). After transport_kunmap_data_sg() vunmap()s that region,
> the retained iport_ptr is dereferenced -> kernel read of an unmapped
> vmalloc address (oops / DoS). Primarily a remotely reachable authenticated
> denial of service. Present in all maintained trees since 4949314c7283
> (v3.3, 2012), which introduced the multi-page vmap() path. Verified at
> mainline v7.1-rc6 and stable v6.12.92.
> 
> Reproducer (authenticated iSCSI initiator, current PR reservation holder):
>   1. PERSISTENT RESERVE OUT / REGISTER a key from the iSCSI nexus.
>   2. PERSISTENT RESERVE OUT / REGISTER AND MOVE, FORMAT CODE 01b TransportID
>      (IQN + ",i,0x" + 12-char ISID), RELATIVE TARGET PORT IDENTIFIER of an
>      existing target port, with PARAMETER LIST LENGTH = 8192 (two pages ->
>      vmap()/vunmap()), the inner ADDITIONAL LENGTH set so tid_len + 24 ==
>      data_length, the remainder zero padding.
> 
> A/B verification (CONFIG_KASAN_VMALLOC=y, kasan.fault=report, x86-64,
> 6.12.90; same kernel for every arm; 64-bit and 32-bit initiator):
>   - Without this patch (8192-byte, two-page request):
>       BUG: KASAN: vmalloc-out-of-bounds in strcmp+0xa7/0xb0
>         strcmp
>         core_scsi3_emulate_pro_register_and_move [target_core]
>         ? remove_vm_area
>         target_scsi3_emulate_pr_out [target_core]
>         __target_execute_cmd / iscsit_execute_cmd / iscsi_target_rx_thread
>       followed by "unable to handle page fault" (PTE 0); the command never
>       completes (the iSCSI rx kthread dies).
>   - Control (128-byte, single-page request): no report (kunmap is a no-op
>     on 64-bit !HIGHMEM, no vunmap).
>   - With this patch (same 8192-byte request, 64-bit and 32-bit initiator):
>     no report, the command completes.
> 
>  drivers/target/target_core_pr.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
> index 11790f2c5d80..7c5bb7d67947 100644
> --- a/drivers/target/target_core_pr.c
> +++ b/drivers/target/target_core_pr.c
> @@ -3293,9 +3293,6 @@ core_scsi3_emulate_pro_register_and_move(struct se_cmd *cmd, u64 res_key,
>  		goto out;
>  	}
>  
> -	transport_kunmap_data_sg(cmd);
> -	buf = NULL;
> -
>  	pr_debug("SPC-3 PR [%s] Extracted initiator %s identifier: %s"
>  		" %s\n", dest_tf_ops->fabric_name, (iport_ptr != NULL) ?
>  		"port" : "device", initiator_str, (iport_ptr != NULL) ?
> @@ -3532,6 +3529,11 @@ core_scsi3_emulate_pro_register_and_move(struct se_cmd *cmd, u64 res_key,
>  	core_scsi3_update_and_write_aptpl(cmd->se_dev, aptpl);
>  
>  	core_scsi3_put_pr_reg(dest_pr_reg);
> +	/*
> +	 * iport_ptr aliases the PR-OUT parameter list mapped above, so the
> +	 * buffer is unmapped only here on success (and at out: on error).
> +	 */
> +	transport_kunmap_data_sg(cmd);
>  	return 0;
>  out:
>  	if (buf)

Looks good.
Reviewed-by: David Disseldorp <ddiss@suse.de>