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

[David Disseldorp <ddiss@suse.de>]

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>