Contrary to when returning in all-good cases at the end of functions we

did not free memory (1) or close the file descriptor (2) in error cases.

Reported by:	Mark Johnston (1)
Reported by:	attilio (2)
Reviewed by:	jhb
Sponsored by:	Sandvine Incorporated
MFC after:	1 week

7 files changed, 216 insertions, 62 deletions

diff --git a/usr.sbin/mfiutil/mfi_config.c b/usr.sbin/mfiutil/mfi_config.c
index fdda117..f1e6516 100644
--- a/usr.sbin/mfiutil/mfi_config.c
+++ b/usr.sbin/mfiutil/mfi_config.c
@@ -85,6 +85,7 @@ mfi_config_read(int fd, struct mfi_config_data **configp)
 {
 	struct mfi_config_data *config;
 	uint32_t config_size;
+	int error;
 
 	/*
 	 * Keep fetching the config in a loop until we have a large enough
@@ -97,8 +98,12 @@ fetch:
 	if (config == NULL)
 		return (-1);
 	if (mfi_dcmd_command(fd, MFI_DCMD_CFG_READ, config,
-	    config_size, NULL, 0, NULL) < 0)
+	    config_size, NULL, 0, NULL) < 0) {
+		error = errno;
+		free(config);
+		errno = error;
 		return (-1);
+	}
 
 	if (config->size > config_size) {
 		config_size = config->size;
@@ -162,12 +167,14 @@ clear_config(int ac, char **av)
 	if (!mfi_reconfig_supported()) {
 		warnx("The current mfi(4) driver does not support "
 		    "configuration changes.");
+		close(fd);
 		return (EOPNOTSUPP);
 	}
 
 	if (mfi_ld_get_list(fd, &list, NULL) < 0) {
 		error = errno;
 		warn("Failed to get volume list");
+		close(fd);
 		return (error);
 	}
 
@@ -175,6 +182,7 @@ clear_config(int ac, char **av)
 		if (mfi_volume_busy(fd, list.ld_list[i].ld.v.target_id)) {
 			warnx("Volume %s is busy and cannot be deleted",
 			    mfi_volume_name(fd, list.ld_list[i].ld.v.target_id));
+			close(fd);
 			return (EBUSY);
 		}
 	}
@@ -185,12 +193,14 @@ clear_config(int ac, char **av)
 	ch = getchar();
 	if (ch != 'y' && ch != 'Y') {
 		printf("\nAborting\n");
+		close(fd);
 		return (0);
 	}
 
 	if (mfi_dcmd_command(fd, MFI_DCMD_CFG_CLEAR, NULL, 0, NULL, 0, NULL) < 0) {
 		error = errno;
 		warn("Failed to clear configuration");
+		close(fd);
 		return (error);
 	}
 
@@ -336,17 +346,21 @@ parse_array(int fd, int raid_type, char *array_str, struct array_info *info)
 	for (pinfo = info->drives; (cp = strsep(&array_str, ",")) != NULL;
 	     pinfo++) {
 		error = mfi_lookup_drive(fd, cp, &device_id);
-		if (error)
+		if (error) {
+			free(info->drives);
 			return (error);
+		}
 
 		if (mfi_pd_get_info(fd, device_id, pinfo, NULL) < 0) {
 			error = errno;
 			warn("Failed to fetch drive info for drive %s", cp);
+			free(info->drives);
 			return (error);
 		}
 
 		if (pinfo->fw_state != MFI_PD_STATE_UNCONFIGURED_GOOD) {
 			warnx("Drive %u is not available", device_id);
+			free(info->drives);
 			return (EINVAL);
 		}
 	}
@@ -551,7 +565,12 @@ create_volume(int ac, char **av)
 		return (EINVAL);
 	}
 
-	
+	bzero(&state, sizeof(state));
+	config = NULL;
+	arrays = NULL;
+	narrays = 0;
+	error = 0;
+
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
 		error = errno;
@@ -562,7 +581,8 @@ create_volume(int ac, char **av)
 	if (!mfi_reconfig_supported()) {
 		warnx("The current mfi(4) driver does not support "
 		    "configuration changes.");
-		return (EOPNOTSUPP);
+		error = EOPNOTSUPP;
+		goto error;
 	}
 
 	/* Lookup the RAID type first. */
@@ -575,7 +595,8 @@ create_volume(int ac, char **av)
 
 	if (raid_type == -1) {
 		warnx("Unknown or unsupported volume type %s", av[1]);
-		return (EINVAL);
+		error = EINVAL;
+		goto error;
 	}
 
 	/* Parse any options. */
@@ -603,7 +624,8 @@ create_volume(int ac, char **av)
 			break;
 		case '?':
 		default:
-			return (EINVAL);
+			error = EINVAL;
+			goto error;
 		}
 	}
 	ac -= optind;
