Skip to content

Commit d04a179

Browse files
bryamzxzmartinkpetersen
authored andcommitted
scsi: target: Bound PR-OUT TransportID parsing to the received buffer
core_scsi3_decode_spec_i_port() and core_scsi3_emulate_register_and_move() hand the raw PERSISTENT RESERVE OUT parameter buffer to target_parse_pr_out_transport_id() without telling it how many bytes are valid. For an iSCSI TransportID (FORMAT CODE 01b), iscsi_parse_pr_out_transport_id() locates the ",i,0x" ISID separator with an unbounded strstr() (and on the error path prints the name with a further unbounded "%s"). An initiator can submit a TransportID whose iSCSI name contains neither a ",i,0x" substring nor a NUL terminator, filling the parameter list to its end, so the scan runs off the end of the buffer. When the parameter list spans more than one page the buffer is a multi-page vmap (transport_kmap_data_sg()), so the over-read walks into the trailing vmalloc guard page and oopses (KASAN: vmalloc-out-of-bounds in strstr). It is reachable by any fabric that delivers a PR OUT to a device exported through an iSCSI TPG, including a guest via vhost-scsi. Pass the number of received bytes down to the parser and validate the iSCSI TransportID's own self-described length (ADDITIONAL LENGTH + 4) once, up front: reject it if it is below the spc4r17 minimum or larger than the received buffer, then bound the separator search, the ISID walk and the name copy by that length. This is the length check the callers already perform after the parse (core_scsi3_decode_spec_i_port() compares tid_len against tpdl, core_scsi3_emulate_register_and_move() validates it against data_length), moved ahead of the scan. Also drop the unbounded "%s" of the unterminated name. Add per-format explicit name-length checks before copying into i_str, rather than silently truncating with min_t: for FORMAT CODE 00b reject if the descriptor body (tid_len - 4 bytes) cannot fit in i_str[TRANSPORT_IQN_LEN]; for FORMAT CODE 01b reject if the name portion (from &buf[4] up to the separator) cannot fit. Both checks make the bounds intent explicit at each format branch. While here, also reject a FORMAT CODE 01b TransportID whose ",i,0x" separator sits at the very end of the descriptor: that leaves an empty ISID and points the returned port nexus pointer at buf + tid_len, one past the descriptor, which the registration code (__core_scsi3_locate_pr_reg(), __core_scsi3_alloc_registration()) then dereferences as the ISID string -- the same over-read of the parameter buffer for a malformed descriptor. Fixes: c66ac9d ("[SCSI] target: Add LIO target core v4.0.0-rc6") Cc: stable@vger.kernel.org Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me> Reviewed-by: John Garry <john.g.garry@oracle.com> Reviewed-by: David Disseldorp <ddiss@suse.de> Link: https://patch.msgid.link/20260611-b4-disp-9f20739e-v6-1-f6630e2aae44@proton.me Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
1 parent 1bd2862 commit d04a179

3 files changed

Lines changed: 73 additions & 23 deletions

File tree

drivers/target/target_core_fabric_lib.c

