[PATCH] scsi: target: copy iSCSI ISID before unmapping the PR OUT buffer

[PATCH] scsi: target: copy iSCSI ISID before unmapping the PR OUT buffer

[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
iport_ptr as a raw pointer into that mapped buffer (the ISID following the
",i,0x" separator).

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() (the last reads 8 bytes via
get_unaligned_be64() and copies the string with snprintf()). 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 on
all architectures and every subsequent dereference is a use-after-free of
the unmapped region.

initiator_str does not have this problem because the parser strscpy()s it
into a caller-owned buffer; iport_ptr is the only output left as a borrowed
alias. core_scsi3_decode_spec_i_port() consumes the same alias safely
because it unmaps only after all uses.

Copy the ISID into a caller-owned stack buffer while the mapping is still
live and repoint iport_ptr at it, mirroring the existing initiator_str
handling. strscpy_pad() NUL-terminates and zero-fills the tail so the fixed
8-byte get_unaligned_be64() read stays in-bounds and deterministic even for
an ISID shorter than 8 bytes. The NULL (device-format / non-iSCSI) case is
preserved by copying only when iport_ptr is non-NULL.

Fixes: 4949314c7283 ("target: Allow control CDBs with data > 1 page")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
Everything below the --- is dropped by git am.

Class / impact: CWE-416 use-after-free (use-after-vunmap) in the LIO SCSI
target. Triggerable 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; memory-safety corruption confirmed by KASAN).
Primarily a remotely-reachable authenticated denial of service.