@@ -613,7 +635,8 @@ create_volume(int ac, char **av)
 	narrays = ac;
 	if (narrays == 0) {
 		warnx("At least one drive list is required");
-		return (EINVAL);
+		error = EINVAL;
+		goto error;
 	}
 	switch (raid_type) {
 	case RT_RAID0:
@@ -623,7 +646,8 @@ create_volume(int ac, char **av)
 	case RT_CONCAT:
 		if (narrays != 1) {
 			warnx("Only one drive list can be specified");
-			return (EINVAL);
+			error = EINVAL;
+			goto error;
 		}
 		break;
 	case RT_RAID10:
@@ -632,24 +656,27 @@ create_volume(int ac, char **av)
 		if (narrays < 1) {
 			warnx("RAID10, RAID50, and RAID60 require at least "
 			    "two drive lists");
-			return (EINVAL);
+			error = EINVAL;
+			goto error;
 		}
 		if (narrays > MFI_MAX_SPAN_DEPTH) {
 			warnx("Volume spans more than %d arrays",
 			    MFI_MAX_SPAN_DEPTH);
-			return (EINVAL);
+			error = EINVAL;
+			goto error;
 		}
 		break;
 	}
 	arrays = calloc(narrays, sizeof(*arrays));
 	if (arrays == NULL) {
 		warnx("malloc failed");
-		return (ENOMEM);
+		error = ENOMEM;
+		goto error;
 	}
 	for (i = 0; i < narrays; i++) {
 		error = parse_array(fd, raid_type, av[i], &arrays[i]);
 		if (error)
-			return (error);
+			goto error;
 	}
 
 	switch (raid_type) {
@@ -660,7 +687,8 @@ create_volume(int ac, char **av)
 			if (arrays[i].drive_count != arrays[0].drive_count) {
 				warnx("All arrays must contain the same "
 				    "number of drives");
-				return (EINVAL);
+				error = EINVAL;
+				goto error;
 			}
 		}
 		break;
@@ -673,7 +701,7 @@ create_volume(int ac, char **av)
 	if (mfi_config_read(fd, &config) < 0) {
 		error = errno;
 		warn("Failed to read configuration");
-		return (error);
+		goto error;
 	}
 	p = (char *)config->array;
 	state.array_ref = 0xffff;
@@ -683,7 +711,8 @@ create_volume(int ac, char **av)
 		state.arrays = calloc(config->array_count, sizeof(int));
 		if (state.arrays == NULL) {
 			warnx("malloc failed");
-			return (ENOMEM);
+			error = ENOMEM;
+			goto error;
 		}
 		for (i = 0; i < config->array_count; i++) {
 			ar = (struct mfi_array *)p;
@@ -699,7 +728,8 @@ create_volume(int ac, char **av)
 		state.volumes = calloc(config->log_drv_count, sizeof(int));
 		if (state.volumes == NULL) {
 			warnx("malloc failed");
-			return (ENOMEM);
+			error = ENOMEM;
+			goto error;
 		}
 		for (i = 0; i < config->log_drv_count; i++) {
 			ld = (struct mfi_ld_config *)p;
@@ -739,7 +769,8 @@ create_volume(int ac, char **av)
 	config = calloc(1, config_size);
 	if (config == NULL) {
 		warnx("malloc failed");
-		return (ENOMEM);
+		error = ENOMEM;
+		goto error;
 	}
 	config->size = config_size;
 	config->array_count = narrays;
@@ -776,21 +807,20 @@ create_volume(int ac, char **av)
 	    NULL, 0, NULL) < 0) {
 		error = errno;
 		warn("Failed to add volume");
-		return (error);
+		/* FALLTHROUGH */
 	}
 
+error:
 	/* Clean up. */
 	free(config);
-	if (state.log_drv_count > 0)
-		free(state.volumes);
-	if (state.array_count > 0)
-		free(state.arrays);
+	free(state.volumes);
+	free(state.arrays);
 	for (i = 0; i < narrays; i++)
 		free(arrays[i].drives);
 	free(arrays);
 	close(fd);
 
-	return (0);
+	return (error);
 }
 MFI_COMMAND(top, create, create_volume);
 
@@ -831,24 +861,28 @@ delete_volume(int ac, char **av)
 	if (!mfi_reconfig_supported()) {
 		warnx("The current mfi(4) driver does not support "
 		    "configuration changes.");
+		close(fd);
 		return (EOPNOTSUPP);
 	}
 
 	if (mfi_lookup_volume(fd, av[1], &target_id) < 0) {
 		error = errno;
 		warn("Invalid volume %s", av[1]);
+		close(fd);
 		return (error);
 	}
 
 	if (mfi_ld_get_info(fd, target_id, &info, NULL) < 0) {
 		error = errno;
 		warn("Failed to get info for volume %d", target_id);
+		close(fd);
 		return (error);
 	}
 
 	if (mfi_volume_busy(fd, target_id)) {
 		warnx("Volume %s is busy and cannot be deleted",
 		    mfi_volume_name(fd, target_id));
+		close(fd);
 		return (EBUSY);
 	}
 
@@ -857,6 +891,7 @@ delete_volume(int ac, char **av)
 	    sizeof(mbox), NULL) < 0) {
 		error = errno;
 		warn("Failed to delete volume");
+		close(fd);
 		return (error);
 	}
 
