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

[sashiko-bot@kernel.org]

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