Author: asomers
Date: Fri Feb 23 00:17:50 2018
New Revision: 329845
URL: https://svnweb.freebsd.org/changeset/base/329845

Log:
  Fix numerous Coverity issues in mptutil
  
  Most are memory or file descriptor leaks. Three were unannotated
  fallthroughs in a switch/case statement. One was an integer overflow before
  widen.
  
  Reported by:  Coverity
  CID:          1007463 1007462 1007461 1007460 1007459 1007458 1007457
  CID:          1006855 1006854 1006853 1006852 1006851 1006850 1006849
  CID:          1006848 1006845 1006844 1006843 1006842 1006841 1006840
  CID:          1006839 1006838 1006837 1006836 1006835 1006834 1006833
  CID:          1006832 1006831 1006831 1006830 1006829 1008334 1008170
  CID:          1008169 1008168
  MFC after:    3 weeks
  Sponsored by: Spectra Logic Corp
  Differential Revision:        https://reviews.freebsd.org/D11013

Modified:
  head/usr.sbin/mptutil/mpt_config.c
  head/usr.sbin/mptutil/mpt_drive.c
  head/usr.sbin/mptutil/mpt_evt.c
  head/usr.sbin/mptutil/mpt_show.c
  head/usr.sbin/mptutil/mpt_volume.c

Modified: head/usr.sbin/mptutil/mpt_config.c
==============================================================================
--- head/usr.sbin/mptutil/mpt_config.c  Fri Feb 23 00:12:51 2018        
(r329844)
+++ head/usr.sbin/mptutil/mpt_config.c  Fri Feb 23 00:17:50 2018        
(r329845)
@@ -67,12 +67,16 @@ dehumanize(const char *value)
         switch (vtp[0]) {
         case 't': case 'T':
                 iv *= 1024;
+                /* FALLTHROUGH */
         case 'g': case 'G':
                 iv *= 1024;
+                /* FALLTHROUGH */
         case 'm': case 'M':
                 iv *= 1024;
+                /* FALLTHROUGH */
         case 'k': case 'K':
                 iv *= 1024;
+                /* FALLTHROUGH */
         case '\0':
                 break;
         default:
@@ -244,6 +248,7 @@ clear_config(int ac, char **av)
        if (ioc2 == NULL) {
                error = errno;
                warn("Failed to fetch volume list");
+               close(fd);
                return (error);
        }
 
@@ -253,6 +258,8 @@ clear_config(int ac, char **av)
                if (mpt_lock_volume(vol->VolumeBus, vol->VolumeID) < 0) {
                        warnx("Volume %s is busy and cannot be deleted",
                            mpt_volume_name(vol->VolumeBus, vol->VolumeID));
+                       free(ioc2);
+                       close(fd);
                        return (EBUSY);
                }
        }
@@ -263,6 +270,8 @@ clear_config(int ac, char **av)
        ch = getchar();
        if (ch != 'y' && ch != 'Y') {
                printf("\nAborting\n");
+               free(ioc2);
+               close(fd);
                return (0);
        }
 
@@ -557,16 +566,16 @@ build_volume(int fd, struct volume_info *info, int rai
        case RT_RAID0:
                vol->VolumeType = MPI_RAID_VOL_TYPE_IS;
                vol->StripeSize = stripe_size / 512;
-               MaxLBA = MinLBA * info->drive_count;
+               MaxLBA = (uint64_t)MinLBA * info->drive_count;
                break;
        case RT_RAID1:
                vol->VolumeType = MPI_RAID_VOL_TYPE_IM;
-               MaxLBA = MinLBA * (info->drive_count / 2);
+               MaxLBA = (uint64_t)MinLBA * (info->drive_count / 2);
                break;
        case RT_RAID1E:
                vol->VolumeType = MPI_RAID_VOL_TYPE_IME;
                vol->StripeSize = stripe_size / 512;
-               MaxLBA = MinLBA * info->drive_count / 2;
+               MaxLBA = (uint64_t)MinLBA * info->drive_count / 2;
                break;
        default:
                /* Pacify gcc. */
@@ -645,6 +654,7 @@ create_volume(int ac, char **av)
 
        if (raid_type == -1) {
                warnx("Unknown or unsupported volume type %s", av[1]);
+               close(fd);
                return (EINVAL);
        }
 
@@ -671,6 +681,7 @@ create_volume(int ac, char **av)
                        stripe_size = dehumanize(optarg);
                        if ((stripe_size < 512) || (!powerof2(stripe_size))) {
                                warnx("Invalid stripe size %s", optarg);
+                               close(fd);
                                return (EINVAL);
                        }
                        break;
@@ -679,6 +690,7 @@ create_volume(int ac, char **av)
                        break;
                case '?':
                default:
+                       close(fd);
                        return (EINVAL);
                }
        }