@@ -891,40 +926,44 @@ add_spare(int ac, char **av)
 		return (error);
 	}
 
+	config = NULL;
+	spare = NULL;
 	error = mfi_lookup_drive(fd, av[1], &device_id);
 	if (error)
-		return (error);
+		goto error;
 
 	if (mfi_pd_get_info(fd, device_id, &info, NULL) < 0) {
 		error = errno;
 		warn("Failed to fetch drive info");
-		return (error);
+		goto error;
 	}
 
 	if (info.fw_state != MFI_PD_STATE_UNCONFIGURED_GOOD) {
 		warnx("Drive %u is not available", device_id);
-		return (EINVAL);
+		error = EINVAL;
+		goto error;
 	}
 
 	if (ac > 2) {
 		if (mfi_lookup_volume(fd, av[2], &target_id) < 0) {
 			error = errno;
 			warn("Invalid volume %s", av[2]);
-			return (error);
+			goto error;
 		}
 	}
 
 	if (mfi_config_read(fd, &config) < 0) {
 		error = errno;
 		warn("Failed to read configuration");
-		return (error);
+		goto error;
 	}
 
 	spare = malloc(sizeof(struct mfi_spare) + sizeof(uint16_t) *
 	    config->array_count);
 	if (spare == NULL) {
 		warnx("malloc failed");
-		return (ENOMEM);
+		error = ENOMEM;
+		goto error;
 	}
 	bzero(spare, sizeof(struct mfi_spare));
 	spare->ref = info.ref;
@@ -937,7 +976,8 @@ add_spare(int ac, char **av)
 			if (ar->size > info.coerced_size) {
 				warnx("Spare isn't large enough for array %u",
 				    ar->array_ref);
-				return (EINVAL);
+				error = EINVAL;
+				goto error;
 			}
 			p += config->array_size;
 		}
@@ -950,7 +990,8 @@ add_spare(int ac, char **av)
 		ld = mfi_config_lookup_volume(config, target_id);
 		if (ld == NULL) {
 			warnx("Did not find volume %d", target_id);
-			return (EINVAL);
+			error = EINVAL;
+			goto error;
 		}
 
 		spare->spare_type |= MFI_SPARE_DEDICATED;
@@ -960,29 +1001,33 @@ add_spare(int ac, char **av)
 			    ld->span[i].array_ref);
 			if (ar == NULL) {
 				warnx("Missing array; inconsistent config?");
-				return (ENXIO);
+				error = ENXIO;
+				goto error;
 			}
 			if (ar->size > info.coerced_size) {
 				warnx("Spare isn't large enough for array %u",
 				    ar->array_ref);
-				return (EINVAL);
+				error = EINVAL;
+				goto error;
 			}				
 			spare->array_ref[i] = ar->array_ref;
 		}
 	}
-	free(config);
 
 	if (mfi_dcmd_command(fd, MFI_DCMD_CFG_MAKE_SPARE, spare,
 	    sizeof(struct mfi_spare) + sizeof(uint16_t) * spare->array_count,
 	    NULL, 0, NULL) < 0) {
 		error = errno;
 		warn("Failed to assign spare");
-		return (error);
+		/* FALLTHROUGH. */
 	}
 
+error:
+	free(spare);
+	free(config);
 	close(fd);
 
-	return (0);
+	return (error);
 }
 MFI_COMMAND(top, add, add_spare);
 
@@ -1007,18 +1052,22 @@ remove_spare(int ac, char **av)
 	}
 
 	error = mfi_lookup_drive(fd, av[1], &device_id);
-	if (error)
+	if (error) {
+		close(fd);
 		return (error);
+	}
 
 	/* Get the info for this drive. */
 	if (mfi_pd_get_info(fd, device_id, &info, NULL) < 0) {
 		error = errno;
 		warn("Failed to fetch info for drive %u", device_id);
+		close(fd);
 		return (error);
 	}
 
 	if (info.fw_state != MFI_PD_STATE_HOT_SPARE) {
 		warnx("Drive %u is not a hot spare", device_id);
+		close(fd);
 		return (EINVAL);
 	}
 
@@ -1027,6 +1076,7 @@ remove_spare(int ac, char **av)
 	    sizeof(mbox), NULL) < 0) {
 		error = errno;
 		warn("Failed to delete spare");
+		close(fd);
 		return (error);
 	}
 
@@ -1151,6 +1201,7 @@ debug_config(int ac, char **av)
 	if (mfi_config_read(fd, &config) < 0) {
 		error = errno;
 		warn("Failed to get config");
+		close(fd);
 		return (error);
 	}
 
@@ -1190,17 +1241,21 @@ dump(int ac, char **av)
 		warn("Failed to read debug command");
 		if (error == ENOENT)
 			error = EOPNOTSUPP;
+		close(fd);
 		return (error);
 	}
 
 	config = malloc(len);
 	if (config == NULL) {
 		warnx("malloc failed");
+		close(fd);
 		return (ENOMEM);
 	}
 	if (sysctlbyname(buf, config, &len, NULL, 0) < 0) {
 		error = errno;
 		warn("Failed to read debug command");
+		free(config);
+		close(fd);
 		return (error);
 	}
 	dump_config(fd, config);
