The problem is that iscsi_login_zero_tsih_s1 sets conn->sess early in
iscsi_login_set_conn_values. If the function fails later like when we
alloc the idr it does kfree(sess) it leaves the conn->sess pointer set.
iscsi_login_zero_tsih_s1 then returns -Exyz and we then call
iscsi_target_login_sess_out and access the freed memory.

This patch has iscsi_login_zero_tsih_s1 either completely setup the
session or completely tear it down, so later in
iscsi_target_login_sess_out we can just check for it being set to the
connection.

Fixes: 0957627a9960 ("iscsi-target: Fix sess allocation leak in...")
Signed-off-by: Mike Christie <mchri...@redhat.com>
Cc: Matthew Wilcox <wi...@infradead.org>

---
 drivers/target/iscsi/iscsi_target_login.c | 35 ++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_login.c 
b/drivers/target/iscsi/iscsi_target_login.c
index 923b1a9..2d8d353 100644
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -348,8 +348,7 @@ static int iscsi_login_zero_tsih_s1(
                pr_err("idr_alloc() for sess_idr failed\n");
                iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,
                                ISCSI_LOGIN_STATUS_NO_RESOURCES);
-               kfree(sess);
-               return -ENOMEM;
+               goto free_sess;
        }
 
        sess->creation_time = get_jiffies_64();
@@ -365,20 +364,28 @@ static int iscsi_login_zero_tsih_s1(
                                ISCSI_LOGIN_STATUS_NO_RESOURCES);
                pr_err("Unable to allocate memory for"
                                " struct iscsi_sess_ops.\n");
-               kfree(sess);
-               return -ENOMEM;
+               goto remove_idr;
        }
 
        sess->se_sess = transport_alloc_session(TARGET_PROT_NORMAL);
        if (IS_ERR(sess->se_sess)) {
                iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,
                                ISCSI_LOGIN_STATUS_NO_RESOURCES);
-               kfree(sess->sess_ops);
-               kfree(sess);
-               return -ENOMEM;
+               goto free_ops;
        }
 
        return 0;
+
+free_ops:
+       kfree(sess->sess_ops);
+remove_idr:
+       spin_lock_bh(&sess_idr_lock);
+       idr_remove(&sess_idr, sess->session_index);
+       spin_unlock_bh(&sess_idr_lock);
+free_sess:
+       kfree(sess);
+       conn->sess = NULL;
+       return -ENOMEM;
 }
 
 static int iscsi_login_zero_tsih_s2(
@@ -1161,13 +1168,13 @@ void iscsi_target_login_sess_out(struct iscsi_conn 
*conn,
                                   ISCSI_LOGIN_STATUS_INIT_ERR);
        if (!zero_tsih || !conn->sess)
                goto old_sess_out;
-       if (conn->sess->se_sess)
-               transport_free_session(conn->sess->se_sess);
-       if (conn->sess->session_index != 0) {
-               spin_lock_bh(&sess_idr_lock);
-               idr_remove(&sess_idr, conn->sess->session_index);
-               spin_unlock_bh(&sess_idr_lock);
-       }
+
+       transport_free_session(conn->sess->se_sess);
+
+       spin_lock_bh(&sess_idr_lock);
+       idr_remove(&sess_idr, conn->sess->session_index);
+       spin_unlock_bh(&sess_idr_lock);
+
        kfree(conn->sess->sess_ops);
        kfree(conn->sess);
        conn->sess = NULL;
-- 
1.8.3.1

Reply via email to