@@ -690,14 +702,18 @@ create_volume(int ac, char **av)
        if (state.ioc2 == NULL) {
                error = errno;
                warn("Failed to read volume list");
+               close(fd);
                return (error);
        }
        state.list = mpt_pd_list(fd);
-       if (state.list == NULL)
+       if (state.list == NULL) {
+               close(fd);
                return (errno);
+       }
        error = mpt_fetch_disks(fd, &state.nsdisks, &state.sdisks);
        if (error) {
                warn("Failed to fetch standalone disk list");
+               close(fd);
                return (error);
        }       
        state.target_id = 0xff;
@@ -705,24 +721,36 @@ create_volume(int ac, char **av)
        /* Parse the drive list. */
        if (ac != 1) {
                warnx("Exactly one drive list is required");
+               close(fd);
                return (EINVAL);
        }
        info = calloc(1, sizeof(*info));
-       if (info == NULL)
+       if (info == NULL) {
+               close(fd);
                return (ENOMEM);
+       }
        error = parse_volume(fd, raid_type, &state, av[0], info);
-       if (error)
+       if (error) {
+               free(info);
+               close(fd);
                return (error);
+       }
 
        /* Create RAID physdisk pages for standalone disks. */
        error = add_drives(fd, info, verbose);
-       if (error)
+       if (error) {
+               free(info);
+               close(fd);
                return (error);
+       }
 
        /* Build the volume. */
        vol = build_volume(fd, info, raid_type, stripe_size, &state, verbose);
-       if (vol == NULL)
+       if (vol == NULL) {
+               free(info);
+               close(fd);
                return (errno);
+       }
 
 #ifdef DEBUG
        if (dump) {
@@ -738,6 +766,8 @@ create_volume(int ac, char **av)
        if (error) {
                errno = error;
                warn("Failed to add volume");
+               free(info);
+               close(fd);
                return (error);
        }
 
@@ -779,11 +809,14 @@ delete_volume(int ac, char **av)
        error = mpt_lookup_volume(fd, av[1], &VolumeBus, &VolumeID);
        if (error) {
                warnc(error, "Invalid volume %s", av[1]);
+               close(fd);
                return (error);
        }
 
-       if (mpt_lock_volume(VolumeBus, VolumeID) < 0)
+       if (mpt_lock_volume(VolumeBus, VolumeID) < 0) {
+               close(fd);
                return (errno);
+       }
 
        error = mpt_raid_action(fd, MPI_RAID_ACTION_DELETE_VOLUME, VolumeBus,
            VolumeID, 0, MPI_RAID_ACTION_ADATA_DEL_PHYS_DISKS |
@@ -791,6 +824,7 @@ delete_volume(int ac, char **av)
            NULL, 0);
        if (error) {
                warnc(error, "Failed to delete volume");
+               close(fd);
                return (error);
        }
 
@@ -828,6 +862,7 @@ find_volume_spare_pool(int fd, const char *name, int *
            0) {
                *pool = 1 << (ffs(info->VolumeSettings.HotSparePool &
                    ~MPI_RAID_HOT_SPARE_POOL_0) - 1);
+               free(info);
                return (0);
        }
        free(info);
@@ -873,6 +908,7 @@ find_volume_spare_pool(int fd, const char *name, int *
        if (error) {
                warnx("Failed to add spare pool %d to %s", new_pool,
                    mpt_volume_name(VolumeBus, VolumeID));
+               free(info);
                return (error);
        }
        free(info);
@@ -908,8 +944,10 @@ add_spare(int ac, char **av)
 
        if (ac == 3) {
                error = find_volume_spare_pool(fd, av[2], &pool);
-               if (error)
+               if (error) {
+                       close(fd);
                        return (error);
+               }
        } else
                pool = MPI_RAID_HOT_SPARE_POOL_0;
 
@@ -922,6 +960,9 @@ add_spare(int ac, char **av)
                error = mpt_fetch_disks(fd, &nsdisks, &sdisks);
                if (error != 0) {
                        warn("Failed to fetch standalone disk list");
+                       mpt_free_pd_list(list);
+                       mpt_free_pd_list(list);
+                       close(fd);
                        return (error);
                }
 
@@ -929,15 +970,25 @@ add_spare(int ac, char **av)
                    0) {
                        error = errno;
                        warn("Unable to lookup drive %s", av[1]);
+                       mpt_free_pd_list(list);
+                       mpt_free_pd_list(list);
+                       close(fd);
                        return (error);
                }
 
-               if (mpt_lock_physdisk(&sdisks[i]) < 0)
+               if (mpt_lock_physdisk(&sdisks[i]) < 0) {
+                       mpt_free_pd_list(list);
+                       mpt_free_pd_list(list);
+                       close(fd);
                        return (errno);
+               }
 
                if (mpt_create_physdisk(fd, &sdisks[i], &PhysDiskNum) < 0) {
                        error = errno;
                        warn("Failed to create physical disk page");
+                       mpt_free_pd_list(list);
+                       mpt_free_pd_list(list);
+                       close(fd);
                        return (error);
                }
                free(sdisks);
@@ -948,6 +999,7 @@ add_spare(int ac, char **av)
        if (info == NULL) {
                error = errno;
                warn("Failed to fetch drive info");
+               close(fd);
                return (error);
        }
 
@@ -957,6 +1009,7 @@ add_spare(int ac, char **av)
            NULL, 0, NULL, NULL, 0);
        if (error) {
                warnc(error, "Failed to assign spare");
+               close(fd);
                return (error);
        }
 
