Skip to content

Commit bf33e01

Browse files
mjbommarmartinkpetersen
authored andcommitted
scsi: target: iscsi: Bound iscsi_encode_text_output() appends to rsp_buf
iscsi_encode_text_output() concatenates "key=value\0" records into login->rsp_buf, an 8192-byte kzalloc(MAX_KEY_VALUE_PAIRS) buffer allocated in iscsit_alloc_login_setup_buffer(). The three sprintf() call sites in this function (lines 1398, 1411, 1424 in v7.1-rc2) never check the remaining buffer capacity: *length += sprintf(output_buf, "%s=%s", er->key, er->value); *length += 1; output_buf = textbuf + *length; The 8192-byte ceiling at iscsi_target_check_login_request() bounds the *input* Login PDU payload, but a single PDU can carry up to 2048 minimal four-byte "a=b\0" pairs, each unknown key expanding to a 16-byte "a=NotUnderstood\0" output record via iscsi_add_notunderstood_response(). 2048 * 16 = 32 KiB of output into an 8 KiB buffer, producing a ~24 KiB heap overrun in the kmalloc-8k slab. The fix introduces a static iscsi_encode_text_record() helper that uses snprintf() with a per-call bounds check against the remaining buffer, and threads a u32 textbuf_size parameter through iscsi_encode_text_output(). Both call sites in iscsi_target_handle_csg_zero() (PHASE_SECURITY) and iscsi_target_handle_csg_one() (PHASE_OPERATIONAL) pass MAX_KEY_VALUE_PAIRS. On overflow the encoder logs the condition, calls iscsi_release_extra_responses() to drop queued records, and returns -1; both caller sites now emit ISCSI_STATUS_CLS_INITIATOR_ERR / ISCSI_LOGIN_STATUS_INIT_ERR via iscsit_tx_login_rsp() before returning, so the initiator sees an explicit failed-login response rather than a silent connection drop. (Prior to this patch only the PHASE_OPERATIONAL caller did that; the PHASE_SECURITY caller is converted to the same shape.) Fixes: e48354c ("iscsi-target: Add iSCSI fabric support for target v4.1") Cc: stable@vger.kernel.org Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com> Tested-by: John Garry <john.g.garry@oracle.com> Reviewed-by: John Garry <john.g.garry@oracle.com> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
1 parent 778c2ab commit bf33e01

3 files changed

Lines changed: 55 additions & 16 deletions

File tree

drivers/target/iscsi/iscsi_target_nego.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -899,10 +899,14 @@ static int iscsi_target_handle_csg_zero(
899899
SENDER_TARGET,
900900
login->rsp_buf,
901901
&login->rsp_length,
902+
MAX_KEY_VALUE_PAIRS,
902903
conn->param_list,
903904
conn->tpg->tpg_attrib.login_keys_workaround);
904-
if (ret < 0)
905+
if (ret < 0) {
906+
iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_INITIATOR_ERR,
907+
ISCSI_LOGIN_STATUS_INIT_ERR);
905908
return -1;
909+
}
906910

