In the passthru IOCTL path, the mfi command pool was freely accessible N times

where as there are limited number(32) of mfi commands in the pool.
The mfi command pool is now restricted to 27 simultaneous accesses by using
a counting semaphore while calling the passthru function.

In the mrsas_cam.c source file there was a same function name mrsas_poll(),
which was same as the mrsas_poll() implemented in the mrsas.c file for the
polling interface.
To clearly distinguish the functionality by usage we have renamed the former
as mrsas_cam_poll().

In the passthru function let's say it has got an mfi command from the pool
but it has failed in one of the DMA function call which will lead to leak
an mfi command because in the ERROR case it directly returns and not freeing up
the occupied mfi command.

Reviewed by:	ambrisko
MFC after:	2 weeks
Sponsored by:	AVAGO Technologies

4 files changed, 62 insertions, 18 deletions

diff --git a/sys/dev/mrsas/mrsas.c b/sys/dev/mrsas/mrsas.c
index c491404..8fce466 100644
--- a/sys/dev/mrsas/mrsas.c
+++ b/sys/dev/mrsas/mrsas.c
@@ -823,6 +823,9 @@ static int mrsas_attach(device_t dev)
     mtx_init(&sc->mfi_cmd_pool_lock, "mrsas_mfi_cmd_pool_lock", NULL, MTX_DEF);
     mtx_init(&sc->raidmap_lock, "mrsas_raidmap_lock", NULL, MTX_DEF);
 
+    /* Intialize a counting Semaphore to take care no. of concurrent IOCTLs */
+    sema_init(&sc->ioctl_count_sema, MRSAS_MAX_MFI_CMDS-5, IOCTL_SEMA_DESCRIPTION);
+
     /* Intialize linked list */
     TAILQ_INIT(&sc->mrsas_mpt_cmd_list_head);
     TAILQ_INIT(&sc->mrsas_mfi_cmd_list_head);
@@ -912,6 +915,8 @@ attach_fail_fw:
     mtx_destroy(&sc->mpt_cmd_pool_lock);
     mtx_destroy(&sc->mfi_cmd_pool_lock);
     mtx_destroy(&sc->raidmap_lock);