@@ -988,12 +1041,15 @@ remove_spare(int ac, char **av)
        }
 
        list = mpt_pd_list(fd);
-       if (list == NULL)
+       if (list == NULL) {
+               close(fd);
                return (errno);
+       }
 
        error = mpt_lookup_drive(list, av[1], &PhysDiskNum);
        if (error) {
                warn("Failed to find drive %s", av[1]);
+               close(fd);
                return (error);
        }
        mpt_free_pd_list(list);
@@ -1003,17 +1059,22 @@ remove_spare(int ac, char **av)
        if (info == NULL) {
                error = errno;
                warn("Failed to fetch drive info");
+               close(fd);
                return (error);
        }
 
        if (info->PhysDiskSettings.HotSparePool == 0) {
                warnx("Drive %u is not a hot spare", PhysDiskNum);
+               free(info);
+               close(fd);
                return (EINVAL);
        }
 
        if (mpt_delete_physdisk(fd, PhysDiskNum) < 0) {
                error = errno;
                warn("Failed to delete physical disk page");
+               free(info);
+               close(fd);
                return (error);
        }
 

Modified: head/usr.sbin/mptutil/mpt_drive.c
==============================================================================
--- head/usr.sbin/mptutil/mpt_drive.c   Fri Feb 23 00:12:51 2018        
(r329844)
+++ head/usr.sbin/mptutil/mpt_drive.c   Fri Feb 23 00:17:50 2018        
(r329845)
@@ -149,6 +149,7 @@ mpt_pd_list(int fd)
        IOC_5_HOT_SPARE *spare;
        struct mpt_drive_list *list;
        int count, error, i, j;
+       size_t listsize;
 
        ioc2 = mpt_read_ioc_page(fd, 2, NULL);
        if (ioc2 == NULL) {
@@ -191,6 +192,10 @@ mpt_pd_list(int fd)
                        error = errno;
                        warn("Failed to read volume info");
                        errno = error;
+                       free(volumes);
+                       free(ioc5);
+                       free(ioc3);
+                       free(ioc2);
                        return (NULL);
                }
                count += volumes[i]->NumPhysDisks;
@@ -199,15 +204,20 @@ mpt_pd_list(int fd)
        count += ioc5->NumHotSpares;
 
        /* Walk the various lists enumerating drives. */
