lkml.org 
[lkml]   [2014]   [Jun]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 3.10 08/18] iscsi-target: Fix wrong buffer / buffer overrun in iscsi_change_param_value()
    Date
    3.10-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Roland Dreier <roland@purestorage.com>

    commit 79d59d08082dd0a0a18f8ceb78c99f9f321d72aa upstream.

    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)

    (Fix up context changes for v3.10.y - nab)

    Reported-by: Santosh Kulkarni <santosh.kulkarni@calsoftinc.com>
    Signed-off-by: Roland Dreier <roland@purestorage.com>
    Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    drivers/target/iscsi/iscsi_target_login.c | 50 ++++++++++++++++--------------
    1 file changed, 27 insertions(+), 23 deletions(-)

    --- a/drivers/target/iscsi/iscsi_target_login.c
    +++ b/drivers/target/iscsi/iscsi_target_login.c
    @@ -250,6 +250,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", ISCSI_TPG_S(sess)->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;
    - }
    }

    return 0;



    \
     
     \ /
      Last update: 2014-06-13 03:21    [W:4.431 / U:0.228 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site