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: e48354ce078c ("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>

diff --git a/drivers/target/iscsi/iscsi_target_nego.c b/drivers/target/iscsi/iscsi_target_nego.c
index 832588f21f9156181385f65a3b212675217cd75b..b03ed154ca34e9854a44debb3004ae3d78bc152c 100644 (file)
--- a/drivers/target/iscsi/iscsi_target_nego.c
+++ b/drivers/target/iscsi/iscsi_target_nego.c
@@ -899,10 +899,14 @@ static int iscsi_target_handle_csg_zero(
                        SENDER_TARGET,
                        login->rsp_buf,
                        &login->rsp_length,
+                       MAX_KEY_VALUE_PAIRS,
                        conn->param_list,
                        conn->tpg->tpg_attrib.login_keys_workaround);
-       if (ret < 0)
+       if (ret < 0) {
+               iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_INITIATOR_ERR,
+                               ISCSI_LOGIN_STATUS_INIT_ERR);
                return -1;
+       }
 
        if (!iscsi_check_negotiated_keys(conn->param_list)) {
                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
                        SENDER_TARGET,
                        login->rsp_buf,
                        &login->rsp_length,
+                       MAX_KEY_VALUE_PAIRS,
                        conn->param_list,
                        conn->tpg->tpg_attrib.login_keys_workaround);
        if (ret < 0) {

diff --git a/drivers/target/iscsi/iscsi_target_parameters.c b/drivers/target/iscsi/iscsi_target_parameters.c
index 4ed578c7b98d55b3e1bfa062c64172beb48008d6..2b318b13268e19bbc8a9b156d0ce96f3c56c7a16 100644 (file)
--- a/drivers/target/iscsi/iscsi_target_parameters.c
+++ b/drivers/target/iscsi/iscsi_target_parameters.c
@@ -1371,19 +1371,42 @@ free_buffer:
        return -1;
 }
 
+/*
+ * Append "key=value" plus a trailing NUL into @textbuf at *@length.
+ * Returns 0 on success and advances *@length, or -EMSGSIZE if the
+ * record (including the NUL) would not fit in the remaining buffer.
+ */
+static int iscsi_encode_text_record(char *textbuf, u32 *length,
+                                   u32 textbuf_size,
+                                   const char *key, const char *value)
+{
+       int n;
+       u32 avail;
+
+       if (*length >= textbuf_size)
+               return -EMSGSIZE;
+
+       avail = textbuf_size - *length;
+       n = snprintf(textbuf + *length, avail, "%s=%s", key, value);
+       if (n < 0 || (u32)n + 1 > avail)
+               return -EMSGSIZE;
+
+       *length += n + 1;
+       return 0;
+}
+
 int iscsi_encode_text_output(
        u8 phase,
        u8 sender,
        char *textbuf,
        u32 *length,
+       u32 textbuf_size,
        struct iscsi_param_list *param_list,
        bool keys_workaround)
 {
-       char *output_buf = NULL;
        struct iscsi_extra_response *er;
        struct iscsi_param *param;
-
-       output_buf = textbuf + *length;
+       int ret;
 
        if (iscsi_enforce_integrity_rules(phase, param_list) < 0)
                return -1;
@@ -1395,10 +1418,12 @@ int iscsi_encode_text_output(
                    !IS_PSTATE_RESPONSE_SENT(param) &&
                    !IS_PSTATE_REPLY_OPTIONAL(param) &&
                    (param->phase & phase)) {
-                       *length += sprintf(output_buf, "%s=%s",
-                               param->name, param->value);
-                       *length += 1;
-                       output_buf = textbuf + *length;
+                       ret = iscsi_encode_text_record(textbuf, length,
+                                                      textbuf_size,
+                                                      param->name,
+                                                      param->value);
+                       if (ret < 0)
+                               goto err_overflow;
                        SET_PSTATE_RESPONSE_SENT(param);
                        pr_debug("Sending key: %s=%s\n",
                                param->name, param->value);
@@ -1408,10 +1433,12 @@ int iscsi_encode_text_output(
                    !IS_PSTATE_ACCEPTOR(param) &&
                    !IS_PSTATE_PROPOSER(param) &&
                    (param->phase & phase)) {
-                       *length += sprintf(output_buf, "%s=%s",
-                               param->name, param->value);
-                       *length += 1;
-                       output_buf = textbuf + *length;
+                       ret = iscsi_encode_text_record(textbuf, length,
+                                                      textbuf_size,
+                                                      param->name,
+                                                      param->value);
+                       if (ret < 0)
+                               goto err_overflow;
                        SET_PSTATE_PROPOSER(param);
                        iscsi_check_proposer_for_optional_reply(param,
                                                                keys_workaround);
@@ -1421,14 +1448,21 @@ int iscsi_encode_text_output(
        }
 
        list_for_each_entry(er, &param_list->extra_response_list, er_list) {
-               *length += sprintf(output_buf, "%s=%s", er->key, er->value);
-               *length += 1;
-               output_buf = textbuf + *length;
+               ret = iscsi_encode_text_record(textbuf, length, textbuf_size,
+                                              er->key, er->value);
+               if (ret < 0)
+                       goto err_overflow;
                pr_debug("Sending key: %s=%s\n", er->key, er->value);
        }
        iscsi_release_extra_responses(param_list);
 
        return 0;
+
+err_overflow:
+       pr_err("iSCSI login response buffer (%u bytes) exhausted, dropping login.\n",
+              textbuf_size);
+       iscsi_release_extra_responses(param_list);
+       return -1;
 }
 
 int iscsi_check_negotiated_keys(struct iscsi_param_list *param_list)

diff --git a/drivers/target/iscsi/iscsi_target_parameters.h b/drivers/target/iscsi/iscsi_target_parameters.h
index c672a971fcb7e1a8170ea2b7c691587aa6761f50..38d2238dfe08ebfc1d83001cc82e4651ae75c7de 100644 (file)
--- a/drivers/target/iscsi/iscsi_target_parameters.h
+++ b/drivers/target/iscsi/iscsi_target_parameters.h
@@ -43,7 +43,7 @@ extern struct iscsi_param *iscsi_find_param_from_key(char *, struct iscsi_param_
 extern int iscsi_extract_key_value(char *, char **, char **);
 extern int iscsi_update_param_value(struct iscsi_param *, char *);
 extern int iscsi_decode_text_input(u8, u8, char *, u32, struct iscsit_conn *);
-extern int iscsi_encode_text_output(u8, u8, char *, u32 *,
+extern int iscsi_encode_text_output(u8, u8, char *, u32 *, u32,
                        struct iscsi_param_list *, bool);
 extern int iscsi_check_negotiated_keys(struct iscsi_param_list *);
 extern void iscsi_set_connection_parameters(struct iscsi_conn_ops *,