-       list = malloc(sizeof(*list) + sizeof(CONFIG_PAGE_RAID_PHYS_DISK_0) *
-           count);
-       list->ndrives = 0;
+       listsize = sizeof(*list) + sizeof(CONFIG_PAGE_RAID_PHYS_DISK_0) * count;
+       list = calloc(1, listsize);
 
        for (i = 0; i < ioc2->NumActiveVolumes; i++) {
                rdisk = volumes[i]->PhysDisk;
                for (j = 0; j < volumes[i]->NumPhysDisks; rdisk++, j++)
-                       if (mpt_pd_insert(fd, list, rdisk->PhysDiskNum) < 0)
+                       if (mpt_pd_insert(fd, list, rdisk->PhysDiskNum) < 0) {
+                               mpt_free_pd_list(list);
+                               free(volumes);
+                               free(ioc5);
+                               free(ioc3);
+                               free(ioc2);
                                return (NULL);
+                       }
                free(volumes[i]);
        }
        free(ioc2);
@@ -215,14 +225,21 @@ mpt_pd_list(int fd)
 
        spare = ioc5->HotSpare;
        for (i = 0; i < ioc5->NumHotSpares; spare++, i++)
-               if (mpt_pd_insert(fd, list, spare->PhysDiskNum) < 0)
+               if (mpt_pd_insert(fd, list, spare->PhysDiskNum) < 0) {
+                       mpt_free_pd_list(list);
+                       free(ioc5);
+                       free(ioc3);
                        return (NULL);
+               }
        free(ioc5);
 
        disk = ioc3->PhysDisk;
        for (i = 0; i < ioc3->NumPhysDisks; disk++, i++)
-               if (mpt_pd_insert(fd, list, disk->PhysDiskNum) < 0)
+               if (mpt_pd_insert(fd, list, disk->PhysDiskNum) < 0) {
+                       mpt_free_pd_list(list);
+                       free(ioc3);
                        return (NULL);
+               }
        free(ioc3);
 
        return (list);
@@ -324,12 +341,15 @@ drive_set_state(char *drive, U8 Action, U8 State, cons
        }
 
        list = mpt_pd_list(fd);
-       if (list == NULL)
+       if (list == NULL) {
+               close(fd);
                return (errno);
+       }
 
        if (mpt_lookup_drive(list, drive, &PhysDiskNum) < 0) {
                error = errno;
                warn("Failed to find drive %s", drive);
+               close(fd);
                return (error);
        }
        mpt_free_pd_list(list);
@@ -339,12 +359,15 @@ drive_set_state(char *drive, U8 Action, U8 State, cons
        if (info == NULL) {
                error = errno;
                warn("Failed to fetch info for drive %u", PhysDiskNum);
+               close(fd);
                return (error);
        }
 
        /* Try to change the state. */
        if (info->PhysDiskStatus.State == State) {
                warnx("Drive %u is already in the desired state", PhysDiskNum);
+               free(info);
+               close(fd);
                return (EINVAL);
        }
 
@@ -352,6 +375,8 @@ drive_set_state(char *drive, U8 Action, U8 State, cons
            NULL, 0, NULL, NULL, 0);
        if (error) {
                warnc(error, "Failed to set drive %u to %s", PhysDiskNum, name);
+               free(info);
+               close(fd);
                return (error);
        }
 

Modified: head/usr.sbin/mptutil/mpt_evt.c
==============================================================================
--- head/usr.sbin/mptutil/mpt_evt.c     Fri Feb 23 00:12:51 2018        
(r329844)
+++ head/usr.sbin/mptutil/mpt_evt.c     Fri Feb 23 00:17:50 2018        
(r329845)
@@ -124,6 +124,8 @@ show_events(int ac, char **av)
                        break;
                case '?':
                default:
+                       free(log);
+                       close(fd);
                        return (EINVAL);
                }
        }
@@ -132,8 +134,11 @@ show_events(int ac, char **av)
 
        /* Build a list of valid entries and sort them by sequence. */
        entries = malloc(sizeof(MPI_LOG_0_ENTRY *) * log->NumLogEntries);