diff --git a/usr.sbin/mfiutil/mfi_drive.c b/usr.sbin/mfiutil/mfi_drive.c
index 75c4a53..5c2ab5d 100644
--- a/usr.sbin/mfiutil/mfi_drive.c
+++ b/usr.sbin/mfiutil/mfi_drive.c
@@ -310,19 +310,23 @@ drive_set_state(char *drive, uint16_t new_state)
 	}
 
 	error = mfi_lookup_drive(fd, drive, &device_id);
-	if (error)
+	if (error) {
+		close(fd);
 		return (error);
+	}
 
 	/* Get the info for this drive. */
 	if (mfi_pd_get_info(fd, device_id, &info, NULL) < 0) {
 		error = errno;
 		warn("Failed to fetch info for drive %u", device_id);
+		close(fd);
 		return (error);
 	}
 
 	/* Try to change the state. */
 	if (info.fw_state == new_state) {
 		warnx("Drive %u is already in the desired state", device_id);
+		close(fd);
 		return (EINVAL);
 	}
 
@@ -334,6 +338,7 @@ drive_set_state(char *drive, uint16_t new_state)
 		error = errno;
 		warn("Failed to set drive %u to %s", device_id,
 		    mfi_pdstate(new_state));
+		close(fd);
 		return (error);
 	}
 
@@ -406,19 +411,23 @@ start_rebuild(int ac, char **av)
 	}
 
 	error = mfi_lookup_drive(fd, av[1], &device_id);
-	if (error)
+	if (error) {
+		close(fd);
 		return (error);
+	}
 
 	/* Get the info for this drive. */
 	if (mfi_pd_get_info(fd, device_id, &info, NULL) < 0) {
 		error = errno;
 		warn("Failed to fetch info for drive %u", device_id);
+		close(fd);
 		return (error);
 	}
 
 	/* Check the state, must be REBUILD. */
 	if (info.fw_state != MFI_PD_STATE_REBUILD) {
 		warnx("Drive %d is not in the REBUILD state", device_id);
+		close(fd);
 		return (EINVAL);
 	}
 
@@ -428,6 +437,7 @@ start_rebuild(int ac, char **av)
 	    NULL) < 0) {
 		error = errno;
 		warn("Failed to start rebuild on drive %u", device_id);
+		close(fd);
 		return (error);
 	}
 	close(fd);
@@ -458,19 +468,23 @@ abort_rebuild(int ac, char **av)
 	}
 
 	error = mfi_lookup_drive(fd, av[1], &device_id);
-	if (error)
+	if (error) {
+		close(fd);
 		return (error);
+	}
 
 	/* Get the info for this drive. */
 	if (mfi_pd_get_info(fd, device_id, &info, NULL) < 0) {
 		error = errno;
 		warn("Failed to fetch info for drive %u", device_id);
+		close(fd);
 		return (error);
 	}
 
 	/* Check the state, must be REBUILD. */
 	if (info.fw_state != MFI_PD_STATE_REBUILD) {
 		warn("Drive %d is not in the REBUILD state", device_id);
+		close(fd);
 		return (EINVAL);
 	}
 
@@ -480,6 +494,7 @@ abort_rebuild(int ac, char **av)
 	    NULL) < 0) {
 		error = errno;
 		warn("Failed to abort rebuild on drive %u", device_id);
+		close(fd);
 		return (error);
 	}
 	close(fd);
@@ -509,13 +524,16 @@ drive_progress(int ac, char **av)
 	}
 
 	error = mfi_lookup_drive(fd, av[1], &device_id);
-	if (error)
+	if (error) {
+		close(fd);
 		return (error);
+	}
 
 	/* Get the info for this drive. */
 	if (mfi_pd_get_info(fd, device_id, &info, NULL) < 0) {
 		error = errno;
 		warn("Failed to fetch info for drive %u", device_id);
+		close(fd);
 		return (error);
 	}
 	close(fd);
@@ -570,13 +588,16 @@ drive_clear(int ac, char **av)
 	}
 
 	error = mfi_lookup_drive(fd, av[1], &device_id);
-	if (error)
+	if (error) {
+		close(fd);
 		return (error);
+	}
 
 	/* Get the info for this drive. */
 	if (mfi_pd_get_info(fd, device_id, &info, NULL) < 0) {
 		error = errno;
 		warn("Failed to fetch info for drive %u", device_id);
+		close(fd);
 		return (error);
 	}
 
@@ -586,6 +607,7 @@ drive_clear(int ac, char **av)
 		warn("Failed to %s clear on drive %u",
 		    opcode == MFI_DCMD_PD_CLEAR_START ? "start" : "stop",
 		    device_id);
+		close(fd);
 		return (error);
 	}
 
@@ -626,8 +648,10 @@ drive_locate(int ac, char **av)
 	}
 
 	error = mfi_lookup_drive(fd, av[1], &device_id);
-	if (error)
+	if (error) {
+		close(fd);
 		return (error);
+	}
 
 
 	mbox_store_device_id(&mbox[0], device_id);