907911
if (!iscsi_check_negotiated_keys(conn->param_list)) {
908912
bool auth_required = iscsi_conn_auth_required(conn);
@@ -986,6 +990,7 @@ static int iscsi_target_handle_csg_one(struct iscsit_conn *conn, struct iscsi_lo
986990
SENDER_TARGET,
987991
login->rsp_buf,
988992
&login->rsp_length,
993+
MAX_KEY_VALUE_PAIRS,
989994
conn->param_list,
990995
conn->tpg->tpg_attrib.login_keys_workaround);
991996
if (ret < 0) {

drivers/target/iscsi/iscsi_target_parameters.c

Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1371,19 +1371,42 @@ int iscsi_decode_text_input(
13711371
return -1;
13721372
}
13731373

1374+
/*
1375+
* Append "key=value" plus a trailing NUL into @textbuf at *@length.
1376+
* Returns 0 on success and advances *@length, or -EMSGSIZE if the
1377+
* record (including the NUL) would not fit in the remaining buffer.
1378+
*/
1379+
static int iscsi_encode_text_record(char *textbuf, u32 *length,
1380+
u32 textbuf_size,
1381+
const char *key, const char *value)
1382+
{
1383+
int n;
1384+
u32 avail;
1385+
1386+
if (*length >= textbuf_size)
1387+
return -EMSGSIZE;
1388+
1389+
avail = textbuf_size - *length;
1390+
n = snprintf(textbuf + *length, avail, "%s=%s", key, value);
1391+
if (n < 0 || (u32)n + 1 > avail)
1392+
return -EMSGSIZE;
1393+
1394+
*length += n + 1;
1395+
return 0;
1396+
}
1397+
13741398
int iscsi_encode_text_output(
13751399
u8 phase,
13761400
u8 sender,
13771401
char *textbuf,
13781402
u32 *length,
1403+
u32 textbuf_size,
13791404
struct iscsi_param_list *param_list,
13801405
bool keys_workaround)
13811406
{
1382-
char *output_buf = NULL;
13831407
struct iscsi_extra_response *er;
13841408
struct iscsi_param *param;
1385-
1386-
output_buf = textbuf + *length;
1409+
int ret;
13871410

13881411
if (iscsi_enforce_integrity_rules(phase, param_list) < 0)
13891412
return -1;
@@ -1395,10 +1418,12 @@ int iscsi_encode_text_output(
13951418
!IS_PSTATE_RESPONSE_SENT(param) &&
13961419
!IS_PSTATE_REPLY_OPTIONAL(param) &&
13971420
(param->phase & phase)) {
1398-
*length += sprintf(output_buf, "%s=%s",
1399-
param->name, param->value);
1400-
*length += 1;
1401-
output_buf = textbuf + *length;
1421+
ret = iscsi_encode_text_record(textbuf, length,
1422+
textbuf_size,
1423+
param->name,
1424+
param->value);
1425+
if (ret < 0)
1426+
goto err_overflow;
14021427
SET_PSTATE_RESPONSE_SENT(param);
14031428
pr_debug("Sending key: %s=%s\n",
14041429
param->name, param->value);
@@ -1408,10 +1433,12 @@ int iscsi_encode_text_output(
14081433
!IS_PSTATE_ACCEPTOR(param) &&
14091434
!IS_PSTATE_PROPOSER(param) &&
14101435
(param->phase & phase)) {
1411-
*length += sprintf(output_buf, "%s=%s",
1412-
param->name, param->value);
1413-
*length += 1;
1414-
output_buf = textbuf + *length;
1436+
ret = iscsi_encode_text_record(textbuf, length,
1437+
textbuf_size,
1438+
param->name,
1439+
param->value);
1440+
if (ret < 0)
1441+
goto err_overflow;
14151442
SET_PSTATE_PROPOSER(param);
14161443
iscsi_check_proposer_for_optional_reply(param,
14171444
keys_workaround);
@@ -1421,14 +1448,21 @@ int iscsi_encode_text_output(
14211448
}
14221449

14231450
list_for_each_entry(er, &param_list->extra_response_list, er_list) {
1424-
*length += sprintf(output_buf, "%s=%s", er->key, er->value);
1425-
*length += 1;
1426-
output_buf = textbuf + *length;
1451+
ret = iscsi_encode_text_record(textbuf, length, textbuf_size,
1452+
er->key, er->value);
1453+
if (ret < 0)
1454+
goto err_overflow;
14271455
pr_debug("Sending key: %s=%s\n", er->key, er->value);
14281456
}
14291457
iscsi_release_extra_responses(param_list);
14301458

14311459
return 0;
1460+
1461+
err_overflow:
1462+
pr_err("iSCSI login response buffer (%u bytes) exhausted, dropping login.\n",
1463+
textbuf_size);
1464+
iscsi_release_extra_responses(param_list);
1465+
return -1;
14321466
}
14331467

14341468
int iscsi_check_negotiated_keys(struct iscsi_param_list *param_list)

drivers/target/iscsi/iscsi_target_parameters.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ extern struct iscsi_param *iscsi_find_param_from_key(char *, struct iscsi_param_
4343
extern int iscsi_extract_key_value(char *, char **, char **);
4444
extern int iscsi_update_param_value(struct iscsi_param *, char *);
4545
extern int iscsi_decode_text_input(u8, u8, char *, u32, struct iscsit_conn *);
46-
extern int iscsi_encode_text_output(u8, u8, char *, u32 *,
46+
extern int iscsi_encode_text_output(u8, u8, char *, u32 *, u32,
4747
struct iscsi_param_list *, bool);
4848
extern int iscsi_check_negotiated_keys(struct iscsi_param_list *);
4949
extern void iscsi_set_connection_parameters(struct iscsi_conn_ops *,

0 commit comments

Comments
 (0)