Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751810AbdINQWz (ORCPT ); Thu, 14 Sep 2017 12:22:55 -0400 Received: from omzsmtpe02.verizonbusiness.com ([199.249.25.209]:7286 "EHLO omzsmtpe02.verizonbusiness.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751645AbdINPwS (ORCPT ); Thu, 14 Sep 2017 11:52:18 -0400 From: "Levin, Alexander (Sasha Levin)" Cc: Jitendra Bhivare , "Martin K . Petersen" , "Levin, Alexander (Sasha Levin)" X-Host: viking.odc.vzwcorp.com To: "linux-kernel@vger.kernel.org" , "stable@vger.kernel.org" Subject: [PATCH for 4.9 16/59] scsi: be2iscsi: Add checks to validate CID alloc/free Thread-Topic: [PATCH for 4.9 16/59] scsi: be2iscsi: Add checks to validate CID alloc/free Thread-Index: AQHTLXFJ140vEMVwtk2oGipxgEaKOQ== Date: Thu, 14 Sep 2017 15:51:10 +0000 Message-ID: <20170914155051.8289-16-alexander.levin@verizon.com> References: <20170914155051.8289-1-alexander.levin@verizon.com> In-Reply-To: <20170914155051.8289-1-alexander.levin@verizon.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-messagesentrepresentingtype: 1 x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.144.60.250] Content-Type: text/plain; charset="iso-8859-1" MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id v8EGNE5h014716 Content-Length: 11216 Lines: 326 From: Jitendra Bhivare [ Upstream commit 413f365657a8b9669bd0ba3628e9fde9ce63604e ] Set CID slot to 0xffff to indicate empty. Check if connection already exists in conn_table before binding. Check if endpoint already NULL before putting back CID. Break ep->conn link in free_ep to ignore completions after freeing. Signed-off-by: Jitendra Bhivare Reviewed-by: Hannes Reinecke Signed-off-by: Martin K. Petersen Signed-off-by: Sasha Levin --- drivers/scsi/be2iscsi/be_iscsi.c | 163 ++++++++++++++++++++------------------- drivers/scsi/be2iscsi/be_main.c | 7 +- drivers/scsi/be2iscsi/be_main.h | 1 + 3 files changed, 87 insertions(+), 84 deletions(-) diff --git a/drivers/scsi/be2iscsi/be_iscsi.c b/drivers/scsi/be2iscsi/be_iscsi.c index ba258217614e..963c732a3d24 100644 --- a/drivers/scsi/be2iscsi/be_iscsi.c +++ b/drivers/scsi/be2iscsi/be_iscsi.c @@ -166,33 +166,6 @@ beiscsi_conn_create(struct iscsi_cls_session *cls_session, u32 cid) } /** - * beiscsi_bindconn_cid - Bind the beiscsi_conn with phba connection table - * @beiscsi_conn: The pointer to beiscsi_conn structure - * @phba: The phba instance - * @cid: The cid to free - */ -static int beiscsi_bindconn_cid(struct beiscsi_hba *phba, - struct beiscsi_conn *beiscsi_conn, - unsigned int cid) -{ - uint16_t cri_index = BE_GET_CRI_FROM_CID(cid); - - if (phba->conn_table[cri_index]) { - beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_CONFIG, - "BS_%d : Connection table already occupied. Detected clash\n"); - - return -EINVAL; - } else { - beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_CONFIG, - "BS_%d : phba->conn_table[%d]=%p(beiscsi_conn)\n", - cri_index, beiscsi_conn); - - phba->conn_table[cri_index] = beiscsi_conn; - } - return 0; -} - -/** * beiscsi_conn_bind - Binds iscsi session/connection with TCP connection * @cls_session: pointer to iscsi cls session * @cls_conn: pointer to iscsi cls conn @@ -212,6 +185,7 @@ int beiscsi_conn_bind(struct iscsi_cls_session *cls_session, struct hwi_wrb_context *pwrb_context; struct beiscsi_endpoint *beiscsi_ep; struct iscsi_endpoint *ep; + uint16_t cri_index; ep = iscsi_lookup_endpoint(transport_fd); if (!ep) @@ -229,20 +203,34 @@ int beiscsi_conn_bind(struct iscsi_cls_session *cls_session, return -EEXIST; } - - pwrb_context = &phwi_ctrlr->wrb_context[BE_GET_CRI_FROM_CID( - beiscsi_ep->ep_cid)]; + cri_index = BE_GET_CRI_FROM_CID(beiscsi_ep->ep_cid); + if (phba->conn_table[cri_index]) { + if (beiscsi_conn != phba->conn_table[cri_index] || + beiscsi_ep != phba->conn_table[cri_index]->ep) { + __beiscsi_log(phba, KERN_ERR, + "BS_%d : conn_table not empty at %u: cid %u conn %p:%p\n", + cri_index, + beiscsi_ep->ep_cid, + beiscsi_conn, + phba->conn_table[cri_index]); + return -EINVAL; + } + } beiscsi_conn->beiscsi_conn_cid = beiscsi_ep->ep_cid; beiscsi_conn->ep = beiscsi_ep; beiscsi_ep->conn = beiscsi_conn; + /** + * Each connection is associated with a WRBQ kept in wrb_context. + * Store doorbell offset for transmit path. + */ + pwrb_context = &phwi_ctrlr->wrb_context[cri_index]; beiscsi_conn->doorbell_offset = pwrb_context->doorbell_offset; - beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_CONFIG, - "BS_%d : beiscsi_conn=%p conn=%p ep_cid=%d\n", - beiscsi_conn, conn, beiscsi_ep->ep_cid); - - return beiscsi_bindconn_cid(phba, beiscsi_conn, beiscsi_ep->ep_cid); + "BS_%d : cid %d phba->conn_table[%u]=%p\n", + beiscsi_ep->ep_cid, cri_index, beiscsi_conn); + phba->conn_table[cri_index] = beiscsi_conn; + return 0; } static int beiscsi_iface_create_ipv4(struct beiscsi_hba *phba) @@ -973,9 +961,9 @@ int beiscsi_conn_start(struct iscsi_cls_conn *cls_conn) */ static int beiscsi_get_cid(struct beiscsi_hba *phba) { - unsigned short cid = 0xFFFF, cid_from_ulp; - struct ulp_cid_info *cid_info = NULL; uint16_t cid_avlbl_ulp0, cid_avlbl_ulp1; + unsigned short cid, cid_from_ulp; + struct ulp_cid_info *cid_info; /* Find the ULP which has more CID available */ cid_avlbl_ulp0 = (phba->cid_array_info[BEISCSI_ULP0]) ? @@ -984,20 +972,27 @@ static int beiscsi_get_cid(struct beiscsi_hba *phba) BEISCSI_ULP1_AVLBL_CID(phba) : 0; cid_from_ulp = (cid_avlbl_ulp0 > cid_avlbl_ulp1) ? BEISCSI_ULP0 : BEISCSI_ULP1; - - if (test_bit(cid_from_ulp, (void *)&phba->fw_config.ulp_supported)) { - cid_info = phba->cid_array_info[cid_from_ulp]; - if (!cid_info->avlbl_cids) - return cid; - - cid = cid_info->cid_array[cid_info->cid_alloc++]; - - if (cid_info->cid_alloc == BEISCSI_GET_CID_COUNT( - phba, cid_from_ulp)) - cid_info->cid_alloc = 0; - - cid_info->avlbl_cids--; + /** + * If iSCSI protocol is loaded only on ULP 0, and when cid_avlbl_ulp + * is ZERO for both, ULP 1 is returned. + * Check if ULP is loaded before getting new CID. + */ + if (!test_bit(cid_from_ulp, (void *)&phba->fw_config.ulp_supported)) + return BE_INVALID_CID; + + cid_info = phba->cid_array_info[cid_from_ulp]; + cid = cid_info->cid_array[cid_info->cid_alloc]; + if (!cid_info->avlbl_cids || cid == BE_INVALID_CID) { + __beiscsi_log(phba, KERN_ERR, + "BS_%d : failed to get cid: available %u:%u\n", + cid_info->avlbl_cids, cid_info->cid_free); + return BE_INVALID_CID; } + /* empty the slot */ + cid_info->cid_array[cid_info->cid_alloc++] = BE_INVALID_CID; + if (cid_info->cid_alloc == BEISCSI_GET_CID_COUNT(phba, cid_from_ulp)) + cid_info->cid_alloc = 0; + cid_info->avlbl_cids--; return cid; } @@ -1008,22 +1003,28 @@ static int beiscsi_get_cid(struct beiscsi_hba *phba) */ static void beiscsi_put_cid(struct beiscsi_hba *phba, unsigned short cid) { - uint16_t cid_post_ulp; - struct hwi_controller *phwi_ctrlr; - struct hwi_wrb_context *pwrb_context; - struct ulp_cid_info *cid_info = NULL; uint16_t cri_index = BE_GET_CRI_FROM_CID(cid); + struct hwi_wrb_context *pwrb_context; + struct hwi_controller *phwi_ctrlr; + struct ulp_cid_info *cid_info; + uint16_t cid_post_ulp; phwi_ctrlr = phba->phwi_ctrlr; pwrb_context = &phwi_ctrlr->wrb_context[cri_index]; cid_post_ulp = pwrb_context->ulp_num; cid_info = phba->cid_array_info[cid_post_ulp]; - cid_info->avlbl_cids++; - + /* fill only in empty slot */ + if (cid_info->cid_array[cid_info->cid_free] != BE_INVALID_CID) { + __beiscsi_log(phba, KERN_ERR, + "BS_%d : failed to put cid %u: available %u:%u\n", + cid, cid_info->avlbl_cids, cid_info->cid_free); + return; + } cid_info->cid_array[cid_info->cid_free++] = cid; if (cid_info->cid_free == BEISCSI_GET_CID_COUNT(phba, cid_post_ulp)) cid_info->cid_free = 0; + cid_info->avlbl_cids++; } /** @@ -1037,8 +1038,8 @@ static void beiscsi_free_ep(struct beiscsi_endpoint *beiscsi_ep) beiscsi_put_cid(phba, beiscsi_ep->ep_cid); beiscsi_ep->phba = NULL; - phba->ep_array[BE_GET_CRI_FROM_CID - (beiscsi_ep->ep_cid)] = NULL; + /* clear this to track freeing in beiscsi_ep_disconnect */ + phba->ep_array[BE_GET_CRI_FROM_CID(beiscsi_ep->ep_cid)] = NULL; /** * Check if any connection resource allocated by driver @@ -1049,6 +1050,11 @@ static void beiscsi_free_ep(struct beiscsi_endpoint *beiscsi_ep) return; beiscsi_conn = beiscsi_ep->conn; + /** + * Break ep->conn link here so that completions after + * this are ignored. + */ + beiscsi_ep->conn = NULL; if (beiscsi_conn->login_in_progress) { beiscsi_free_mgmt_task_handles(beiscsi_conn, beiscsi_conn->task); @@ -1079,7 +1085,7 @@ static int beiscsi_open_conn(struct iscsi_endpoint *ep, "BS_%d : In beiscsi_open_conn\n"); beiscsi_ep->ep_cid = beiscsi_get_cid(phba); - if (beiscsi_ep->ep_cid == 0xFFFF) { + if (beiscsi_ep->ep_cid == BE_INVALID_CID) { beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_CONFIG, "BS_%d : No free cid available\n"); return ret; @@ -1285,26 +1291,6 @@ static int beiscsi_close_conn(struct beiscsi_endpoint *beiscsi_ep, int flag) } /** - * beiscsi_unbind_conn_to_cid - Unbind the beiscsi_conn from phba conn table - * @phba: The phba instance - * @cid: The cid to free - */ -static int beiscsi_unbind_conn_to_cid(struct beiscsi_hba *phba, - unsigned int cid) -{ - uint16_t cri_index = BE_GET_CRI_FROM_CID(cid); - - if (phba->conn_table[cri_index]) - phba->conn_table[cri_index] = NULL; - else { - beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_CONFIG, - "BS_%d : Connection table Not occupied.\n"); - return -EINVAL; - } - return 0; -} - -/** * beiscsi_ep_disconnect - Tears down the TCP connection * @ep: endpoint to be used * @@ -1318,13 +1304,23 @@ void beiscsi_ep_disconnect(struct iscsi_endpoint *ep) unsigned int tag; uint8_t mgmt_invalidate_flag, tcp_upload_flag; unsigned short savecfg_flag = CMD_ISCSI_SESSION_SAVE_CFG_ON_FLASH; + uint16_t cri_index; beiscsi_ep = ep->dd_data; phba = beiscsi_ep->phba; beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_CONFIG, - "BS_%d : In beiscsi_ep_disconnect for ep_cid = %d\n", + "BS_%d : In beiscsi_ep_disconnect for ep_cid = %u\n", beiscsi_ep->ep_cid); + cri_index = BE_GET_CRI_FROM_CID(beiscsi_ep->ep_cid); + if (!phba->ep_array[cri_index]) { + __beiscsi_log(phba, KERN_ERR, + "BS_%d : ep_array at %u cid %u empty\n", + cri_index, + beiscsi_ep->ep_cid); + return; + } + if (beiscsi_ep->conn) { beiscsi_conn = beiscsi_ep->conn; iscsi_suspend_queue(beiscsi_conn->conn); @@ -1356,7 +1352,12 @@ void beiscsi_ep_disconnect(struct iscsi_endpoint *ep) free_ep: msleep(BEISCSI_LOGOUT_SYNC_DELAY); beiscsi_free_ep(beiscsi_ep); - beiscsi_unbind_conn_to_cid(phba, beiscsi_ep->ep_cid); + if (!phba->conn_table[cri_index]) + __beiscsi_log(phba, KERN_ERR, + "BS_%d : conn_table empty at %u: cid %u\n", + cri_index, + beiscsi_ep->ep_cid); + phba->conn_table[cri_index] = NULL; iscsi_destroy_endpoint(beiscsi_ep->openiscsi_ep); } diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c index d9239c2d49b1..741cc96379cb 100644 --- a/drivers/scsi/be2iscsi/be_main.c +++ b/drivers/scsi/be2iscsi/be_main.c @@ -4085,9 +4085,10 @@ static int hba_setup_cid_tbls(struct beiscsi_hba *phba) } /* Allocate memory for CID array */ - ptr_cid_info->cid_array = kzalloc(sizeof(void *) * - BEISCSI_GET_CID_COUNT(phba, - ulp_num), GFP_KERNEL); + ptr_cid_info->cid_array = + kcalloc(BEISCSI_GET_CID_COUNT(phba, ulp_num), + sizeof(*ptr_cid_info->cid_array), + GFP_KERNEL); if (!ptr_cid_info->cid_array) { beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT, "BM_%d : Failed to allocate memory" diff --git a/drivers/scsi/be2iscsi/be_main.h b/drivers/scsi/be2iscsi/be_main.h index 6376657e45f7..02d00ab6d981 100644 --- a/drivers/scsi/be2iscsi/be_main.h +++ b/drivers/scsi/be2iscsi/be_main.h @@ -358,6 +358,7 @@ struct beiscsi_hba { unsigned int age; struct list_head hba_queue; #define BE_MAX_SESSION 2048 +#define BE_INVALID_CID 0xffff #define BE_SET_CID_TO_CRI(cri_index, cid) \ (phba->cid_to_cri_map[cid] = cri_index) #define BE_GET_CRI_FROM_CID(cid) (phba->cid_to_cri_map[cid]) -- 2.11.0