Date: Fri, 23 Feb 2018 00:17:50 +0000 (UTC) From: Alan Somers <asomers@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r329845 - head/usr.sbin/mptutil Message-ID: <201802230017.w1N0HoFf037170@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201802230017.w1N0HoFf037170>