scsi: be2iscsi: Add checks to validate CID alloc/free

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 <jitendra.bhivare@broadcom.com>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/scsi/be2iscsi/be_iscsi.c b/drivers/scsi/be2iscsi/be_iscsi.c
index 0ec06256a1b8bb6963ff30a2e63dcde723a52d4a..a4844578e3577e833952af48e02dcb5570446d97 100644 (file)
--- a/drivers/scsi/be2iscsi/be_iscsi.c
+++ b/drivers/scsi/be2iscsi/be_iscsi.c
@@ -165,33 +165,6 @@ beiscsi_conn_create(struct iscsi_cls_session *cls_session, u32 cid)
        return cls_conn;
 }
 
-/**
- * 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
@@ -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;
@@ -1284,26 +1290,6 @@ static int beiscsi_close_conn(struct  beiscsi_endpoint *beiscsi_ep, int flag)
        return ret;
 }
 
-/**
- * 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 a68f1beda693865d4dfcb2c57283a1c58888cc88..a8a23d61fd9045ed497c6271f1608dd3c6951bc8 100644 (file)
--- a/drivers/scsi/be2iscsi/be_main.c
+++ b/drivers/scsi/be2iscsi/be_main.c
@@ -4074,9 +4074,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 30379de98dcb85fd090eaa95a7073437edb2d416..02a9b2d0d643272cd9ab610c1c9b769441e3bce2 100644 (file)
--- a/drivers/scsi/be2iscsi/be_main.h
+++ b/drivers/scsi/be2iscsi/be_main.h
@@ -343,6 +343,7 @@ struct beiscsi_hba {
        spinlock_t async_pdu_lock;
        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])