-       if (entries == NULL)
+       if (entries == NULL) {
+               free(log);
+               close(fd);
                return (ENOMEM);
+       }
        num_events = 0;
        for (i = 0; i < log->NumLogEntries; i++) {
                if (log->LogEntry[i].LogEntryQualifier ==
@@ -154,6 +159,7 @@ show_events(int ac, char **av)
        }
        
        free(entries);
+       free(log);
        close(fd);
 
        return (0);

Modified: head/usr.sbin/mptutil/mpt_show.c
==============================================================================
--- head/usr.sbin/mptutil/mpt_show.c    Fri Feb 23 00:12:51 2018        
(r329844)
+++ head/usr.sbin/mptutil/mpt_show.c    Fri Feb 23 00:17:50 2018        
(r329845)
@@ -99,10 +99,13 @@ show_adapter(int ac, char **av)
        if (man0 == NULL) {
                error = errno;
                warn("Failed to get controller info");
+               close(fd);
                return (error);
        }
        if (man0->Header.PageLength < sizeof(*man0) / 4) {
                warnx("Invalid controller info");
+               free(man0);
+               close(fd);
                return (EINVAL);
        }
        printf("mpt%d Adapter:\n", mpt_unit);
@@ -307,11 +310,16 @@ show_config(int ac, char **av)
        if (ioc2 == NULL || ioc5 == NULL) {
                error = errno;
                warn("Failed to get config");
+               free(ioc2);
+               close(fd);
                return (error);
        }
        if (mpt_fetch_disks(fd, &nsdisks, &sdisks) < 0) {
                error = errno;
                warn("Failed to get standalone drive list");
+               free(ioc5);
+               free(ioc2);
+               close(fd);
                return (error);
        }
 
@@ -463,6 +471,7 @@ show_volumes(int ac, char **av)
                }
                printf("\n");
        }
+       free(volumes);
        free(ioc2);
        close(fd);
 
@@ -493,6 +502,7 @@ show_drives(int ac, char **av)
        list = mpt_pd_list(fd);
        if (list == NULL) {
                error = errno;
+               close(fd);
                warn("Failed to get drive list");
                return (error);
        }

Modified: head/usr.sbin/mptutil/mpt_volume.c
==============================================================================
--- head/usr.sbin/mptutil/mpt_volume.c  Fri Feb 23 00:12:51 2018        
(r329844)
+++ head/usr.sbin/mptutil/mpt_volume.c  Fri Feb 23 00:17:50 2018        
(r329845)
@@ -100,11 +100,14 @@ volume_name(int ac, char **av)
        if (vnames == NULL) {
                error = errno;
                warn("Failed to fetch volume names");
+               close(fd);
                return (error);
        }
 
        if (vnames->Header.PageType != MPI_CONFIG_PAGEATTR_CHANGEABLE) {
                warnx("Volume name is read only");
+               free(vnames);
+               close(fd);
                return (EOPNOTSUPP);
        }
        printf("mpt%u changing volume %s name from \"%s\" to \"%s\"\n",
@@ -116,6 +119,8 @@ volume_name(int ac, char **av)
        if (mpt_write_config_page(fd, vnames, NULL) < 0) {
                error = errno;
                warn("Failed to set volume name");
+               free(vnames);
+               close(fd);
                return (error);
        }
 
@@ -152,6 +157,7 @@ volume_status(int ac, char **av)
        error = mpt_lookup_volume(fd, av[1], &VolumeBus, &VolumeID);
        if (error) {
                warnc(error, "Invalid volume: %s", av[1]);
+               close(fd);
                return (error);
        }
 
@@ -160,6 +166,7 @@ volume_status(int ac, char **av)
            NULL, NULL, 0);
        if (error) {
                warnc(error, "Fetching volume status failed");
+               close(fd);
                return (error);
        }
 
@@ -226,12 +233,15 @@ volume_cache(int ac, char **av)
        error = mpt_lookup_volume(fd, av[1], &VolumeBus, &VolumeID);
        if (error) {
                warnc(error, "Invalid volume: %s", av[1]);
+               close(fd);
                return (error);
        }
 
        volume = mpt_vol_info(fd, VolumeBus, VolumeID, NULL);
-       if (volume == NULL)
+       if (volume == NULL) {
+               close(fd);
                return (errno);
+       }
 
        Settings = volume->VolumeSettings.Settings;
 
@@ -243,6 +253,7 @@ volume_cache(int ac, char **av)
 
        if (NewSettings == Settings) {
                warnx("volume cache unchanged");
+               free(volume);
                close(fd);
                return (0);
        }
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to