Skip to content

Commit

Permalink
scsi: iscsi: target: Fix conn_ops double free
Browse files Browse the repository at this point in the history 
If iscsi_login_init_conn fails it can free conn_ops.
__iscsi_target_login_thread will then call iscsi_target_login_sess_out
which will also free it.

This fixes the problem by organizing conn allocation/setup into parts that
are needed through the life of the conn and parts that are only needed for
the login. The free functions then release what was allocated in the alloc
functions.

With this patch we have:

iscsit_alloc_conn/iscsit_free_conn - allocs/frees the conn we need for the
entire life of the conn.

iscsi_login_init_conn/iscsi_target_nego_release - allocs/frees the parts
of the conn that are only needed during login.

Signed-off-by: Mike Christie <mchristi@redhat.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
  • Loading branch information
Mike Christie authored and Martin K. Petersen committed Aug 30, 2018
1 parent 7915919 commit 05a86e7
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 75 deletions.
9 changes: 1 addition & 8 deletions drivers/target/iscsi/iscsi_target.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4208,22 +4208,15 @@ int iscsit_close_connection(
crypto_free_ahash(tfm);
}

free_cpumask_var(conn->conn_cpumask);

kfree(conn->conn_ops);
conn->conn_ops = NULL;

if (conn->sock)
sock_release(conn->sock);

if (conn->conn_transport->iscsit_free_conn)
conn->conn_transport->iscsit_free_conn(conn);

iscsit_put_transport(conn->conn_transport);

pr_debug("Moving to TARG_CONN_STATE_FREE.\n");
conn->conn_state = TARG_CONN_STATE_FREE;
kfree(conn);
iscsit_free_conn(conn);

spin_lock_bh(&sess->conn_lock);
atomic_dec(&sess->nconn);
Expand Down
141 changes: 75 additions & 66 deletions drivers/target/iscsi/iscsi_target_login.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,45 +67,10 @@ static struct iscsi_login *iscsi_login_init_conn(struct iscsi_conn *conn)
goto out_req_buf;
}

conn->conn_ops = kzalloc(sizeof(struct iscsi_conn_ops), GFP_KERNEL);
if (!conn->conn_ops) {
pr_err("Unable to allocate memory for"
" struct iscsi_conn_ops.\n");
goto out_rsp_buf;
}

init_waitqueue_head(&conn->queues_wq);
INIT_LIST_HEAD(&conn->conn_list);
INIT_LIST_HEAD(&conn->conn_cmd_list);
INIT_LIST_HEAD(&conn->immed_queue_list);
INIT_LIST_HEAD(&conn->response_queue_list);
init_completion(&conn->conn_post_wait_comp);
init_completion(&conn->conn_wait_comp);
init_completion(&conn->conn_wait_rcfr_comp);
init_completion(&conn->conn_waiting_on_uc_comp);
init_completion(&conn->conn_logout_comp);
init_completion(&conn->rx_half_close_comp);
init_completion(&conn->tx_half_close_comp);
init_completion(&conn->rx_login_comp);
spin_lock_init(&conn->cmd_lock);
spin_lock_init(&conn->conn_usage_lock);
spin_lock_init(&conn->immed_queue_lock);
spin_lock_init(&conn->nopin_timer_lock);
spin_lock_init(&conn->response_queue_lock);
spin_lock_init(&conn->state_lock);

if (!zalloc_cpumask_var(&conn->conn_cpumask, GFP_KERNEL)) {
pr_err("Unable to allocate conn->conn_cpumask\n");
goto out_conn_ops;
}
conn->conn_login = login;

return login;

out_conn_ops:
kfree(conn->conn_ops);
out_rsp_buf:
kfree(login->rsp_buf);
out_req_buf:
kfree(login->req_buf);
out_login:
Expand Down Expand Up @@ -1147,6 +1112,75 @@ iscsit_conn_set_transport(struct iscsi_conn *conn, struct iscsit_transport *t)
return 0;
}

static struct iscsi_conn *iscsit_alloc_conn(struct iscsi_np *np)
{
struct iscsi_conn *conn;

conn = kzalloc(sizeof(struct iscsi_conn), GFP_KERNEL);
if (!conn) {
pr_err("Could not allocate memory for new connection\n");
return NULL;
}
pr_debug("Moving to TARG_CONN_STATE_FREE.\n");
conn->conn_state = TARG_CONN_STATE_FREE;

init_waitqueue_head(&conn->queues_wq);
INIT_LIST_HEAD(&conn->conn_list);
INIT_LIST_HEAD(&conn->conn_cmd_list);
INIT_LIST_HEAD(&conn->immed_queue_list);
INIT_LIST_HEAD(&conn->response_queue_list);
init_completion(&conn->conn_post_wait_comp);
init_completion(&conn->conn_wait_comp);
init_completion(&conn->conn_wait_rcfr_comp);
init_completion(&conn->conn_waiting_on_uc_comp);
init_completion(&conn->conn_logout_comp);
init_completion(&conn->rx_half_close_comp);
init_completion(&conn->tx_half_close_comp);
init_completion(&conn->rx_login_comp);
spin_lock_init(&conn->cmd_lock);
spin_lock_init(&conn->conn_usage_lock);
spin_lock_init(&conn->immed_queue_lock);
spin_lock_init(&conn->nopin_timer_lock);
spin_lock_init(&conn->response_queue_lock);
spin_lock_init(&conn->state_lock);

timer_setup(&conn->nopin_response_timer,
iscsit_handle_nopin_response_timeout, 0);
timer_setup(&conn->nopin_timer, iscsit_handle_nopin_timeout, 0);

if (iscsit_conn_set_transport(conn, np->np_transport) < 0)
goto free_conn;

conn->conn_ops = kzalloc(sizeof(struct iscsi_conn_ops), GFP_KERNEL);
if (!conn->conn_ops) {
pr_err("Unable to allocate memory for struct iscsi_conn_ops.\n");
goto put_transport;
}

if (!zalloc_cpumask_var(&conn->conn_cpumask, GFP_KERNEL)) {
pr_err("Unable to allocate conn->conn_cpumask\n");
goto free_mask;
}

return conn;

free_mask:
free_cpumask_var(conn->conn_cpumask);
put_transport:
iscsit_put_transport(conn->conn_transport);
free_conn:
kfree(conn);
return NULL;
}

