Skip to content

Commit 05a86e7

Browse files
Mike Christiemartinkpetersen
Mike Christie
authored and
martinkpetersen
committed
scsi: iscsi: target: Fix conn_ops double free
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>
1 parent 7915919 commit 05a86e7

File tree

3 files changed

+77
-75
lines changed

3 files changed

+77
-75
lines changed

‎drivers/target/iscsi/iscsi_target.c

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4208,22 +4208,15 @@ int iscsit_close_connection(
42084208
crypto_free_ahash(tfm);
42094209
}
42104210

4211-
free_cpumask_var(conn->conn_cpumask);
4212-
4213-
kfree(conn->conn_ops);
4214-
conn->conn_ops = NULL;
4215-
42164211
if (conn->sock)
42174212
sock_release(conn->sock);
42184213

42194214
if (conn->conn_transport->iscsit_free_conn)
42204215
conn->conn_transport->iscsit_free_conn(conn);
42214216

4222-
iscsit_put_transport(conn->conn_transport);
4223-
42244217
pr_debug("Moving to TARG_CONN_STATE_FREE.\n");
42254218
conn->conn_state = TARG_CONN_STATE_FREE;
4226-
kfree(conn);
4219+
iscsit_free_conn(conn);
42274220

42284221
spin_lock_bh(&sess->conn_lock);
42294222
atomic_dec(&sess->nconn);

‎drivers/target/iscsi/iscsi_target_login.c

Lines changed: 75 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -67,45 +67,10 @@ static struct iscsi_login *iscsi_login_init_conn(struct iscsi_conn *conn)
6767
goto out_req_buf;
6868
}
6969

70-
conn->conn_ops = kzalloc(sizeof(struct iscsi_conn_ops), GFP_KERNEL);
71-
if (!conn->conn_ops) {
72-
pr_err("Unable to allocate memory for"
73-
" struct iscsi_conn_ops.\n");
74-
goto out_rsp_buf;
75-
}
76-
77-
init_waitqueue_head(&conn->queues_wq);
78-
INIT_LIST_HEAD(&conn->conn_list);
79-
INIT_LIST_HEAD(&conn->conn_cmd_list);
80-
INIT_LIST_HEAD(&conn->immed_queue_list);
81-
INIT_LIST_HEAD(&conn->response_queue_list);
82-
init_completion(&conn->conn_post_wait_comp);
83-
init_completion(&conn->conn_wait_comp);
84-
init_completion(&conn->conn_wait_rcfr_comp);
85-
init_completion(&conn->conn_waiting_on_uc_comp);
86-
init_completion(&conn->conn_logout_comp);
87-
init_completion(&conn->rx_half_close_comp);
88-
init_completion(&conn->tx_half_close_comp);
89-
init_completion(&conn->rx_login_comp);
90-
spin_lock_init(&conn->cmd_lock);
91-
spin_lock_init(&conn->conn_usage_lock);
92-
spin_lock_init(&conn->immed_queue_lock);
93-
spin_lock_init(&conn->nopin_timer_lock);
94-
spin_lock_init(&conn->response_queue_lock);
95-
spin_lock_init(&conn->state_lock);
96-
97-
if (!zalloc_cpumask_var(&conn->conn_cpumask, GFP_KERNEL)) {
98-
pr_err("Unable to allocate conn->conn_cpumask\n");
99-
goto out_conn_ops;
100-
}
10170
conn->conn_login = login;
10271

10372
return login;
10473

105-
out_conn_ops:
106-
kfree(conn->conn_ops);
107-
out_rsp_buf:
108-
kfree(login->rsp_buf);
10974
out_req_buf:
11075
kfree(login->req_buf);
11176
out_login:
@@ -1147,6 +1112,75 @@ iscsit_conn_set_transport(struct iscsi_conn *conn, struct iscsit_transport *t)
11471112
return 0;
11481113
}
11491114