@@ -638,6 +662,7 @@ drive_locate(int ac, char **av)
 		warn("Failed to %s locate on drive %u",
 		    opcode == MFI_DCMD_PD_LOCATE_START ? "start" : "stop",
 		    device_id);
+		close(fd);
 		return (error);
 	}
 	close(fd);
diff --git a/usr.sbin/mfiutil/mfi_evt.c b/usr.sbin/mfiutil/mfi_evt.c
index a8a8775..336fbd3 100644
--- a/usr.sbin/mfiutil/mfi_evt.c
+++ b/usr.sbin/mfiutil/mfi_evt.c
@@ -83,6 +83,7 @@ show_logstate(int ac, char **av)
 	if (mfi_event_get_info(fd, &info, NULL) < 0) {
 		error = errno;
 		warn("Failed to get event log info");
+		close(fd);
 		return (error);
 	}
 
@@ -550,6 +551,7 @@ show_events(int ac, char **av)
 	if (mfi_event_get_info(fd, &info, NULL) < 0) {
 		error = errno;
 		warn("Failed to get event log info");
+		close(fd);
 		return (error);
 	}
 
@@ -570,6 +572,7 @@ show_events(int ac, char **av)
 			if (parse_class(optarg, &filter.members.evt_class) < 0) {
 				error = errno;
 				warn("Error parsing event class");
+				close(fd);
 				return (error);
 			}
 			break;
@@ -577,6 +580,7 @@ show_events(int ac, char **av)
 			if (parse_locale(optarg, &filter.members.locale) < 0) {
 				error = errno;
 				warn("Error parsing event locale");
+				close(fd);
 				return (error);
 			}
 			break;
@@ -584,6 +588,7 @@ show_events(int ac, char **av)
 			val = strtol(optarg, &cp, 0);
 			if (*cp != '\0' || val <= 0) {
 				warnx("Invalid event count");
+				close(fd);
 				return (EINVAL);
 			}
 			num_events = val;
@@ -593,6 +598,7 @@ show_events(int ac, char **av)
 			break;
 		case '?':
 		default:
+			close(fd);
 			return (EINVAL);
 		}
 	}
@@ -604,28 +610,33 @@ show_events(int ac, char **av)
 	    (num_events - 1);
 	if (size > getpagesize()) {
 		warnx("Event count is too high");
+		close(fd);
 		return (EINVAL);
 	}
 
 	/* Handle optional start and stop sequence numbers. */
 	if (ac > 2) {
 		warnx("show events: extra arguments");
+		close(fd);
 		return (EINVAL);
 	}
 	if (ac > 0 && parse_seq(&info, av[0], &start) < 0) {
 		error = errno;
 		warn("Error parsing starting sequence number");
+		close(fd);
 		return (error);
 	}
 	if (ac > 1 && parse_seq(&info, av[1], &stop) < 0) {
 		error = errno;
 		warn("Error parsing ending sequence number");
+		close(fd);
 		return (error);
 	}
 
 	list = malloc(size);
 	if (list == NULL) {
 		warnx("malloc failed");
+		close(fd);
 		return (ENOMEM);
 	}
 	for (seq = start;;) {
@@ -633,6 +644,8 @@ show_events(int ac, char **av)
 		    &status) < 0) {
 			error = errno;
 			warn("Failed to fetch events");
+			free(list);
+			close(fd);
 			return (error);
 		}
 		if (status == MFI_STAT_NOT_FOUND) {
@@ -642,6 +655,8 @@ show_events(int ac, char **av)
 		}
 		if (status != MFI_STAT_OK) {
 			warnx("Error fetching events: %s", mfi_status(status));
+			free(list);
+			close(fd);
 			return (EIO);
 		}
 
diff --git a/usr.sbin/mfiutil/mfi_flash.c b/usr.sbin/mfiutil/mfi_flash.c
index 4039beb..6d07cb0 100644
--- a/usr.sbin/mfiutil/mfi_flash.c
+++ b/usr.sbin/mfiutil/mfi_flash.c
@@ -136,21 +136,25 @@ flash_adapter(int ac, char **av)
 		return (error);
 	}
 
+	buf = NULL;
+	fd = -1;
+
 	if (fstat(flash, &sb) < 0) {
 		error = errno;
 		warn("fstat(%s)", av[1]);
-		return (error);
+		goto error;
 	}
 	if (sb.st_size % 1024 != 0 || sb.st_size > 0x7fffffff) {
 		warnx("Invalid flash file size");
-		return (EINVAL);
+		error = EINVAL;
+		goto error;
 	}
 
 	fd = mfi_open(mfi_unit);
 	if (fd < 0) {
 		error = errno;
 		warn("mfi_open");
-		return (error);
+		goto error;
 	}
 
 	/* First, ask the firmware to allocate space for the flash file. */