void iscsit_free_conn(struct iscsi_conn *conn)
{
free_cpumask_var(conn->conn_cpumask);
kfree(conn->conn_ops);
iscsit_put_transport(conn->conn_transport);
kfree(conn);
}

void iscsi_target_login_sess_out(struct iscsi_conn *conn,
struct iscsi_np *np, bool zero_tsih, bool new_sess)
{
Expand Down Expand Up @@ -1196,10 +1230,6 @@ void iscsi_target_login_sess_out(struct iscsi_conn *conn,
crypto_free_ahash(tfm);
}

free_cpumask_var(conn->conn_cpumask);

kfree(conn->conn_ops);

if (conn->param_list) {
iscsi_release_param_list(conn->param_list);
conn->param_list = NULL;
Expand All @@ -1217,8 +1247,7 @@ void iscsi_target_login_sess_out(struct iscsi_conn *conn,
if (conn->conn_transport->iscsit_free_conn)
conn->conn_transport->iscsit_free_conn(conn);

iscsit_put_transport(conn->conn_transport);
kfree(conn);
iscsit_free_conn(conn);
}

static int __iscsi_target_login_thread(struct iscsi_np *np)
Expand Down Expand Up @@ -1248,49 +1277,30 @@ static int __iscsi_target_login_thread(struct iscsi_np *np)
}
spin_unlock_bh(&np->np_thread_lock);

conn = kzalloc(sizeof(struct iscsi_conn), GFP_KERNEL);
conn = iscsit_alloc_conn(np);
if (!conn) {
pr_err("Could not allocate memory for"
" new connection\n");
/* Get another socket */
return 1;
}
pr_debug("Moving to TARG_CONN_STATE_FREE.\n");
conn->conn_state = TARG_CONN_STATE_FREE;

timer_setup(&conn->nopin_response_timer,
iscsit_handle_nopin_response_timeout, 0);
timer_setup(&conn->nopin_timer, iscsit_handle_nopin_timeout, 0);

if (iscsit_conn_set_transport(conn, np->np_transport) < 0) {
kfree(conn);
return 1;
}

rc = np->np_transport->iscsit_accept_np(np, conn);
if (rc == -ENOSYS) {
complete(&np->np_restart_comp);
iscsit_put_transport(conn->conn_transport);
kfree(conn);
conn = NULL;
iscsit_free_conn(conn);
goto exit;
} else if (rc < 0) {
spin_lock_bh(&np->np_thread_lock);
if (atomic_dec_if_positive(&np->np_reset_count) >= 0) {
np->np_thread_state = ISCSI_NP_THREAD_ACTIVE;
spin_unlock_bh(&np->np_thread_lock);
complete(&np->np_restart_comp);
iscsit_put_transport(conn->conn_transport);
kfree(conn);
conn = NULL;
iscsit_free_conn(conn);
/* Get another socket */
return 1;
}
spin_unlock_bh(&np->np_thread_lock);
iscsit_put_transport(conn->conn_transport);
kfree(conn);
conn = NULL;
goto out;
iscsit_free_conn(conn);
return 1;
}
/*
* Perform the remaining iSCSI connection initialization items..
Expand Down Expand Up @@ -1440,7 +1450,6 @@ static int __iscsi_target_login_thread(struct iscsi_np *np)
tpg_np = NULL;
}

out:
return 1;

exit:
Expand Down
2 changes: 1 addition & 1 deletion drivers/target/iscsi/iscsi_target_login.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ extern int iscsi_target_setup_login_socket(struct iscsi_np *,
extern int iscsit_accept_np(struct iscsi_np *, struct iscsi_conn *);
extern int iscsit_get_login_rx(struct iscsi_conn *, struct iscsi_login *);
extern int iscsit_put_login_tx(struct iscsi_conn *, struct iscsi_login *, u32);
extern void iscsit_free_conn(struct iscsi_np *, struct iscsi_conn *);
extern void iscsit_free_conn(struct iscsi_conn *);
extern int iscsit_start_kthreads(struct iscsi_conn *);
extern void iscsi_post_login_handler(struct iscsi_np *, struct iscsi_conn *, u8);
extern void iscsi_target_login_sess_out(struct iscsi_conn *, struct iscsi_np *,
Expand Down

0 comments on commit 05a86e7

Please sign in to comment.