1115+
static struct iscsi_conn *iscsit_alloc_conn(struct iscsi_np *np)
1116+
{
1117+
struct iscsi_conn *conn;
1118+
1119+
conn = kzalloc(sizeof(struct iscsi_conn), GFP_KERNEL);
1120+
if (!conn) {
1121+
pr_err("Could not allocate memory for new connection\n");
1122+
return NULL;
1123+
}
1124+
pr_debug("Moving to TARG_CONN_STATE_FREE.\n");
1125+
conn->conn_state = TARG_CONN_STATE_FREE;
1126+
1127+
init_waitqueue_head(&conn->queues_wq);
1128+
INIT_LIST_HEAD(&conn->conn_list);
1129+
INIT_LIST_HEAD(&conn->conn_cmd_list);
1130+
INIT_LIST_HEAD(&conn->immed_queue_list);
1131+
INIT_LIST_HEAD(&conn->response_queue_list);
1132+
init_completion(&conn->conn_post_wait_comp);
1133+
init_completion(&conn->conn_wait_comp);
1134+
init_completion(&conn->conn_wait_rcfr_comp);
1135+
init_completion(&conn->conn_waiting_on_uc_comp);
1136+
init_completion(&conn->conn_logout_comp);
1137+
init_completion(&conn->rx_half_close_comp);
1138+
init_completion(&conn->tx_half_close_comp);
1139+
init_completion(&conn->rx_login_comp);
1140+
spin_lock_init(&conn->cmd_lock);
1141+
spin_lock_init(&conn->conn_usage_lock);
1142+
spin_lock_init(&conn->immed_queue_lock);
1143+
spin_lock_init(&conn->nopin_timer_lock);
1144+
spin_lock_init(&conn->response_queue_lock);
1145+
spin_lock_init(&conn->state_lock);
1146+
1147+
timer_setup(&conn->nopin_response_timer,
1148+
iscsit_handle_nopin_response_timeout, 0);
1149+
timer_setup(&conn->nopin_timer, iscsit_handle_nopin_timeout, 0);
1150+
1151+
if (iscsit_conn_set_transport(conn, np->np_transport) < 0)
1152+
goto free_conn;
1153+
1154+
conn->conn_ops = kzalloc(sizeof(struct iscsi_conn_ops), GFP_KERNEL);
1155+
if (!conn->conn_ops) {
1156+
pr_err("Unable to allocate memory for struct iscsi_conn_ops.\n");
1157+
goto put_transport;
1158+
}
1159+
1160+
if (!zalloc_cpumask_var(&conn->conn_cpumask, GFP_KERNEL)) {
1161+
pr_err("Unable to allocate conn->conn_cpumask\n");
1162+
goto free_mask;
1163+
}
1164+
1165+
return conn;
1166+
1167+
free_mask:
1168+
free_cpumask_var(conn->conn_cpumask);
1169+
put_transport:
1170+
iscsit_put_transport(conn->conn_transport);
1171+
free_conn:
1172+
kfree(conn);
1173+
return NULL;
1174+
}
1175+
1176+
void iscsit_free_conn(struct iscsi_conn *conn)
1177+
{
1178+
free_cpumask_var(conn->conn_cpumask);
1179+
kfree(conn->conn_ops);
1180+
iscsit_put_transport(conn->conn_transport);
1181+
kfree(conn);
1182+
}
1183+
11501184
void iscsi_target_login_sess_out(struct iscsi_conn *conn,
11511185
struct iscsi_np *np, bool zero_tsih, bool new_sess)
11521186
{
@@ -1196,10 +1230,6 @@ void iscsi_target_login_sess_out(struct iscsi_conn *conn,
11961230
crypto_free_ahash(tfm);
11971231
}
11981232

1199-
free_cpumask_var(conn->conn_cpumask);
1200-
1201-
kfree(conn->conn_ops);
1202-
12031233
if (conn->param_list) {
12041234
iscsi_release_param_list(conn->param_list);
12051235
conn->param_list = NULL;
@@ -1217,8 +1247,7 @@ void iscsi_target_login_sess_out(struct iscsi_conn *conn,
12171247
if (conn->conn_transport->iscsit_free_conn)
12181248
conn->conn_transport->iscsit_free_conn(conn);
12191249

1220-
iscsit_put_transport(conn->conn_transport);
1221-
kfree(conn);
1250+
iscsit_free_conn(conn);
12221251
}
12231252

12241253
static int __iscsi_target_login_thread(struct iscsi_np *np)
@@ -1248,49 +1277,30 @@ static int __iscsi_target_login_thread(struct iscsi_np *np)
12481277
}
12491278
spin_unlock_bh(&np->np_thread_lock);
12501279

1251-
conn = kzalloc(sizeof(struct iscsi_conn), GFP_KERNEL);
1280+
conn = iscsit_alloc_conn(np);
12521281
if (!conn) {
1253-
pr_err("Could not allocate memory for"
1254-
" new connection\n");
12551282
/* Get another socket */
12561283
return 1;
12571284
}
1258-
pr_debug("Moving to TARG_CONN_STATE_FREE.\n");
1259-
conn->conn_state = TARG_CONN_STATE_FREE;
1260-
1261-
timer_setup(&conn->nopin_response_timer,
1262-
iscsit_handle_nopin_response_timeout, 0);
1263-
timer_setup(&conn->nopin_timer, iscsit_handle_nopin_timeout, 0);
1264-
1265-
if (iscsit_conn_set_transport(conn, np->np_transport) < 0) {
1266-
kfree(conn);
1267-
return 1;
1268-
}
12691285

12701286
rc = np->np_transport->iscsit_accept_np(np, conn);
12711287
if (rc == -ENOSYS) {
12721288
complete(&np->np_restart_comp);
1273-
iscsit_put_transport(conn->conn_transport);
1274-
kfree(conn);
1275-
conn = NULL;
1289+
iscsit_free_conn(conn);
12761290
goto exit;
12771291
} else if (rc < 0) {
12781292
spin_lock_bh(&np->np_thread_lock);
12791293
if (atomic_dec_if_positive(&np->np_reset_count) >= 0) {
12801294
np->np_thread_state = ISCSI_NP_THREAD_ACTIVE;
12811295
spin_unlock_bh(&np->np_thread_lock);
12821296
complete(&np->np_restart_comp);
1283-
iscsit_put_transport(conn->conn_transport);
1284-
kfree(conn);
1285-
conn = NULL;
1297+
iscsit_free_conn(conn);
12861298
/* Get another socket */
12871299
return 1;
12881300
}
12891301
spin_unlock_bh(&np->np_thread_lock);
1290-
iscsit_put_transport(conn->conn_transport);
1291-
kfree(conn);
1292-
conn = NULL;
1293-
goto out;
1302+
iscsit_free_conn(conn);
1303+
return 1;
12941304
}
12951305
/*
12961306
* Perform the remaining iSCSI connection initialization items..
@@ -1440,7 +1450,6 @@ static int __iscsi_target_login_thread(struct iscsi_np *np)
14401450
tpg_np = NULL;
14411451
}
14421452

1443-
out:
14441453
return 1;
14451454

14461455
exit:

‎drivers/target/iscsi/iscsi_target_login.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ extern int iscsi_target_setup_login_socket(struct iscsi_np *,
1919
extern int iscsit_accept_np(struct iscsi_np *, struct iscsi_conn *);
2020
extern int iscsit_get_login_rx(struct iscsi_conn *, struct iscsi_login *);
2121
extern int iscsit_put_login_tx(struct iscsi_conn *, struct iscsi_login *, u32);
22-
extern void iscsit_free_conn(struct iscsi_np *, struct iscsi_conn *);
22+
extern void iscsit_free_conn(struct iscsi_conn *);
2323
extern int iscsit_start_kthreads(struct iscsi_conn *);
2424
extern void iscsi_post_login_handler(struct iscsi_np *, struct iscsi_conn *, u8);
2525
extern void iscsi_target_login_sess_out(struct iscsi_conn *, struct iscsi_np *,

0 commit comments

Comments
 (0)