@@ -158,14 +162,16 @@ flash_adapter(int ac, char **av)
 	mfi_dcmd_command(fd, MFI_DCMD_FLASH_FW_OPEN, NULL, 0, mbox, 4, &status);
 	if (status != MFI_STAT_OK) {
 		warnx("Failed to alloc flash memory: %s", mfi_status(status));
-		return (EIO);
+		error = EIO;
+		goto error;
 	}
 
 	/* Upload the file 64k at a time. */
 	buf = malloc(FLASH_BUF_SIZE);
 	if (buf == NULL) {
 		warnx("malloc failed");
-		return (ENOMEM);
+		error = ENOMEM;
+		goto error;
 	}
 	offset = 0;
 	while (sb.st_size > 0) {
@@ -174,7 +180,8 @@ flash_adapter(int ac, char **av)
 			warnx("Bad read from flash file");
 			mfi_dcmd_command(fd, MFI_DCMD_FLASH_FW_CLOSE, NULL, 0,
 			    NULL, 0, NULL);
-			return (ENXIO);
+			error = ENXIO;
+			goto error;
 		}
 
 		mbox_store_word(mbox, offset);
@@ -184,12 +191,12 @@ flash_adapter(int ac, char **av)
 			warnx("Flash download failed: %s", mfi_status(status));
 			mfi_dcmd_command(fd, MFI_DCMD_FLASH_FW_CLOSE, NULL, 0,
 			    NULL, 0, NULL);
-			return (ENXIO);
+			error = ENXIO;
+			goto error;
 		}
 		sb.st_size -= nread;
 		offset += nread;
 	}
-	close(flash);
 
 	/* Kick off the flash. */
 	printf("WARNING: Firmware flash in progress, do not reboot machine... ");
@@ -198,12 +205,17 @@ flash_adapter(int ac, char **av)
 	    NULL, 0, &status);
 	if (status != MFI_STAT_OK) {
 		printf("failed:\n\t%s\n", mfi_status(status));
-		return (ENXIO);
+		error = ENXIO;
+		goto error;
 	}
 	printf("finished\n");
 	error = display_pending_firmware(fd);
 
-	close(fd);
+error:
+	free(buf);
+	if (fd >= 0)
+		close(fd);
+	close(flash);
 
 	return (error);
 }
diff --git a/usr.sbin/mfiutil/mfi_patrol.c b/usr.sbin/mfiutil/mfi_patrol.c
index da7ddb5..d70c4ab 100644
--- a/usr.sbin/mfiutil/mfi_patrol.c
+++ b/usr.sbin/mfiutil/mfi_patrol.c
@@ -96,8 +96,10 @@ show_patrol(int ac, char **av)
 	time(&now);
 	mfi_get_time(fd, &at);
 	error = patrol_get_props(fd, &prop);
-	if (error)
+	if (error) {
+		close(fd);
 		return (error);
+	}
 	printf("Operation Mode: ");
 	switch (prop.op_mode) {
 	case MFI_PR_OPMODE_AUTO:
@@ -128,6 +130,7 @@ show_patrol(int ac, char **av)
 	    sizeof(status), NULL, 0, NULL) < 0) {
 		error = errno;
 		warn("Failed to get patrol read properties");
+		close(fd);
 		return (error);
 	}
 	printf("Runs Completed: %u\n", status.num_iteration);
@@ -153,6 +156,7 @@ show_patrol(int ac, char **av)
 		if (mfi_pd_get_list(fd, &list, NULL) < 0) {
 			error = errno;
 			warn("Failed to get drive list");
+			close(fd);
 			return (error);
 		}
 
@@ -165,6 +169,8 @@ show_patrol(int ac, char **av)
 				error = errno;
 				warn("Failed to fetch info for drive %u",
 				    list->addr[i].device_id);
+				free(list);
+				close(fd);
 				return (error);
 			}
 			if (info.prog_info.active & MFI_PD_PROGRESS_PATROL) {
@@ -174,6 +180,7 @@ show_patrol(int ac, char **av)
 				    &info.prog_info.patrol);
 			}
 		}
+		free(list);
 	}
 
 	close(fd);
@@ -198,6 +205,7 @@ start_patrol(int ac, char **av)
 	    0) {
 		error = errno;
 		warn("Failed to start patrol read");
+		close(fd);
 		return (error);
 	}
 
@@ -223,6 +231,7 @@ stop_patrol(int ac, char **av)
 	    0) {
 		error = errno;
 		warn("Failed to stop patrol read");
+		close(fd);
 		return (error);
 	}
 
@@ -289,8 +298,10 @@ patrol_config(int ac, char **av)
 	}
 
 	error = patrol_get_props(fd, &prop);
-	if (error)
+	if (error) {
+		close(fd);
 		return (error);
+	}
 	prop.op_mode = op_mode;
 	if (op_mode == MFI_PR_OPMODE_AUTO) {
 		if (ac > 2)
@@ -298,8 +309,10 @@ patrol_config(int ac, char **av)
 		if (ac > 3) {
 			time(&now);
 			mfi_get_time(fd, &at);
-			if (at == 0)
+			if (at == 0) {
+				close(fd);
 				return (ENXIO);
+			}
 			prop.next_exec = at + next_exec;
 			printf("Starting next patrol read at %s",
 			    adapter_time(now, at, prop.next_exec));
@@ -309,6 +322,7 @@ patrol_config(int ac, char **av)
 	    sizeof(prop), NULL, 0, NULL) < 0) {
 		error = errno;
 		warn("Failed to set patrol read properties");
+		close(fd);
 		return (error);
 	}
 