Affected: all maintained trees -- the bug predates the git history reachable
here; it became a destructive dangling dereference with 4949314c7283 (v3.3,
2012), which introduced the multi-page vmap() path. Verified present 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; reproduced with both a 64-bit and a 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
      The buggy address belongs to a vmalloc virtual mapping
      BUG: unable to handle page fault for address ... (PTE 0)
  - Control (56/128-byte, single-page request): no report (kunmap is a
    no-op on 64-bit !HIGHMEM, so the alias stays valid) -- confirming the
    multi-page vmap()/vunmap() path is what makes iport_ptr dangle.
  - With this patch (same 8192-byte request): no report, command completes.

 drivers/target/target_core_pr.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index 11790f2c5d80..b102f5f67793 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -3160,6 +3160,7 @@ core_scsi3_emulate_pro_register_and_move(struct se_cmd *cmd, u64 res_key,
 	unsigned char *buf;
 	unsigned char initiator_str[TRANSPORT_IQN_LEN];
 	char *iport_ptr = NULL, i_buf[PR_REG_ISID_ID_LEN] = { };
+	char isid_buf[PR_REG_ISID_LEN] = { };
 	u32 tid_len, tmp_tid_len;
 	int new_reg = 0, type, scope, matching_iname;
 	sense_reason_t ret;
@@ -3293,6 +3294,22 @@ core_scsi3_emulate_pro_register_and_move(struct se_cmd *cmd, u64 res_key,
 		goto out;
 	}

+	/*
+	 * For an iSCSI TransportID, iport_ptr aliases directly into the data
+	 * buffer mapped above.  When that buffer spans more than one page it is
+	 * a vmap() region that transport_kunmap_data_sg() is about to vunmap(),
+	 * tearing down the kernel mapping and leaving iport_ptr dangling for
+	 * every consumer below.  Copy the ISID into caller-owned storage now,
+	 * while the mapping is still live.  strscpy_pad() NUL-terminates and
+	 * zero-fills the tail so the later 8-byte get_unaligned_be64() read in
+	 * __core_scsi3_do_alloc_registration() stays in-bounds and deterministic
+	 * even for an ISID shorter than 8 bytes.
+	 */
+	if (iport_ptr) {
+		strscpy_pad(isid_buf, iport_ptr, sizeof(isid_buf));
+		iport_ptr = isid_buf;
+	}
+
 	transport_kunmap_data_sg(cmd);
 	buf = NULL;

Re: [PATCH] scsi: target: copy iSCSI ISID before unmapping the PR OUT buffer

[John Garry]

On 06/06/2026 02:54, 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
> iport_ptr as a raw pointer into that mapped buffer (the ISID following the
> ",i,0x" separator).
> 
> 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() (the last reads 8 bytes via
> get_unaligned_be64() and copies the string with snprintf()). 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 on
> all architectures and every subsequent dereference is a use-after-free of
> the unmapped region.
> 
> initiator_str does not have this problem because the parser strscpy()s it
> into a caller-owned buffer; iport_ptr is the only output left as a borrowed
> alias. core_scsi3_decode_spec_i_port() consumes the same alias safely
> because it unmaps only after all uses.
> 
> Copy the ISID into a caller-owned stack buffer while the mapping is still
> live and repoint iport_ptr at it, mirroring the existing initiator_str
> handling. strscpy_pad() NUL-terminates and zero-fills the tail so the fixed
> 8-byte get_unaligned_be64() read stays in-bounds and deterministic even for
> an ISID shorter than 8 bytes. The NULL (device-format / non-iSCSI) case is
> preserved by copying only when iport_ptr is non-NULL.
> 
> Fixes: 4949314c7283 ("target: Allow control CDBs with data > 1 page")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
> ---
> Everything below the --- is dropped by git am.
> 
> Class / impact: CWE-416 use-after-free (use-after-vunmap) in the LIO SCSI
> target. Triggerable 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; memory-safety corruption confirmed by KASAN).
> Primarily a remotely-reachable authenticated denial of service.
> 
> Affected: all maintained trees -- the bug predates the git history reachable
> here; it became a destructive dangling dereference with 4949314c7283 (v3.3,
> 2012), which introduced the multi-page vmap() path. Verified present 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; reproduced with both a 64-bit and a 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
>        The buggy address belongs to a vmalloc virtual mapping
>        BUG: unable to handle page fault for address ... (PTE 0)
>    - Control (56/128-byte, single-page request): no report (kunmap is a
>      no-op on 64-bit !HIGHMEM, so the alias stays valid) -- confirming the
>      multi-page vmap()/vunmap() path is what makes iport_ptr dangle.
>    - With this patch (same 8192-byte request): no report, command completes.
> 
>   drivers/target/target_core_pr.c | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
> index 11790f2c5d80..b102f5f67793 100644
> --- a/drivers/target/target_core_pr.c
> +++ b/drivers/target/target_core_pr.c
> @@ -3160,6 +3160,7 @@ core_scsi3_emulate_pro_register_and_move(struct se_cmd *cmd, u64 res_key,
>   	unsigned char *buf;
>   	unsigned char initiator_str[TRANSPORT_IQN_LEN];
>   	char *iport_ptr = NULL, i_buf[PR_REG_ISID_ID_LEN] = { };
> +	char isid_buf[PR_REG_ISID_LEN] = { };
>   	u32 tid_len, tmp_tid_len;
>   	int new_reg = 0, type, scope, matching_iname;
>   	sense_reason_t ret;
> @@ -3293,6 +3294,22 @@ core_scsi3_emulate_pro_register_and_move(struct se_cmd *cmd, u64 res_key,
>   		goto out;
>   	}
> 
> +	/*
> +	 * For an iSCSI TransportID, iport_ptr aliases directly into the data
> +	 * buffer mapped above.  When that buffer spans more than one page it is
> +	 * a vmap() region that transport_kunmap_data_sg() is about to vunmap(),
> +	 * tearing down the kernel mapping and leaving iport_ptr dangling for
> +	 * every consumer below.  Copy the ISID into caller-owned storage now,
> +	 * while the mapping is still live.  strscpy_pad() NUL-terminates and
> +	 * zero-fills the tail so the later 8-byte get_unaligned_be64() read in
> +	 * __core_scsi3_do_alloc_registration() stays in-bounds and deterministic
> +	 * even for an ISID shorter than 8 bytes.
> +	 */
> +	if (iport_ptr) {
> +		strscpy_pad(isid_buf, iport_ptr, sizeof(isid_buf));
> +		iport_ptr = isid_buf;

It's not so nice to re-assign the pointer like this or have it even 
pointing at a local array.

Is it really messy for iscsi_parse_pr_out_transport_id() to do something 
like kstrdup and then the caller has the job of later free'ing it?

Thanks,
John

Re: [PATCH] scsi: target: copy iSCSI ISID before unmapping the PR OUT buffer

[Bryam Vargas]

On 06/06/2026, John Garry wrote:
> It's not so nice to re-assign the pointer like this or have it even
> pointing at a local array.
>
> Is it really messy for iscsi_parse_pr_out_transport_id() to do something
> like kstrdup and then the caller has the job of later free'ing it?

You are right -- v2 moves the copy into iscsi_parse_pr_out_transport_id()
so the parser returns an owned allocation via *port_nexus_ptr and callers
kfree() it.

We use kzalloc(PR_REG_ISID_LEN) + strscpy_pad() rather than plain kstrdup()
because __core_scsi3_do_alloc_registration() reads the ISID with a fixed
8-byte get_unaligned_be64(isid).  A malformed TransportID with an ISID
shorter than 8 characters would give a kstrdup allocation smaller than 8
bytes, turning that read into a heap out-of-bounds.  kzalloc zero-fills the
full PR_REG_ISID_LEN (16) bytes so the be64 read is always in-bounds and
returns a deterministic value; strscpy_pad() copies the lowercased ISID and
NUL-fills the tail.

core_scsi3_decode_spec_i_port() also receives the allocated pointer and
needs to kfree() it.  The inner list_for_each_entry iterates over multiple
TPGs per TransportID, calling the parser at each; we kfree(iport_ptr) before
the reset at the top of each inner-loop iteration so a failed ACL match that
triggers a continue does not leak the previous parse's allocation.
kfree(iport_ptr) is also added at out_unmap: (error exit) and before
return 0 (success exit).

[PATCH v2] follows.