From 12d5a43b2dffb6cd28062b4e19024f7982393288 Mon Sep 17 00:00:00 2001 From: tangwenji Date: Fri, 15 Sep 2017 16:03:13 +0800 Subject: iscsi-target: fix memory leak in lio_target_tiqn_addtpg() tpg must free when call core_tpg_register() return fail Signed-off-by: tangwenji Signed-off-by: Nicholas Bellinger --- drivers/target/iscsi/iscsi_target_configfs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/target/iscsi') diff --git a/drivers/target/iscsi/iscsi_target_configfs.c b/drivers/target/iscsi/iscsi_target_configfs.c index 0dd4c45f7575..0ebc4818e132 100644 --- a/drivers/target/iscsi/iscsi_target_configfs.c +++ b/drivers/target/iscsi/iscsi_target_configfs.c @@ -1123,7 +1123,7 @@ static struct se_portal_group *lio_target_tiqn_addtpg( ret = core_tpg_register(wwn, &tpg->tpg_se_tpg, SCSI_PROTOCOL_ISCSI); if (ret < 0) - return NULL; + goto free_out; ret = iscsit_tpg_add_portal_group(tiqn, tpg); if (ret != 0) @@ -1135,6 +1135,7 @@ static struct se_portal_group *lio_target_tiqn_addtpg( return &tpg->tpg_se_tpg; out: core_tpg_deregister(&tpg->tpg_se_tpg); +free_out: kfree(tpg); return NULL; } -- cgit v1.2.3 From a0884d489e016606e5e040624edaa13efefc3fcb Mon Sep 17 00:00:00 2001 From: tangwenji Date: Fri, 15 Sep 2017 17:18:07 +0800 Subject: iscsi-target: fix memory leak in iscsit_release_discovery_tpg() Need to release the param_list for tpg in iscsi_release_discovery_tpg function, this is also required before the iscsit_load_discovery_tpg function exits abnormally. Signed-off-by: tangwenji Signed-off-by: Nicholas Bellinger --- drivers/target/iscsi/iscsi_target_tpg.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'drivers/target/iscsi') diff --git a/drivers/target/iscsi/iscsi_target_tpg.c b/drivers/target/iscsi/iscsi_target_tpg.c index 594d07a1e995..4b34f71547c6 100644 --- a/drivers/target/iscsi/iscsi_target_tpg.c +++ b/drivers/target/iscsi/iscsi_target_tpg.c @@ -90,10 +90,10 @@ int iscsit_load_discovery_tpg(void) */ param = iscsi_find_param_from_key(AUTHMETHOD, tpg->param_list); if (!param) - goto out; + goto free_pl_out; if (iscsi_update_param_value(param, "CHAP,None") < 0) - goto out; + goto free_pl_out; tpg->tpg_attrib.authentication = 0; @@ -105,6 +105,8 @@ int iscsit_load_discovery_tpg(void) pr_debug("CORE[0] - Allocated Discovery TPG\n"); return 0; +free_pl_out: + iscsi_release_param_list(tpg->param_list); out: if (tpg->sid == 1) core_tpg_deregister(&tpg->tpg_se_tpg); @@ -119,6 +121,7 @@ void iscsit_release_discovery_tpg(void) if (!tpg) return; + iscsi_release_param_list(tpg->param_list); core_tpg_deregister(&tpg->tpg_se_tpg); kfree(tpg); -- cgit v1.2.3 From 1ae01724ae92004be36a6c11c4d5a9f94e915204 Mon Sep 17 00:00:00 2001 From: Varun Prakash Date: Wed, 4 Oct 2017 22:03:35 +0530 Subject: cxgbit: Abort the TCP connection in case of data out timeout If DDP is programmed for a WRITE cmd and data out timer gets expired then abort the TCP connection before freeing the cmd to avoid any possibility of DDP after freeing the cmd. Signed-off-by: Varun Prakash Signed-off-by: Nicholas Bellinger --- drivers/target/iscsi/cxgbit/cxgbit.h | 2 ++ drivers/target/iscsi/cxgbit/cxgbit_cm.c | 45 ++++++++++++++++++++++++++++++++ drivers/target/iscsi/cxgbit/cxgbit_ddp.c | 8 ++++++ 3 files changed, 55 insertions(+) (limited to 'drivers/target/iscsi') diff --git a/drivers/target/iscsi/cxgbit/cxgbit.h b/drivers/target/iscsi/cxgbit/cxgbit.h index 90388698c222..417b9e66b0cd 100644 --- a/drivers/target/iscsi/cxgbit/cxgbit.h +++ b/drivers/target/iscsi/cxgbit/cxgbit.h @@ -165,6 +165,7 @@ enum cxgbit_csk_flags { CSK_LOGIN_PDU_DONE, CSK_LOGIN_DONE, CSK_DDP_ENABLE, + CSK_ABORT_RPL_WAIT, }; struct cxgbit_sock_common { @@ -321,6 +322,7 @@ int cxgbit_setup_np(struct iscsi_np *, struct sockaddr_storage *); int cxgbit_setup_conn_digest(struct cxgbit_sock *); int cxgbit_accept_np(struct iscsi_np *, struct iscsi_conn *); void cxgbit_free_np(struct iscsi_np *); +void cxgbit_abort_conn(struct cxgbit_sock *csk); void cxgbit_free_conn(struct iscsi_conn *); extern cxgbit_cplhandler_func cxgbit_cplhandlers[NUM_CPL_CMDS]; int cxgbit_get_login_rx(struct iscsi_conn *, struct iscsi_login *); diff --git a/drivers/target/iscsi/cxgbit/cxgbit_cm.c b/drivers/target/iscsi/cxgbit/cxgbit_cm.c index d4fa41be80f9..92eb57e2adaf 100644 --- a/drivers/target/iscsi/cxgbit/cxgbit_cm.c +++ b/drivers/target/iscsi/cxgbit/cxgbit_cm.c @@ -665,6 +665,46 @@ static int cxgbit_send_abort_req(struct cxgbit_sock *csk) return cxgbit_l2t_send(csk->com.cdev, skb, csk->l2t); } +static void +__cxgbit_abort_conn(struct cxgbit_sock *csk, struct sk_buff *skb) +{ + __kfree_skb(skb); + + if (csk->com.state != CSK_STATE_ESTABLISHED) + goto no_abort; + + set_bit(CSK_ABORT_RPL_WAIT, &csk->com.flags); + csk->com.state = CSK_STATE_ABORTING; + + cxgbit_send_abort_req(csk); + + return; + +no_abort: + cxgbit_wake_up(&csk->com.wr_wait, __func__, CPL_ERR_NONE); + cxgbit_put_csk(csk); +} + +void cxgbit_abort_conn(struct cxgbit_sock *csk) +{ + struct sk_buff *skb = alloc_skb(0, GFP_KERNEL | __GFP_NOFAIL); + + cxgbit_get_csk(csk); + cxgbit_init_wr_wait(&csk->com.wr_wait); + + spin_lock_bh(&csk->lock); + if (csk->lock_owner) { + cxgbit_skcb_rx_backlog_fn(skb) = __cxgbit_abort_conn; + __skb_queue_tail(&csk->backlogq, skb); + } else { + __cxgbit_abort_conn(csk, skb); + } + spin_unlock_bh(&csk->lock); + + cxgbit_wait_for_reply(csk->com.cdev, &csk->com.wr_wait, + csk->tid, 600, __func__); +} + void cxgbit_free_conn(struct iscsi_conn *conn) { struct cxgbit_sock *csk = conn->context; @@ -1709,12 +1749,17 @@ rel_skb: static void cxgbit_abort_rpl_rss(struct cxgbit_sock *csk, struct sk_buff *skb) { + struct cpl_abort_rpl_rss *rpl = cplhdr(skb); + pr_debug("%s: csk %p; tid %u; state %d\n", __func__, csk, csk->tid, csk->com.state); switch (csk->com.state) { case CSK_STATE_ABORTING: csk->com.state = CSK_STATE_DEAD; + if (test_bit(CSK_ABORT_RPL_WAIT, &csk->com.flags)) + cxgbit_wake_up(&csk->com.wr_wait, __func__, + rpl->status); cxgbit_put_csk(csk); break; default: diff --git a/drivers/target/iscsi/cxgbit/cxgbit_ddp.c b/drivers/target/iscsi/cxgbit/cxgbit_ddp.c index 5fdb57cac968..768cce0ccb80 100644 --- a/drivers/target/iscsi/cxgbit/cxgbit_ddp.c +++ b/drivers/target/iscsi/cxgbit/cxgbit_ddp.c @@ -275,6 +275,14 @@ void cxgbit_release_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd) struct cxgbit_device *cdev = csk->com.cdev; struct cxgbi_ppm *ppm = cdev2ppm(cdev); + /* Abort the TCP conn if DDP is not complete to + * avoid any possibility of DDP after freeing + * the cmd. + */ + if (unlikely(cmd->write_data_done != + cmd->se_cmd.data_length)) + cxgbit_abort_conn(csk); + cxgbi_ppm_ppod_release(ppm, ttinfo->idx); dma_unmap_sg(&ppm->pdev->dev, ttinfo->sgl, -- cgit v1.2.3 From d7e595ddd5b243aa9ba1948d5e0a37783b5415af Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 31 Oct 2017 11:03:10 -0700 Subject: target: Suppress gcc 7 fallthrough warnings Avoid that gcc 7 reports the following warning when building with W=1: warning: this statement may fall through [-Wimplicit-fallthrough=] Signed-off-by: Bart Van Assche Cc: Christoph Hellwig Cc: Mike Christie Cc: Varun Prakash Reviewed-by: Hannes Reinecke Signed-off-by: Nicholas Bellinger --- drivers/target/iscsi/cxgbit/cxgbit_main.c | 1 + drivers/target/target_core_pr.c | 2 ++ drivers/target/target_core_transport.c | 4 ++-- 3 files changed, 5 insertions(+), 2 deletions(-) (limited to 'drivers/target/iscsi') diff --git a/drivers/target/iscsi/cxgbit/cxgbit_main.c b/drivers/target/iscsi/cxgbit/cxgbit_main.c index 4fd775ace541..f3f8856bfb68 100644 --- a/drivers/target/iscsi/cxgbit/cxgbit_main.c +++ b/drivers/target/iscsi/cxgbit/cxgbit_main.c @@ -446,6 +446,7 @@ cxgbit_uld_lro_rx_handler(void *hndl, const __be64 *rsp, case CPL_RX_ISCSI_DDP: case CPL_FW4_ACK: lro_flush = false; + /* fall through */ case CPL_ABORT_RPL_RSS: case CPL_PASS_ESTABLISH: case CPL_PEER_CLOSE: diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c index 3d2b46472dfd..b024613f9217 100644 --- a/drivers/target/target_core_pr.c +++ b/drivers/target/target_core_pr.c @@ -353,6 +353,7 @@ static int core_scsi3_pr_seq_non_holder(struct se_cmd *cmd, u32 pr_reg_type, break; case PR_TYPE_WRITE_EXCLUSIVE_REGONLY: we = 1; + /* fall through */ case PR_TYPE_EXCLUSIVE_ACCESS_REGONLY: /* * Some commands are only allowed for registered I_T Nexuses. @@ -361,6 +362,7 @@ static int core_scsi3_pr_seq_non_holder(struct se_cmd *cmd, u32 pr_reg_type, break; case PR_TYPE_WRITE_EXCLUSIVE_ALLREG: we = 1; + /* fall through */ case PR_TYPE_EXCLUSIVE_ACCESS_ALLREG: /* * Each registered I_T Nexus is a reservation holder. diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index e791a7b63382..e2bf547054bd 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -2092,7 +2092,7 @@ static void transport_complete_qf(struct se_cmd *cmd) ret = cmd->se_tfo->queue_data_in(cmd); break; } - /* Fall through for DMA_TO_DEVICE */ + /* fall through */ case DMA_NONE: queue_status: trace_target_cmd_complete(cmd); @@ -2270,7 +2270,7 @@ queue_rsp: goto queue_full; break; } - /* Fall through for DMA_TO_DEVICE */ + /* fall through */ case DMA_NONE: queue_status: trace_target_cmd_complete(cmd); -- cgit v1.2.3 From 8d973ab5d4520f84e89aaa38e0a50bb2876b09eb Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 31 Oct 2017 11:03:13 -0700 Subject: target/iscsi: Define OFFLOAD_BUF_SIZE once The constant OFFLOAD_BUF_SIZE is defined twice - once in iscsi_target_seq_pdu_list.c and once in iscsi_target_erl1.c. Since that constant is not used in the former source file, remove its definition from that source file. Signed-off-by: Bart Van Assche Cc: Christoph Hellwig Cc: Mike Christie Reviewed-by: Hannes Reinecke Signed-off-by: Nicholas Bellinger --- drivers/target/iscsi/iscsi_target_seq_pdu_list.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'drivers/target/iscsi') diff --git a/drivers/target/iscsi/iscsi_target_seq_pdu_list.c b/drivers/target/iscsi/iscsi_target_seq_pdu_list.c index e446a09c886b..f65e5e584212 100644 --- a/drivers/target/iscsi/iscsi_target_seq_pdu_list.c +++ b/drivers/target/iscsi/iscsi_target_seq_pdu_list.c @@ -25,8 +25,6 @@ #include "iscsi_target_tpg.h" #include "iscsi_target_seq_pdu_list.h" -#define OFFLOAD_BUF_SIZE 32768 - #ifdef DEBUG static void iscsit_dump_seq_list(struct iscsi_cmd *cmd) { -- cgit v1.2.3 From 919765e9680fe26acdcad782ee693f31dbde2def Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 31 Oct 2017 11:03:14 -0700 Subject: target/iscsi: Use min() in iscsit_dump_data_payload() instead of open-coding it This patch does not change any functionality. Signed-off-by: Bart Van Assche Cc: Christoph Hellwig Cc: Mike Christie Reviewed-by: Hannes Reinecke Signed-off-by: Nicholas Bellinger --- drivers/target/iscsi/iscsi_target_erl1.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'drivers/target/iscsi') diff --git a/drivers/target/iscsi/iscsi_target_erl1.c b/drivers/target/iscsi/iscsi_target_erl1.c index fe9b7f1e44ac..659efafb43ec 100644 --- a/drivers/target/iscsi/iscsi_target_erl1.c +++ b/drivers/target/iscsi/iscsi_target_erl1.c @@ -34,7 +34,7 @@ #include "iscsi_target_erl2.h" #include "iscsi_target.h" -#define OFFLOAD_BUF_SIZE 32768 +#define OFFLOAD_BUF_SIZE 32768U /* * Used to dump excess datain payload for certain error recovery @@ -56,7 +56,7 @@ int iscsit_dump_data_payload( if (conn->sess->sess_ops->RDMAExtensions) return 0; - length = (buf_len > OFFLOAD_BUF_SIZE) ? OFFLOAD_BUF_SIZE : buf_len; + length = min(buf_len, OFFLOAD_BUF_SIZE); buf = kzalloc(length, GFP_ATOMIC); if (!buf) { @@ -67,8 +67,7 @@ int iscsit_dump_data_payload( memset(&iov, 0, sizeof(struct kvec)); while (offset < buf_len) { - size = ((offset + length) > buf_len) ? - (buf_len - offset) : length; + size = min(buf_len - offset, length); iov.iov_len = size; iov.iov_base = buf; -- cgit v1.2.3 From de3493aea656ad7ae2e8dd492ee928712d147c74 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 31 Oct 2017 11:03:15 -0700 Subject: target/iscsi: Fix endianness in an error message Since hdr->offset is a big endian number, convert it to CPU endian before printing it. Signed-off-by: Bart Van Assche Cc: Christoph Hellwig Cc: Mike Christie Reviewed-by: Hannes Reinecke Signed-off-by: Nicholas Bellinger --- drivers/target/iscsi/iscsi_target.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/target/iscsi') diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index 5001261f5d69..37bc8a37401d 100644 --- a/drivers/target/iscsi/iscsi_target.c +++ b/drivers/target/iscsi/iscsi_target.c @@ -1460,9 +1460,9 @@ __iscsit_check_dataout_hdr(struct iscsi_conn *conn, void *buf, iscsit_mod_dataout_timer(cmd); if ((be32_to_cpu(hdr->offset) + payload_length) > cmd->se_cmd.data_length) { - pr_err("DataOut Offset: %u, Length %u greater than" - " iSCSI Command EDTL %u, protocol error.\n", - hdr->offset, payload_length, cmd->se_cmd.data_length); + pr_err("DataOut Offset: %u, Length %u greater than iSCSI Command EDTL %u, protocol error.\n", + be32_to_cpu(hdr->offset), payload_length, + cmd->se_cmd.data_length); return iscsit_reject_cmd(cmd, ISCSI_REASON_BOOKMARK_INVALID, buf); } -- cgit v1.2.3 From e1dfb21f004f403a16539e8a037963b57a25e0ad Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 31 Oct 2017 11:03:16 -0700 Subject: target/iscsi: Modify iscsit_do_crypto_hash_buf() prototype Change the type of the last two arguments from u8 * into const void * and void * respectively such that the u8 * casts can be left out from the callers. This patch does not change any functionality. Signed-off-by: Bart Van Assche Cc: Christoph Hellwig Cc: Mike Christie Reviewed-by: Hannes Reinecke Signed-off-by: Nicholas Bellinger --- drivers/target/iscsi/iscsi_target.c | 41 +++++++++++++++---------------------- 1 file changed, 17 insertions(+), 24 deletions(-) (limited to 'drivers/target/iscsi') diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index 37bc8a37401d..91fbada7cdc2 100644 --- a/drivers/target/iscsi/iscsi_target.c +++ b/drivers/target/iscsi/iscsi_target.c @@ -500,7 +500,7 @@ void iscsit_aborted_task(struct iscsi_conn *conn, struct iscsi_cmd *cmd) EXPORT_SYMBOL(iscsit_aborted_task); static void iscsit_do_crypto_hash_buf(struct ahash_request *, const void *, - u32, u32, u8 *, u8 *); + u32, u32, const void *, void *); static void iscsit_tx_thread_wait_for_tcp(struct iscsi_conn *); static int @@ -521,7 +521,7 @@ iscsit_xmit_nondatain_pdu(struct iscsi_conn *conn, struct iscsi_cmd *cmd, iscsit_do_crypto_hash_buf(conn->conn_tx_hash, hdr, ISCSI_HDR_LEN, 0, NULL, - (u8 *)header_digest); + header_digest); iov[0].iov_len += ISCSI_CRC_LEN; tx_size += ISCSI_CRC_LEN; @@ -548,9 +548,8 @@ iscsit_xmit_nondatain_pdu(struct iscsi_conn *conn, struct iscsi_cmd *cmd, if (conn->conn_ops->DataDigest) { iscsit_do_crypto_hash_buf(conn->conn_tx_hash, data_buf, data_buf_len, - padding, - (u8 *)&cmd->pad_bytes, - (u8 *)&cmd->data_crc); + padding, &cmd->pad_bytes, + &cmd->data_crc); iov[niov].iov_base = &cmd->data_crc; iov[niov++].iov_len = ISCSI_CRC_LEN; @@ -595,7 +594,7 @@ iscsit_xmit_datain_pdu(struct iscsi_conn *conn, struct iscsi_cmd *cmd, iscsit_do_crypto_hash_buf(conn->conn_tx_hash, cmd->pdu, ISCSI_HDR_LEN, 0, NULL, - (u8 *)header_digest); + header_digest); iov[0].iov_len += ISCSI_CRC_LEN; tx_size += ISCSI_CRC_LEN; @@ -1408,13 +1407,9 @@ static u32 iscsit_do_crypto_hash_sg( return data_crc; } -static void iscsit_do_crypto_hash_buf( - struct ahash_request *hash, - const void *buf, - u32 payload_length, - u32 padding, - u8 *pad_bytes, - u8 *data_crc) +static void iscsit_do_crypto_hash_buf(struct ahash_request *hash, + const void *buf, u32 payload_length, u32 padding, + const void *pad_bytes, void *data_crc) { struct scatterlist sg[2]; @@ -1876,10 +1871,9 @@ static int iscsit_handle_nop_out(struct iscsi_conn *conn, struct iscsi_cmd *cmd, } if (conn->conn_ops->DataDigest) { - iscsit_do_crypto_hash_buf(conn->conn_rx_hash, - ping_data, payload_length, - padding, cmd->pad_bytes, - (u8 *)&data_crc); + iscsit_do_crypto_hash_buf(conn->conn_rx_hash, ping_data, + payload_length, padding, + cmd->pad_bytes, &data_crc); if (checksum != data_crc) { pr_err("Ping data CRC32C DataDigest" @@ -2285,10 +2279,9 @@ iscsit_handle_text_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd, goto reject; if (conn->conn_ops->DataDigest) { - iscsit_do_crypto_hash_buf(conn->conn_rx_hash, - text_in, payload_length, - padding, (u8 *)&pad_bytes, - (u8 *)&data_crc); + iscsit_do_crypto_hash_buf(conn->conn_rx_hash, text_in, + payload_length, padding, + &pad_bytes, &data_crc); if (checksum != data_crc) { pr_err("Text data CRC32C DataDigest" @@ -3976,9 +3969,9 @@ static void iscsit_get_rx_pdu(struct iscsi_conn *conn) return; } - iscsit_do_crypto_hash_buf(conn->conn_rx_hash, - buffer, ISCSI_HDR_LEN, - 0, NULL, (u8 *)&checksum); + iscsit_do_crypto_hash_buf(conn->conn_rx_hash, buffer, + ISCSI_HDR_LEN, 0, NULL, + &checksum); if (digest != checksum) { pr_err("HeaderDigest CRC32C failed," -- cgit v1.2.3 From cfe2b621bb18d86e93271febf8c6e37622da2d14 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 31 Oct 2017 11:03:17 -0700 Subject: target/iscsi: Fix a race condition in iscsit_add_reject_from_cmd() Avoid that cmd->se_cmd.se_tfo is read after a command has already been freed. Signed-off-by: Bart Van Assche Cc: Christoph Hellwig Cc: Mike Christie Reviewed-by: Hannes Reinecke Signed-off-by: Nicholas Bellinger --- drivers/target/iscsi/iscsi_target.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/target/iscsi') diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index 91fbada7cdc2..541f66a875fc 100644 --- a/drivers/target/iscsi/iscsi_target.c +++ b/drivers/target/iscsi/iscsi_target.c @@ -833,6 +833,7 @@ static int iscsit_add_reject_from_cmd( unsigned char *buf) { struct iscsi_conn *conn; + const bool do_put = cmd->se_cmd.se_tfo != NULL; if (!cmd->conn) { pr_err("cmd->conn is NULL for ITT: 0x%08x\n", @@ -863,7 +864,7 @@ static int iscsit_add_reject_from_cmd( * Perform the kref_put now if se_cmd has already been setup by * scsit_setup_scsi_cmd() */ - if (cmd->se_cmd.se_tfo != NULL) { + if (do_put) { pr_debug("iscsi reject: calling target_put_sess_cmd >>>>>>\n"); target_put_sess_cmd(&cmd->se_cmd); } -- cgit v1.2.3 From 6eaf69e4ec075f5af236c0c89f75639a195db904 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 31 Oct 2017 11:03:18 -0700 Subject: target/iscsi: Detect conn_cmd_list corruption early Certain behavior of the initiator can cause the target driver to send both a reject and a SCSI response. If that happens two target_put_sess_cmd() calls will occur without the command having been removed from conn_cmd_list. In other words, conn_cmd_list will get corrupted once the freed memory is reused. Although the Linux kernel can detect list corruption if list debugging is enabled, in this case the context in which list corruption is detected is not related to the context that caused list corruption. Hence add WARN_ON() statements that report the context that is causing list corruption. Signed-off-by: Bart Van Assche Cc: Christoph Hellwig Cc: Mike Christie Reviewed-by: Hannes Reinecke Signed-off-by: Nicholas Bellinger --- drivers/target/iscsi/iscsi_target_util.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers/target/iscsi') diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c index 1e36f83b5961..70c6b9bfc04e 100644 --- a/drivers/target/iscsi/iscsi_target_util.c +++ b/drivers/target/iscsi/iscsi_target_util.c @@ -694,6 +694,8 @@ void iscsit_release_cmd(struct iscsi_cmd *cmd) struct iscsi_session *sess; struct se_cmd *se_cmd = &cmd->se_cmd; + WARN_ON(!list_empty(&cmd->i_conn_node)); + if (cmd->conn) sess = cmd->conn->sess; else @@ -716,6 +718,8 @@ void __iscsit_free_cmd(struct iscsi_cmd *cmd, bool check_queues) { struct iscsi_conn *conn = cmd->conn; + WARN_ON(!list_empty(&cmd->i_conn_node)); + if (cmd->data_direction == DMA_TO_DEVICE) { iscsit_stop_dataout_timer(cmd); iscsit_free_r2ts_from_list(cmd); -- cgit v1.2.3 From 17c45b90061a76fceadffbce8d85a9107a05a918 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Fri, 3 Nov 2017 22:20:38 +0100 Subject: iSCSI-target: Use common error handling code in iscsi_decode_text_input() Add a jump target so that a bit of exception handling can be better reused at the end of this function. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring Signed-off-by: Nicholas Bellinger --- drivers/target/iscsi/iscsi_target_parameters.c | 39 ++++++++++++-------------- 1 file changed, 18 insertions(+), 21 deletions(-) (limited to 'drivers/target/iscsi') diff --git a/drivers/target/iscsi/iscsi_target_parameters.c b/drivers/target/iscsi/iscsi_target_parameters.c index caab1045742d..29a37b242d30 100644 --- a/drivers/target/iscsi/iscsi_target_parameters.c +++ b/drivers/target/iscsi/iscsi_target_parameters.c @@ -1380,10 +1380,8 @@ int iscsi_decode_text_input( char *key, *value; struct iscsi_param *param; - if (iscsi_extract_key_value(start, &key, &value) < 0) { - kfree(tmpbuf); - return -1; - } + if (iscsi_extract_key_value(start, &key, &value) < 0) + goto free_buffer; pr_debug("Got key: %s=%s\n", key, value); @@ -1396,38 +1394,37 @@ int iscsi_decode_text_input( param = iscsi_check_key(key, phase, sender, param_list); if (!param) { - if (iscsi_add_notunderstood_response(key, - value, param_list) < 0) { - kfree(tmpbuf); - return -1; - } + if (iscsi_add_notunderstood_response(key, value, + param_list) < 0) + goto free_buffer; + start += strlen(key) + strlen(value) + 2; continue; } - if (iscsi_check_value(param, value) < 0) { - kfree(tmpbuf); - return -1; - } + if (iscsi_check_value(param, value) < 0) + goto free_buffer; start += strlen(key) + strlen(value) + 2; if (IS_PSTATE_PROPOSER(param)) { - if (iscsi_check_proposer_state(param, value) < 0) { - kfree(tmpbuf); - return -1; - } + if (iscsi_check_proposer_state(param, value) < 0) + goto free_buffer; + SET_PSTATE_RESPONSE_GOT(param); } else { - if (iscsi_check_acceptor_state(param, value, conn) < 0) { - kfree(tmpbuf); - return -1; - } + if (iscsi_check_acceptor_state(param, value, conn) < 0) + goto free_buffer; + SET_PSTATE_ACCEPTOR(param); } } kfree(tmpbuf); return 0; + +free_buffer: + kfree(tmpbuf); + return -1; } int iscsi_encode_text_output( -- cgit v1.2.3 From ae072726f6109bb1c94841d6fb3a82dde298ea85 Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Fri, 27 Oct 2017 12:32:59 -0700 Subject: iscsi-target: Make TASK_REASSIGN use proper se_cmd->cmd_kref Since commit 59b6986dbf fixed a potential NULL pointer dereference by allocating a se_tmr_req for ISCSI_TM_FUNC_TASK_REASSIGN, the se_tmr_req is currently leaked by iscsit_free_cmd() because no iscsi_cmd->se_cmd.se_tfo was associated. To address this, treat ISCSI_TM_FUNC_TASK_REASSIGN like any other TMR and call transport_init_se_cmd() + target_get_sess_cmd() to setup iscsi_cmd->se_cmd.se_tfo with se_cmd->cmd_kref of 2. This will ensure normal release operation once se_cmd->cmd_kref reaches zero and target_release_cmd_kref() is invoked, se_tmr_req will be released via existing target_free_cmd_mem() and core_tmr_release_req() code. Reported-by: Donald White Cc: Donald White Cc: Mike Christie Cc: Hannes Reinecke Cc: stable@vger.kernel.org # 3.10+ Signed-off-by: Nicholas Bellinger --- drivers/target/iscsi/iscsi_target.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) (limited to 'drivers/target/iscsi') diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index 541f66a875fc..048d4227327c 100644 --- a/drivers/target/iscsi/iscsi_target.c +++ b/drivers/target/iscsi/iscsi_target.c @@ -1955,7 +1955,6 @@ iscsit_handle_task_mgt_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd, struct iscsi_tmr_req *tmr_req; struct iscsi_tm *hdr; int out_of_order_cmdsn = 0, ret; - bool sess_ref = false; u8 function, tcm_function = TMR_UNKNOWN; hdr = (struct iscsi_tm *) buf; @@ -1988,22 +1987,23 @@ iscsit_handle_task_mgt_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd, cmd->data_direction = DMA_NONE; cmd->tmr_req = kzalloc(sizeof(*cmd->tmr_req), GFP_KERNEL); - if (!cmd->tmr_req) + if (!cmd->tmr_req) { return iscsit_add_reject_cmd(cmd, ISCSI_REASON_BOOKMARK_NO_RESOURCES, buf); + } + + transport_init_se_cmd(&cmd->se_cmd, &iscsi_ops, + conn->sess->se_sess, 0, DMA_NONE, + TCM_SIMPLE_TAG, cmd->sense_buffer + 2); + + target_get_sess_cmd(&cmd->se_cmd, true); /* * TASK_REASSIGN for ERL=2 / connection stays inside of * LIO-Target $FABRIC_MOD */ if (function != ISCSI_TM_FUNC_TASK_REASSIGN) { - transport_init_se_cmd(&cmd->se_cmd, &iscsi_ops, - conn->sess->se_sess, 0, DMA_NONE, - TCM_SIMPLE_TAG, cmd->sense_buffer + 2); - - target_get_sess_cmd(&cmd->se_cmd, true); - sess_ref = true; tcm_function = iscsit_convert_tmf(function); if (tcm_function == TMR_UNKNOWN) { pr_err("Unknown iSCSI TMR Function:" @@ -2119,12 +2119,8 @@ attach: * For connection recovery, this is also the default action for * TMR TASK_REASSIGN. */ - if (sess_ref) { - pr_debug("Handle TMR, using sess_ref=true check\n"); - target_put_sess_cmd(&cmd->se_cmd); - } - iscsit_add_cmd_to_response_queue(cmd, conn, cmd->i_state); + target_put_sess_cmd(&cmd->se_cmd); return 0; } EXPORT_SYMBOL(iscsit_handle_task_mgt_cmd); -- cgit v1.2.3 From 3fc9fb13a4b2576aeab86c62fd64eb29ab68659c Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Fri, 27 Oct 2017 20:52:56 -0700 Subject: iscsi-target: Fix non-immediate TMR reference leak This patch fixes a se_cmd->cmd_kref reference leak that can occur when a non immediate TMR is proceeded our of command sequence number order, and CMDSN_LOWER_THAN_EXP is returned by iscsit_sequence_cmd(). To address this bug, call target_put_sess_cmd() during this special case following what iscsit_process_scsi_cmd() does upon CMDSN_LOWER_THAN_EXP. Cc: Mike Christie Cc: Hannes Reinecke Cc: stable@vger.kernel.org # 3.10+ Signed-off-by: Nicholas Bellinger --- drivers/target/iscsi/iscsi_target.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'drivers/target/iscsi') diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index 048d4227327c..3b7bb589d301 100644 --- a/drivers/target/iscsi/iscsi_target.c +++ b/drivers/target/iscsi/iscsi_target.c @@ -2094,12 +2094,14 @@ attach: if (!(hdr->opcode & ISCSI_OP_IMMEDIATE)) { int cmdsn_ret = iscsit_sequence_cmd(conn, cmd, buf, hdr->cmdsn); - if (cmdsn_ret == CMDSN_HIGHER_THAN_EXP) + if (cmdsn_ret == CMDSN_HIGHER_THAN_EXP) { out_of_order_cmdsn = 1; - else if (cmdsn_ret == CMDSN_LOWER_THAN_EXP) + } else if (cmdsn_ret == CMDSN_LOWER_THAN_EXP) { + target_put_sess_cmd(&cmd->se_cmd); return 0; - else if (cmdsn_ret == CMDSN_ERROR_CANNOT_RECOVER) + } else if (cmdsn_ret == CMDSN_ERROR_CANNOT_RECOVER) { return -1; + } } iscsit_ack_from_expstatsn(conn, be32_to_cpu(hdr->exp_statsn)); -- cgit v1.2.3