diff options
author | Roland Dreier <roland@purestorage.com> | 2014-05-29 13:32:30 -0700 |
---|---|---|
committer | Nicholas Bellinger <nab@linux-iscsi.org> | 2014-06-03 19:17:31 -0700 |
commit | 79d59d08082dd0a0a18f8ceb78c99f9f321d72aa (patch) | |
tree | 94515b2560bdc01a57939ae00b7ca0c9abc965e4 /drivers/target | |
parent | 6cc44a6fb46e1ecc1c28125aa8fa34d317aa9ea7 (diff) | |
download | linux-stable-79d59d08082dd0a0a18f8ceb78c99f9f321d72aa.tar.gz linux-stable-79d59d08082dd0a0a18f8ceb78c99f9f321d72aa.tar.bz2 linux-stable-79d59d08082dd0a0a18f8ceb78c99f9f321d72aa.zip |
iscsi-target: Fix wrong buffer / buffer overrun in iscsi_change_param_value()
In non-leading connection login, iscsi_login_non_zero_tsih_s1() calls
iscsi_change_param_value() with the buffer it uses to hold the login
PDU, not a temporary buffer. This leads to the login header getting
corrupted and login failing for non-leading connections in MC/S.
Fix this by adding a wrapper iscsi_change_param_sprintf() that handles
the temporary buffer itself to avoid confusion. Also handle sending a
reject in case of failure in the wrapper, which lets the calling code
get quite a bit smaller and easier to read.
Finally, bump the size of the temporary buffer from 32 to 64 bytes to be
safe, since "MaxRecvDataSegmentLength=" by itself is 25 bytes; with a
trailing NUL, a value >= 1M will lead to a buffer overrun. (This isn't
the default but we don't need to run right at the ragged edge here)
Reported-by: Santosh Kulkarni <santosh.kulkarni@calsoftinc.com>
Signed-off-by: Roland Dreier <roland@purestorage.com>
Cc: stable@vger.kernel.org # 3.10+
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
Diffstat (limited to 'drivers/target')
-rw-r--r-- | drivers/target/iscsi/iscsi_target_login.c | 70 |
1 files changed, 31 insertions, 39 deletions
diff --git a/drivers/target/iscsi/iscsi_target_login.c b/drivers/target/iscsi/iscsi_target_login.c index ca31fa1b8a4b..d9b1d88e1ad3 100644 --- a/drivers/target/iscsi/iscsi_target_login.c +++ b/drivers/target/iscsi/iscsi_target_login.c @@ -249,6 +249,28 @@ static void iscsi_login_set_conn_values( mutex_unlock(&auth_id_lock); } +static __printf(2, 3) int iscsi_change_param_sprintf( + struct iscsi_conn *conn, + const char *fmt, ...) +{ + va_list args; + unsigned char buf[64]; + + memset(buf, 0, sizeof buf); + + va_start(args, fmt); + vsnprintf(buf, sizeof buf, fmt, args); + va_end(args); + + if (iscsi_change_param_value(buf, conn->param_list, 0) < 0) { + iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR, + ISCSI_LOGIN_STATUS_NO_RESOURCES); + return -1; + } + + return 0; +} + /* * This is the leading connection of a new session, * or session reinstatement. @@ -339,7 +361,6 @@ static int iscsi_login_zero_tsih_s2( { struct iscsi_node_attrib *na; struct iscsi_session *sess = conn->sess; - unsigned char buf[32]; bool iser = false; sess->tpg = conn->tpg; @@ -380,26 +401,16 @@ static int iscsi_login_zero_tsih_s2( * * In our case, we have already located the struct iscsi_tiqn at this point. */ - memset(buf, 0, 32); - sprintf(buf, "TargetPortalGroupTag=%hu", sess->tpg->tpgt); - if (iscsi_change_param_value(buf, conn->param_list, 0) < 0) { - iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR, - ISCSI_LOGIN_STATUS_NO_RESOURCES); + if (iscsi_change_param_sprintf(conn, "TargetPortalGroupTag=%hu", sess->tpg->tpgt)) return -1; - } /* * Workaround for Initiators that have broken connection recovery logic. * * "We would really like to get rid of this." Linux-iSCSI.org team */ - memset(buf, 0, 32); - sprintf(buf, "ErrorRecoveryLevel=%d", na->default_erl); - if (iscsi_change_param_value(buf, conn->param_list, 0) < 0) { - iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR, - ISCSI_LOGIN_STATUS_NO_RESOURCES); + if (iscsi_change_param_sprintf(conn, "ErrorRecoveryLevel=%d", na->default_erl)) return -1; - } if (iscsi_login_disable_FIM_keys(conn->param_list, conn) < 0) return -1; @@ -411,12 +422,9 @@ static int iscsi_login_zero_tsih_s2( unsigned long mrdsl, off; int rc; - sprintf(buf, "RDMAExtensions=Yes"); - if (iscsi_change_param_value(buf, conn->param_list, 0) < 0) { - iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR, - ISCSI_LOGIN_STATUS_NO_RESOURCES); + if (iscsi_change_param_sprintf(conn, "RDMAExtensions=Yes")) return -1; - } + /* * Make MaxRecvDataSegmentLength PAGE_SIZE aligned for * Immediate Data + Unsolicitied Data-OUT if necessary.. @@ -446,12 +454,8 @@ static int iscsi_login_zero_tsih_s2( pr_warn("Aligning ISER MaxRecvDataSegmentLength: %lu down" " to PAGE_SIZE\n", mrdsl); - sprintf(buf, "MaxRecvDataSegmentLength=%lu\n", mrdsl); - if (iscsi_change_param_value(buf, conn->param_list, 0) < 0) { - iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR, - ISCSI_LOGIN_STATUS_NO_RESOURCES); + if (iscsi_change_param_sprintf(conn, "MaxRecvDataSegmentLength=%lu\n", mrdsl)) return -1; - } /* * ISER currently requires that ImmediateData + Unsolicited * Data be disabled when protection / signature MRs are enabled. @@ -461,19 +465,12 @@ check_prot: (TARGET_PROT_DOUT_STRIP | TARGET_PROT_DOUT_PASS | TARGET_PROT_DOUT_INSERT)) { - sprintf(buf, "ImmediateData=No"); - if (iscsi_change_param_value(buf, conn->param_list, 0) < 0) { - iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR, - ISCSI_LOGIN_STATUS_NO_RESOURCES); + if (iscsi_change_param_sprintf(conn, "ImmediateData=No")) return -1; - } - sprintf(buf, "InitialR2T=Yes"); - if (iscsi_change_param_value(buf, conn->param_list, 0) < 0) { - iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR, - ISCSI_LOGIN_STATUS_NO_RESOURCES); + if (iscsi_change_param_sprintf(conn, "InitialR2T=Yes")) return -1; - } + pr_debug("Forcing ImmediateData=No + InitialR2T=Yes for" " T10-PI enabled ISER session\n"); } @@ -618,13 +615,8 @@ static int iscsi_login_non_zero_tsih_s2( * * In our case, we have already located the struct iscsi_tiqn at this point. */ - memset(buf, 0, 32); - sprintf(buf, "TargetPortalGroupTag=%hu", sess->tpg->tpgt); - if (iscsi_change_param_value(buf, conn->param_list, 0) < 0) { - iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR, - ISCSI_LOGIN_STATUS_NO_RESOURCES); + if (iscsi_change_param_sprintf(conn, "TargetPortalGroupTag=%hu", sess->tpg->tpgt)) return -1; - } return iscsi_login_disable_FIM_keys(conn->param_list, conn); } |