diff --git a/usr.sbin/mfiutil/mfi_show.c b/usr.sbin/mfiutil/mfi_show.c
index 22a735d..76fd6b9 100644
--- a/usr.sbin/mfiutil/mfi_show.c
+++ b/usr.sbin/mfiutil/mfi_show.c
@@ -71,6 +71,7 @@ show_adapter(int ac, char **av)
 	if (mfi_ctrl_get_info(fd, &info, NULL) < 0) {
 		error = errno;
 		warn("Failed to get controller info");
+		close(fd);
 		return (error);
 	}
 	printf("mfi%d Adapter:\n", mfi_unit);
@@ -158,10 +159,12 @@ show_battery(int ac, char **av)
 	    sizeof(cap), NULL, 0, &status) < 0) {
 		if (status == MFI_STAT_NO_HW_PRESENT) {
 			printf("mfi%d: No battery present\n", mfi_unit);
+			close(fd);
 			return (0);
 		}
 		error = errno;
 		warn("Failed to get capacity info");
+		close(fd);
 		return (error);
 	}
 
@@ -169,6 +172,7 @@ show_battery(int ac, char **av)
 	    sizeof(design), NULL, 0, NULL) < 0) {
 		error = errno;
 		warn("Failed to get design info");
+		close(fd);
 		return (error);
 	}
 
@@ -176,6 +180,7 @@ show_battery(int ac, char **av)
 	    NULL, 0, NULL) < 0) {
 		error = errno;
 		warn("Failed to get status");
+		close(fd);
 		return (error);
 	}
 
@@ -308,6 +313,7 @@ show_config(int ac, char **av)
 	if (mfi_config_read(fd, &config) < 0) {
 		error = errno;
 		warn("Failed to get config");
+		close(fd);
 		return (error);
 	}
 
@@ -376,6 +382,7 @@ show_config(int ac, char **av)
 			printf("\n");
 		p += config->spares_size;
 	}
+	free(config);
 	close(fd);
 
 	return (0);
@@ -406,6 +413,7 @@ show_volumes(int ac, char **av)
 	if (mfi_ld_get_list(fd, &list, NULL) < 0) {
 		error = errno;
 		warn("Failed to get volume list");
+		close(fd);
 		return (error);
 	}
 
@@ -431,6 +439,7 @@ show_volumes(int ac, char **av)
 			error = errno;
 			warn("Failed to get info for volume %d",
 			    list.ld_list[i].ld.v.target_id);
+			close(fd);
 			return (error);
 		}
 		printf("%6s ",
@@ -483,10 +492,11 @@ show_drives(int ac, char **av)
 		return (error);
 	}
 
+	list = NULL;
 	if (mfi_pd_get_list(fd, &list, NULL) < 0) {
 		error = errno;
 		warn("Failed to get drive list");
-		return (error);
+		goto error;
 	}
 
 	/* Walk the list of drives to determine width of state column. */
@@ -500,7 +510,7 @@ show_drives(int ac, char **av)
 			error = errno;
 			warn("Failed to fetch info for drive %u",
 			    list->addr[i].device_id);
-			return (error);
+			goto error;
 		}
 		len = strlen(mfi_pdstate(info.fw_state));
 		if (len > state_len)
@@ -521,15 +531,17 @@ show_drives(int ac, char **av)
 			error = errno;
 			warn("Failed to fetch info for drive %u",
 			    list->addr[i].device_id);
-			return (error);
+			goto error;
 		}
 
 		print_pd(&info, state_len, 1);
 		printf("\n");
 	}
+error:
+	free(list);
 	close(fd);
 
-	return (0);
+	return (error);
 }
 MFI_COMMAND(show, drives, show_drives);
 
@@ -586,6 +598,7 @@ show_firmware(int ac, char **av)
 	if (mfi_ctrl_get_info(fd, &info, NULL) < 0) {
 		error = errno;
 		warn("Failed to get controller info");
+		close(fd);
 		return (error);
 	}
 
@@ -627,7 +640,6 @@ show_progress(int ac, char **av)
 	struct mfi_pd_info pinfo;
 	int busy, error, fd;
 	u_int i;
-	
 	uint16_t device_id;
 	uint8_t target_id;
 
@@ -642,25 +654,29 @@ show_progress(int ac, char **av)
 		warn("mfi_open");
 		return (error);
 	}
-	busy = 0;
 
 	if (mfi_ld_get_list(fd, &llist, NULL) < 0) {
 		error = errno;
 		warn("Failed to get volume list");
+		close(fd);
 		return (error);
 	}
 	if (mfi_pd_get_list(fd, &plist, NULL) < 0) {
 		error = errno;
 		warn("Failed to get drive list");
+		close(fd);
 		return (error);
 	}
 