Lines changed: 69 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -290,13 +290,24 @@ static void sbp_parse_pr_out_transport_id(char *buf, char *i_str)
290290
static bool iscsi_parse_pr_out_transport_id(
291291
struct se_portal_group *se_tpg,
292292
char *buf,
293+
u32 buf_len,
293294
u32 *out_tid_len,
294295
char **port_nexus_ptr,
295296
char *i_str)
296297
{
297298
char *p;
299+
u32 tid_len;
298300
int i;
299-
u8 format_code = (buf[0] & 0xc0);
301+
u8 format_code;
302+
303+
/*
304+
* The 4-byte iSCSI TransportID header (FORMAT CODE + 2-byte ADDITIONAL
305+
* LENGTH) must be present before any of it can be parsed.
306+
*/
307+
if (buf_len < 4)
308+
return false;
309+
310+
format_code = buf[0] & 0xc0;
300311
/*
301312
* Check for FORMAT CODE 00b or 01b from spc4r17, section 7.5.4.6:
302313
*
@@ -316,40 +327,58 @@ static bool iscsi_parse_pr_out_transport_id(
316327
return false;
317328
}
318329
/*
319-
* If the caller wants the TransportID Length, we set that value for the
320-
* entire iSCSI Tarnsport ID now.
330+
* Reconstruct the self-described TransportID length from the ADDITIONAL
331+
* LENGTH field plus the 4-byte header. Reject it if it is below the
332+
* spc4r17 section 7.5.4.6 minimum (ADDITIONAL LENGTH shall be at least
333+
* 20) or if it runs past the bytes actually received, so that every
334+
* access below stays inside the TransportID.
321335
*/
322-
if (out_tid_len) {
323-
/* The shift works thanks to integer promotion rules */
324-
*out_tid_len = get_unaligned_be16(&buf[2]);
325-
/* Add four bytes for iSCSI Transport ID header */
326-
*out_tid_len += 4;
327-
}
336+
tid_len = get_unaligned_be16(&buf[2]) + 4;
337+
if (tid_len < 24 || tid_len > buf_len)
338+
return false;
339+
if (out_tid_len)
340+
*out_tid_len = tid_len;
328341

329342
/*
330343
* Check for ',i,0x' separator between iSCSI Name and iSCSI Initiator
331344
* Session ID as defined in Table 390 - iSCSI initiator port TransportID
332345
* format.
333346
*/
334347
if (format_code == 0x40) {
335-
p = strstr(&buf[4], ",i,0x");
348+
p = strnstr(&buf[4], ",i,0x", tid_len - 4);
336349
if (!p) {
337-
pr_err("Unable to locate \",i,0x\" separator"
338-
" for Initiator port identifier: %s\n",
339-
&buf[4]);
350+
pr_err("Unable to locate \",i,0x\" separator in iSCSI TransportID\n");
351+
return false;
352+
}
353+
/*
354+
* The iSCSI name runs from &buf[4] up to the separator; reject it
355+
* if it cannot fit in i_str[TRANSPORT_IQN_LEN].
356+
*/
357+
if (p - &buf[4] >= TRANSPORT_IQN_LEN) {
358+
pr_err("iSCSI Initiator port name too long in TransportID\n");
340359
return false;
341360
}
342361
*p = '\0'; /* Terminate iSCSI Name */
343362
p += 5; /* Skip over ",i,0x" separator */
344363

364+
/*
365+
* The ISID must follow the separator. A ",i,0x" sitting at the
366+
* very end of the TransportID leaves no ISID and would point the
367+
* port nexus at buf + tid_len, i.e. past the descriptor, which
368+
* the registration code then reads as the ISID string.
369+
*/
370+
if (p >= buf + tid_len) {
371+
pr_err("Missing ISID in iSCSI Initiator port TransportID\n");
372+
return false;
373+
}
345374
*port_nexus_ptr = p;
346375
/*
347376
* Go ahead and do the lower case conversion of the received
348377
* 12 ASCII characters representing the ISID in the TransportID
349378
* for comparison against the running iSCSI session's ISID from
350379
* iscsi_target.c:lio_sess_get_initiator_sid()
351380
*/
352-
for (i = 0; i < 12; i++) {
381+
for (i = 0; i < 12 && p < buf + tid_len; i++) {
353382
/*
354383
* The first ISCSI INITIATOR SESSION ID field byte
355384
* containing an ASCII null character terminates the
@@ -367,10 +396,22 @@ static bool iscsi_parse_pr_out_transport_id(
367396
*p = tolower(*p);
368397
p++;
369398
}
370-
} else
399+
strscpy(i_str, &buf[4], TRANSPORT_IQN_LEN);
400+
} else {
371401
*port_nexus_ptr = NULL;
372-
373-
strscpy(i_str, &buf[4], TRANSPORT_IQN_LEN);
402+
/*
403+
* FORMAT CODE 00b: the name occupies buf[4..tid_len-1]. The
404+
* declared length tid_len - 4 must fit in i_str[TRANSPORT_IQN_LEN].
405+
* (For 01b the same tid_len bound would be over-restrictive: the
406+
* descriptor also carries the separator and ISID, so a legal
407+
* <=223-byte name gives tid_len up to 244.)
408+
*/
409+
if (tid_len - 4 >= TRANSPORT_IQN_LEN) {
410+
pr_err("iSCSI Initiator port name too long in TransportID\n");
411+
return false;
412+
}
413+
strscpy(i_str, &buf[4], tid_len - 4);
414+
}
374415
return true;
375416
}
376417

@@ -420,8 +461,16 @@ int target_get_pr_transport_id(struct se_node_acl *nacl,
420461
}
421462

422463
bool target_parse_pr_out_transport_id(struct se_portal_group *tpg,
423-
char *buf, u32 *out_tid_len, char **port_nexus_ptr, char *i_str)
464+
char *buf, u32 buf_len, u32 *out_tid_len,
465+
char **port_nexus_ptr, char *i_str)
424466
{
467+
/*
468+
* The fixed-length SAS/SRP/FCP/SBP TransportIDs are 24 bytes; the iSCSI
469+
* format is variable and bounds itself against buf_len below.
470+
*/
471+
if (tpg->proto_id != SCSI_PROTOCOL_ISCSI && buf_len < 24)
472+
return false;
473+
425474
switch (tpg->proto_id) {
426475
case SCSI_PROTOCOL_SAS:
427476
/*
@@ -440,8 +489,8 @@ bool target_parse_pr_out_transport_id(struct se_portal_group *tpg,
440489
sbp_parse_pr_out_transport_id(buf, i_str);
441490
break;
442491
case SCSI_PROTOCOL_ISCSI:
443-
return iscsi_parse_pr_out_transport_id(tpg, buf, out_tid_len,
444-
port_nexus_ptr, i_str);
492+
return iscsi_parse_pr_out_transport_id(tpg, buf, buf_len,
493+
out_tid_len, port_nexus_ptr, i_str);
445494
default:
446495
pr_err("Unknown proto_id: 0x%02x\n", tpg->proto_id);
447496
return false;

drivers/target/target_core_internal.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,8 @@ int target_get_pr_transport_id(struct se_node_acl *nacl,
104104
struct t10_pr_registration *pr_reg, int *format_code,
105105
unsigned char *buf);
106106
bool target_parse_pr_out_transport_id(struct se_portal_group *tpg,
107-
char *buf, u32 *out_tid_len, char **port_nexus_ptr, char *i_str);
107+
char *buf, u32 buf_len, u32 *out_tid_len,
108+
char **port_nexus_ptr, char *i_str);
108109

109110
/* target_core_hba.c */
110111
struct se_hba *core_alloc_hba(const char *, u32, u32);

drivers/target/target_core_pr.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1573,7 +1573,7 @@ core_scsi3_decode_spec_i_port(
15731573

15741574
iport_ptr = NULL;
15751575
tid_found = target_parse_pr_out_transport_id(tmp_tpg,
1576-
ptr, &tid_len, &iport_ptr, i_str);
1576+
ptr, tpdl, &tid_len, &iport_ptr, i_str);
15771577
if (!tid_found)
15781578
continue;
15791579
/*
@@ -3285,7 +3285,7 @@ core_scsi3_emulate_pro_register_and_move(struct se_cmd *cmd, u64 res_key,
32853285
goto out;
32863286
}
32873287
tid_found = target_parse_pr_out_transport_id(dest_se_tpg,
3288-
&buf[24], &tmp_tid_len, &iport_ptr, initiator_str);
3288+
&buf[24], tid_len, &tmp_tid_len, &iport_ptr, initiator_str);
32893289
if (!tid_found) {
32903290
pr_err("SPC-3 PR REGISTER_AND_MOVE: Unable to locate"
32913291
" initiator_str from Transport ID\n");

0 commit comments

Comments
 (0)