+    /* Destroy the counting semaphore created for Ioctl */
+    sema_destroy(&sc->ioctl_count_sema);
 attach_fail:
     destroy_dev(sc->mrsas_cdev);
     if (sc->reg_res){
@@ -937,10 +942,13 @@ static int mrsas_detach(device_t dev)
     sc = device_get_softc(dev);
     sc->remove_in_progress = 1;
 
+    /* Destroy the character device so no other IOCTL will be handled */
+    destroy_dev(sc->mrsas_cdev);
+
     /*
      * Take the instance off the instance array. Note that we will not
      * decrement the max_index. We let this array be sparse array
-    */
+     */
     for (i = 0; i < mrsas_mgmt_info.max_index; i++) {
             if (mrsas_mgmt_info.sc_ptr[i] == sc) {
                     mrsas_mgmt_info.count--;
@@ -984,13 +992,22 @@ static int mrsas_detach(device_t dev)
     mtx_destroy(&sc->mpt_cmd_pool_lock);
     mtx_destroy(&sc->mfi_cmd_pool_lock);
     mtx_destroy(&sc->raidmap_lock);
+
+    /* Wait for all the semaphores to be released */
+    while (sema_value(&sc->ioctl_count_sema) != (MRSAS_MAX_MFI_CMDS-5))
+        pause("mr_shutdown", hz);
+
+    /* Destroy the counting semaphore created for Ioctl */
+    sema_destroy(&sc->ioctl_count_sema);
+
     if (sc->reg_res){
         bus_release_resource(sc->mrsas_dev,
                  SYS_RES_MEMORY, sc->reg_res_id, sc->reg_res);
     }
-    destroy_dev(sc->mrsas_cdev);
+
     if (sc->sysctl_tree != NULL)
         sysctl_ctx_free(&sc->sysctl_ctx);
+
     return (0);
 }
 
@@ -1254,13 +1271,21 @@ do_ioctl:
 #ifdef COMPAT_FREEBSD32
         case MRSAS_IOC_FIRMWARE_PASS_THROUGH32:
 #endif
+            /* Decrement the Ioctl counting Semaphore before getting an mfi command */
+            sema_wait(&sc->ioctl_count_sema);
+
             ret = mrsas_passthru(sc, (void *)arg, cmd);
+
+	    /* Increment the Ioctl counting semaphore value */
+	    sema_post(&sc->ioctl_count_sema);
+
             break;
         case MRSAS_IOC_SCAN_BUS:
             ret = mrsas_bus_scan(sc);
             break;
 	default:
 		mrsas_dprint(sc, MRSAS_TRACE, "IOCTL command 0x%lx is not handled\n", cmd);
+		ret = ENOENT;
     }
  
     return (ret);
diff --git a/sys/dev/mrsas/mrsas.h b/sys/dev/mrsas/mrsas.h
index 19f119b..5830f8bc 100644
--- a/sys/dev/mrsas/mrsas.h
+++ b/sys/dev/mrsas/mrsas.h
@@ -67,12 +67,16 @@ __FBSDID("$FreeBSD$");
 #include <dev/pci/pcireg.h>
 
 #include <sys/types.h>
+#include <sys/lock.h>
+#include <sys/sema.h>
 #include <sys/sysctl.h>
 #include <sys/stat.h>
 #include <sys/taskqueue.h>
 #include <sys/poll.h>
 #include <sys/selinfo.h>
 
+#define IOCTL_SEMA_DESCRIPTION	"mrsas semaphore for MFI pool"
+
 /*
  * Device IDs and PCI
  */
@@ -2494,6 +2498,8 @@ struct mrsas_softc {
     struct selinfo mrsas_select;	  // poll select interface for application
     uint32_t mrsas_aen_triggered;
     uint32_t mrsas_poll_waiting;
+
+    struct sema ioctl_count_sema;         // counting semaphore for ioctl
     uint32_t           max_fw_cmds;       // Max commands from FW
     uint32_t           max_num_sge;       // Max number of SGEs
     struct resource    *mrsas_irq[MAX_MSIX_COUNT];        // interrupt interface window
diff --git a/sys/dev/mrsas/mrsas_cam.c b/sys/dev/mrsas/mrsas_cam.c
index f4d0838..03f05b0 100644
--- a/sys/dev/mrsas/mrsas_cam.c
+++ b/sys/dev/mrsas/mrsas_cam.c
@@ -86,7 +86,7 @@ void mrsas_set_pd_lba(MRSAS_RAID_SCSI_IO_REQUEST *io_request, u_int8_t cdb_len,
                     MR_DRV_RAID_MAP_ALL *local_map_ptr, u_int32_t ref_tag,
                     u_int32_t ld_block_size);
 static void mrsas_freeze_simq(struct mrsas_mpt_cmd *cmd, struct cam_sim *sim);
-static void mrsas_poll(struct cam_sim *sim);
+static void mrsas_cam_poll(struct cam_sim *sim);
 static void mrsas_action(struct cam_sim *sim, union ccb *ccb);
 static void mrsas_scsiio_timeout(void *data);
 static void mrsas_data_load_cb(void *arg, bus_dma_segment_t *segs,
@@ -137,7 +137,7 @@ int mrsas_cam_attach(struct mrsas_softc *sc)
     /* 
      * Create SIM for bus 0 and register, also create path 
      */
-    sc->sim_0 = cam_sim_alloc(mrsas_action, mrsas_poll, "mrsas", sc,
+    sc->sim_0 = cam_sim_alloc(mrsas_action, mrsas_cam_poll, "mrsas", sc,
         device_get_unit(sc->mrsas_dev), &sc->sim_lock, mrsas_cam_depth,
         mrsas_cam_depth, devq);
     if (sc->sim_0 == NULL){
@@ -173,7 +173,7 @@ int mrsas_cam_attach(struct mrsas_softc *sc)
     /* 
      * Create SIM for bus 1 and register, also create path 
      */
-    sc->sim_1 = cam_sim_alloc(mrsas_action, mrsas_poll, "mrsas", sc,
+    sc->sim_1 = cam_sim_alloc(mrsas_action, mrsas_cam_poll, "mrsas", sc,
         device_get_unit(sc->mrsas_dev), &sc->sim_lock, mrsas_cam_depth,
         mrsas_cam_depth, devq);
     if (sc->sim_1 == NULL){
@@ -1110,12 +1110,12 @@ void mrsas_cmd_done(struct mrsas_softc *sc, struct mrsas_mpt_cmd *cmd)
 }
 
 /**
- * mrsas_poll:               Polling entry point 
+ * mrsas_cam_poll:               Polling entry point
  * input:                    Pointer to SIM  
  *
  * This is currently a stub function.
  */
-static void mrsas_poll(struct cam_sim *sim)
+static void mrsas_cam_poll(struct cam_sim *sim)
 {
     struct mrsas_softc *sc = (struct mrsas_softc *)cam_sim_softc(sim);
     mrsas_isr((void *) sc);
diff --git a/sys/dev/mrsas/mrsas_ioctl.c b/sys/dev/mrsas/mrsas_ioctl.c
index 1e77fd8..f0066b6 100644
--- a/sys/dev/mrsas/mrsas_ioctl.c
+++ b/sys/dev/mrsas/mrsas_ioctl.c
@@ -168,18 +168,21 @@ int mrsas_passthru( struct mrsas_softc *sc, void *arg, u_long ioctlCmd )
                                 NULL, NULL,             // lockfunc, lockarg
                                 &ioctl_data_tag[i])) {
 		device_printf(sc->mrsas_dev, "Cannot allocate ioctl data tag\n");
-		return (ENOMEM);
+		ret = ENOMEM;
+		goto out;
         }
         if (bus_dmamem_alloc(ioctl_data_tag[i], (void **)&ioctl_data_mem[i],
                 (BUS_DMA_NOWAIT | BUS_DMA_ZERO), &ioctl_data_dmamap[i])) {
             device_printf(sc->mrsas_dev, "Cannot allocate ioctl data mem\n");
-            return (ENOMEM);
+			ret = ENOMEM;
+			goto out;
         }
         if (bus_dmamap_load(ioctl_data_tag[i], ioctl_data_dmamap[i],
                 ioctl_data_mem[i], ioctl_data_size, mrsas_alloc_cb,
                 &ioctl_data_phys_addr[i], BUS_DMA_NOWAIT)) {
             device_printf(sc->mrsas_dev, "Cannot load ioctl data mem\n");
-            return (ENOMEM);
+			ret = ENOMEM;
+			goto out;
         }
 
         /* Save the physical address and length */
@@ -222,18 +225,21 @@ int mrsas_passthru( struct mrsas_softc *sc, void *arg, u_long ioctlCmd )
                                 NULL, NULL,             // lockfunc, lockarg
                                 &ioctl_sense_tag)) {
             device_printf(sc->mrsas_dev, "Cannot allocate ioctl sense tag\n");
-            return (ENOMEM);
+            ret = ENOMEM;
+			goto out;
         }
         if (bus_dmamem_alloc(ioctl_sense_tag, (void **)&ioctl_sense_mem,
                 (BUS_DMA_NOWAIT | BUS_DMA_ZERO), &ioctl_sense_dmamap)) {
-            device_printf(sc->mrsas_dev, "Cannot allocate ioctl data mem\n");
-            return (ENOMEM);
+            device_printf(sc->mrsas_dev, "Cannot allocate ioctl sense mem\n");
+            ret = ENOMEM;
+			goto out;
         }
         if (bus_dmamap_load(ioctl_sense_tag, ioctl_sense_dmamap,
                 ioctl_sense_mem, ioctl_sense_size, mrsas_alloc_cb,
                 &ioctl_sense_phys_addr, BUS_DMA_NOWAIT)) {
             device_printf(sc->mrsas_dev, "Cannot load ioctl sense mem\n");
-            return (ENOMEM);
+            ret = ENOMEM;
+			goto out;
         }
         sense_ptr = 
             (unsigned long *)((unsigned long)cmd->frame + user_ioc->sense_off);
@@ -299,17 +305,24 @@ out:
      */
     if (ioctl_sense_phys_addr)
         bus_dmamap_unload(ioctl_sense_tag, ioctl_sense_dmamap);
-    if (ioctl_sense_mem)
+    if (ioctl_sense_mem != NULL)
         bus_dmamem_free(ioctl_sense_tag, ioctl_sense_mem, ioctl_sense_dmamap);
-    if (ioctl_sense_tag)
+    if (ioctl_sense_tag != NULL)
         bus_dma_tag_destroy(ioctl_sense_tag);
 
     /* 
      * Release data buffers 
      */
     for (i = 0; i < user_ioc->sge_count; i++) {
-        if (!user_ioc->sgl[i].iov_len)
-            continue;
+	    if (ioctlCmd == MRSAS_IOC_FIRMWARE_PASS_THROUGH64) {
+		    if (!user_ioc->sgl[i].iov_len)
+			    continue;
+#ifdef COMPAT_FREEBSD32
+	    } else {
+		    if (!user_ioc32->sgl[i].iov_len)
+			    continue;
+#endif
+	    }
         if (ioctl_data_phys_addr[i])
             bus_dmamap_unload(ioctl_data_tag[i], ioctl_data_dmamap[i]);
         if (ioctl_data_mem[i] != NULL)