+	busy = 0;
 	for (i = 0; i < llist.ld_count; i++) {
 		target_id = llist.ld_list[i].ld.v.target_id;
 		if (mfi_ld_get_info(fd, target_id, &linfo, NULL) < 0) {
 			error = errno;
 			warn("Failed to get info for volume %s",
 			    mfi_volume_name(fd, target_id));
+			free(plist);
+			close(fd);
 			return (error);
 		}
 		if (linfo.progress.active & MFI_LD_PROGRESS_CC) {
@@ -697,6 +713,8 @@ show_progress(int ac, char **av)
 		if (mfi_pd_get_info(fd, device_id, &pinfo, NULL) < 0) {
 			error = errno;
 			warn("Failed to fetch info for drive %u", device_id);
+			free(plist);
+			close(fd);
 			return (error);
 		}
 
@@ -718,6 +736,7 @@ show_progress(int ac, char **av)
 		}
 	}
 
+	free(plist);
 	close(fd);
 
 	if (!busy)
diff --git a/usr.sbin/mfiutil/mfi_volume.c b/usr.sbin/mfiutil/mfi_volume.c
index 1e679c4..0d9300a 100644
--- a/usr.sbin/mfiutil/mfi_volume.c
+++ b/usr.sbin/mfiutil/mfi_volume.c
@@ -174,12 +174,14 @@ volume_cache(int ac, char **av)
 	if (mfi_lookup_volume(fd, av[1], &target_id) < 0) {
 		error = errno;
 		warn("Invalid volume: %s", av[1]);
+		close(fd);
 		return (error);
 	}
 
 	if (mfi_ld_get_props(fd, target_id, &props) < 0) {
 		error = errno;
 		warn("Failed to fetch volume properties");
+		close(fd);
 		return (error);
 	}
 
@@ -264,6 +266,7 @@ volume_cache(int ac, char **av)
 		else if (strcmp(av[2], "read-ahead") == 0) {
 			if (ac < 4) {
 				warnx("cache: read-ahead setting required");
+				close(fd);
 				return (EINVAL);
 			}
 			if (strcmp(av[3], "none") == 0)
@@ -275,6 +278,7 @@ volume_cache(int ac, char **av)
 				    MR_LD_CACHE_READ_ADAPTIVE;
 			else {
 				warnx("cache: invalid read-ahead setting");
+				close(fd);
 				return (EINVAL);
 			}
 			error = update_cache_policy(fd, &props, policy,
@@ -283,6 +287,7 @@ volume_cache(int ac, char **av)
 		} else if (strcmp(av[2], "bad-bbu-write-cache") == 0) {
 			if (ac < 4) {
 				warnx("cache: bad BBU setting required");
+				close(fd);
 				return (EINVAL);
 			}
 			if (strcmp(av[3], "enable") == 0)
@@ -291,6 +296,7 @@ volume_cache(int ac, char **av)
 				policy = 0;
 			else {
 				warnx("cache: invalid bad BBU setting");
+				close(fd);
 				return (EINVAL);
 			}
 			error = update_cache_policy(fd, &props, policy,
@@ -298,6 +304,7 @@ volume_cache(int ac, char **av)
 		} else if (strcmp(av[2], "write-cache") == 0) {
 			if (ac < 4) {
 				warnx("cache: write-cache setting required");
+				close(fd);
 				return (EINVAL);
 			}
 			if (strcmp(av[3], "enable") == 0)
@@ -308,6 +315,7 @@ volume_cache(int ac, char **av)
 				policy = MR_PD_CACHE_UNCHANGED;
 			else {
 				warnx("cache: invalid write-cache setting");
+				close(fd);
 				return (EINVAL);
 			}
 			error = 0;
@@ -331,6 +339,7 @@ volume_cache(int ac, char **av)
 			}
 		} else {
 			warnx("cache: Invalid command");
+			close(fd);
 			return (EINVAL);
 		}
 	}
@@ -367,12 +376,14 @@ volume_name(int ac, char **av)
 	if (mfi_lookup_volume(fd, av[1], &target_id) < 0) {
 		error = errno;
 		warn("Invalid volume: %s", av[1]);
+		close(fd);
 		return (error);
 	}
 
 	if (mfi_ld_get_props(fd, target_id, &props) < 0) {
 		error = errno;
 		warn("Failed to fetch volume properties");
+		close(fd);
 		return (error);
 	}
 
@@ -383,6 +394,7 @@ volume_name(int ac, char **av)
 	if (mfi_ld_set_props(fd, &props) < 0) {
 		error = errno;
 		warn("Failed to set volume properties");
+		close(fd);
 		return (error);
 	}
 
@@ -415,6 +427,7 @@ volume_progress(int ac, char **av)
 	if (mfi_lookup_volume(fd, av[1], &target_id) < 0) {
 		error = errno;
 		warn("Invalid volume: %s", av[1]);
+		close(fd);
 		return (error);
 	}
 
@@ -423,6 +436,7 @@ volume_progress(int ac, char **av)
 		error = errno;
 		warn("Failed to fetch info for volume %s",
 		    mfi_volume_name(fd, target_id));
+		close(fd);
 		return (error);
 	}