From: Hannes Reinecke Date: Fri, 22 Apr 2022 11:04:37 +0000 (+0200) Subject: plugins/wdc: coverity fixes X-Git-Tag: v2.1-rc0~61^2~13 X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=db3034bb764daba4ebc19de318552b190a1bbbf9;p=users%2Fsagi%2Fnvme-cli.git plugins/wdc: coverity fixes Coverity found an off-by-one error when printing raw values; looks like someone forgot convert the 1-based NVMe spec convention into the 0-based C convention. And, of course, various resource leaks. Signed-off-by: Hannes Reinecke --- diff --git a/plugins/wdc/wdc-nvme.c b/plugins/wdc/wdc-nvme.c index b9f7d3fd..f6654a05 100644 --- a/plugins/wdc/wdc-nvme.c +++ b/plugins/wdc/wdc-nvme.c @@ -1271,7 +1271,7 @@ static int wdc_get_pci_ids(nvme_root_t r, uint32_t *device_id, fprintf(stderr, "%s: Read of pci vendor id failed\n", __func__); return -1; } - + id[ret < 32 ? ret : 31] = '\0'; if (id[strlen(id) - 1] == '\n') id[strlen(id) - 1] = '\0'; @@ -1291,7 +1291,7 @@ static int wdc_get_pci_ids(nvme_root_t r, uint32_t *device_id, fprintf(stderr, "%s: Read of pci device id failed\n", __func__); return -1; } - + id[ret < 32 ? ret : 31] = '\0'; if (id[strlen(id) - 1] == '\n') id[strlen(id) - 1] = '\0'; @@ -1744,6 +1744,7 @@ static int wdc_create_log_file(char *file, __u8 *drive_log_data, ret = write(fd, drive_log_data, WRITE_SIZE); if (ret < 0) { fprintf (stderr, "ERROR : WDC: write : %s\n", strerror(errno)); + close(fd); return -1; } drive_log_data += WRITE_SIZE; @@ -1753,11 +1754,13 @@ static int wdc_create_log_file(char *file, __u8 *drive_log_data, ret = write(fd, drive_log_data, drive_log_length); if (ret < 0) { fprintf(stderr, "ERROR : WDC : write : %s\n", strerror(errno)); + close(fd); return -1; } if (fsync(fd) < 0) { fprintf(stderr, "ERROR : WDC : fsync : %s\n", strerror(errno)); + close(fd); return -1; } close(fd); @@ -2425,7 +2428,7 @@ static int wdc_do_cap_telemetry_log(int fd, char *file, __u32 bs, int type, int if (fsync(output) < 0) { fprintf(stderr, "ERROR : %s: fsync : %s\n", __func__, strerror(errno)); - return -1; + err = -1; } free(log); @@ -2873,7 +2876,7 @@ static int wdc_cap_diag(int argc, char **argv, struct command *command, char *size = "Data retrieval transfer size."; char f[PATH_MAX] = {0}; __u32 xfer_size = 0; - int fd; + int fd, ret = 0; __u64 capabilities = 0; struct config { @@ -2892,30 +2895,40 @@ static int wdc_cap_diag(int argc, char **argv, struct command *command, OPT_END() }; - r = nvme_scan(NULL); - fd = parse_and_open(argc, argv, desc, opts); if (fd < 0) return fd; + r = nvme_scan(NULL); + if (cfg.file != NULL) strncpy(f, cfg.file, PATH_MAX - 1); if (cfg.xfer_size != 0) xfer_size = cfg.xfer_size; - if (wdc_get_serial_name(fd, f, PATH_MAX, "cap_diag") == -1) { + ret = wdc_get_serial_name(fd, f, PATH_MAX, "cap_diag"); + if (ret) { fprintf(stderr, "ERROR : WDC: failed to generate file name\n"); - return -1; + goto out; + } + if (cfg.file == NULL) { + if (strlen(f) > PATH_MAX - 5) { + fprintf(stderr, "ERROR : WDC: file name overflow\n"); + ret = -1; + goto out; + } + strcat(f, ".bin"); } - if (cfg.file == NULL) - snprintf(f + strlen(f), PATH_MAX, "%s", ".bin"); capabilities = wdc_get_drive_capabilities(r, fd); if ((capabilities & WDC_DRIVE_CAP_CAP_DIAG) == WDC_DRIVE_CAP_CAP_DIAG) - return wdc_do_cap_diag(r, fd, f, xfer_size, 0, 0); - - fprintf(stderr, "ERROR : WDC: unsupported device for this command\n"); + ret = wdc_do_cap_diag(r, fd, f, xfer_size, 0, 0); + else + fprintf(stderr, + "ERROR : WDC: unsupported device for this command\n"); +out: nvme_free_tree(r); - return 0; + close(fd); + return ret; } static int wdc_do_get_sn730_log_len(int fd, uint32_t *len_buf, uint32_t subopcode) @@ -3163,7 +3176,7 @@ static int wdc_vs_internal_fw_log(int argc, char **argv, struct command *command char fileSuffix[PATH_MAX] = {0}; nvme_root_t r; __u32 xfer_size = 0; - int fd; + int fd, ret = -1; int telemetry_type = 0, telemetry_data_area = 0; UtilsTimeInfo timeInfo; __u8 timeStamp[MAX_PATH_LEN]; @@ -3205,16 +3218,14 @@ static int wdc_vs_internal_fw_log(int argc, char **argv, struct command *command return fd; r = nvme_scan(NULL); - if (!wdc_check_device(r, fd)) { - nvme_free_tree(r); - return -1; - } + if (!wdc_check_device(r, fd)) + goto out; + if (cfg.xfer_size != 0) xfer_size = cfg.xfer_size; else { fprintf(stderr, "ERROR : WDC : Invalid length\n"); - nvme_free_tree(r); - return -1; + goto out; } if (cfg.file != NULL) { @@ -3224,7 +3235,7 @@ static int wdc_vs_internal_fw_log(int argc, char **argv, struct command *command verify_file = open(cfg.file, O_WRONLY | O_CREAT | O_TRUNC, 0666); if (verify_file < 0) { fprintf(stderr, "ERROR : WDC: open : %s\n", strerror(errno)); - return -1; + goto out; } close(verify_file); strncpy(f, cfg.file, PATH_MAX - 1); @@ -3236,20 +3247,28 @@ static int wdc_vs_internal_fw_log(int argc, char **argv, struct command *command timeInfo.hour, timeInfo.minute, timeInfo.second); snprintf(fileSuffix, PATH_MAX, "_internal_fw_log_%s", (char*)timeStamp); - if (wdc_get_serial_name(fd, f, PATH_MAX, fileSuffix) == -1) { + ret = wdc_get_serial_name(fd, f, PATH_MAX, fileSuffix); + if (ret) { fprintf(stderr, "ERROR : WDC: failed to generate file name\n"); - return -1; + goto out; } } - if (cfg.file == NULL) - snprintf(f + strlen(f), PATH_MAX, "%s", ".bin"); + if (cfg.file == NULL) { + if (strlen(f) > PATH_MAX - 5) { + fprintf(stderr, "ERROR : WDC: file name overflow\n"); + ret = -1; + goto out; + } + strcat(f, ".bin"); + } fprintf(stderr, "%s: filename = %s\n", __func__, f); if (cfg.data_area) { if (cfg.data_area > 5 || cfg.data_area < 1) { fprintf(stderr, "ERROR : WDC: Data area must be 1-5\n"); - return -1; + ret = -1; + goto out; } } @@ -3268,7 +3287,8 @@ static int wdc_vs_internal_fw_log(int argc, char **argv, struct command *command telemetry_data_area = cfg.data_area; } else { fprintf(stderr, "ERROR : WDC: Invalid type - Must be NONE, HOST or CONTROLLER\n"); - return -1; + ret = -1; + goto out; } capabilities = wdc_get_drive_capabilities(r, fd); @@ -3276,8 +3296,9 @@ static int wdc_vs_internal_fw_log(int argc, char **argv, struct command *command if (telemetry_data_area == 0) telemetry_data_area = 3; /* Set the default DA to 3 if not specified */ - return wdc_do_cap_diag(r, fd, f, xfer_size, - telemetry_type, telemetry_data_area); + ret = wdc_do_cap_diag(r, fd, f, xfer_size, + telemetry_type, telemetry_data_area); + goto out; } if ((capabilities & WDC_DRIVE_CAP_DUI) == WDC_DRIVE_CAP_DUI) { if ((telemetry_type == WDC_TELEMETRY_TYPE_HOST) || @@ -3285,18 +3306,20 @@ static int wdc_vs_internal_fw_log(int argc, char **argv, struct command *command if (telemetry_data_area == 0) telemetry_data_area = 3; /* Set the default DA to 3 if not specified */ /* Get the desired telemetry log page */ - return wdc_do_cap_telemetry_log(fd, f, xfer_size, telemetry_type, telemetry_data_area); - } - else { - if (cfg.data_area == 0) { + ret = wdc_do_cap_telemetry_log(fd, f, xfer_size, + telemetry_type, telemetry_data_area); + goto out; + } else { + if (cfg.data_area == 0) cfg.data_area = 1; - } /* FW requirement - xfer size must be 256k for data area 4 */ if (cfg.data_area >= 4) xfer_size = 0x40000; - return wdc_do_cap_dui(fd, f, xfer_size, cfg.data_area, - cfg.verbose, cfg.file_size, cfg.offset); + ret = wdc_do_cap_dui(fd, f, xfer_size, cfg.data_area, + cfg.verbose, cfg.file_size, + cfg.offset); + goto out; } } if ((capabilities & WDC_DRIVE_CAP_DUI_DATA) == WDC_DRIVE_CAP_DUI_DATA){ @@ -3305,17 +3328,26 @@ static int wdc_vs_internal_fw_log(int argc, char **argv, struct command *command if (telemetry_data_area == 0) telemetry_data_area = 3; /* Set the default DA to 3 if not specified */ /* Get the desired telemetry log page */ - return wdc_do_cap_telemetry_log(fd, f, xfer_size, telemetry_type, telemetry_data_area); - } - else { - return wdc_do_cap_dui(fd, f, xfer_size, WDC_NVME_DUI_MAX_DATA_AREA, cfg.verbose, 0, 0); + ret = wdc_do_cap_telemetry_log(fd, f, xfer_size, + telemetry_type, telemetry_data_area); + goto out; + } else { + ret = wdc_do_cap_dui(fd, f, xfer_size, + WDC_NVME_DUI_MAX_DATA_AREA, + cfg.verbose, 0, 0); + goto out; } } if ((capabilities & WDC_SN730B_CAP_VUC_LOG) == WDC_SN730B_CAP_VUC_LOG) - return wdc_do_sn730_get_and_tar(fd, f); - - fprintf(stderr, "ERROR : WDC: unsupported device for this command\n"); - return -1; + ret = wdc_do_sn730_get_and_tar(fd, f); + else { + fprintf(stderr, "ERROR : WDC: unsupported device for this command\n"); + ret = -1; + } +out: + nvme_free_tree(r); + close(fd); + return ret; } static int wdc_do_crash_dump(int fd, char *file, int type) @@ -4373,7 +4405,7 @@ static void wdc_print_bd_ca_log_normal(void *data) } bd_data++; if (bd_data->field_id == 0x01) { - raw = (__u64*)&bd_data->raw_value[1]; + raw = (__u64*)&bd_data->raw_value[0]; printf("erase_fail_count : %3"PRIu8"%% %"PRIu64"\n", bd_data->normalized_value, le64_to_cpu(*raw & 0x00FFFFFFFFFFFFFF)); } else { @@ -4394,7 +4426,7 @@ static void wdc_print_bd_ca_log_normal(void *data) } bd_data++; if (bd_data->field_id == 0x03) { - raw = (__u64*)&bd_data->raw_value[1]; + raw = (__u64*)&bd_data->raw_value[0]; printf("end_to_end_error_detection_count: %3"PRIu8"%% %"PRIu64"\n", bd_data->normalized_value, le64_to_cpu(*raw & 0x00FFFFFFFFFFFFFF)); } else { @@ -4402,7 +4434,7 @@ static void wdc_print_bd_ca_log_normal(void *data) } bd_data++; if (bd_data->field_id == 0x04) { - raw = (__u64*)&bd_data->raw_value[1]; + raw = (__u64*)&bd_data->raw_value[0]; printf("crc_error_count : %3"PRIu8"%% %"PRIu64"\n", bd_data->normalized_value, le64_to_cpu(*raw & 0x00FFFFFFFFFFFFFF)); } else { @@ -4410,7 +4442,7 @@ static void wdc_print_bd_ca_log_normal(void *data) } bd_data++; if (bd_data->field_id == 0x05) { - raw = (__u64*)&bd_data->raw_value[1]; + raw = (__u64*)&bd_data->raw_value[0]; printf("timed_workload_media_wear : %3"PRIu8"%% %-.3f%%\n", bd_data->normalized_value, safe_div_fp((*raw & 0x00FFFFFFFFFFFFFF), 1024.0)); @@ -4419,7 +4451,7 @@ static void wdc_print_bd_ca_log_normal(void *data) } bd_data++; if (bd_data->field_id == 0x06) { - raw = (__u64*)&bd_data->raw_value[1]; + raw = (__u64*)&bd_data->raw_value[0]; printf("timed_workload_host_reads : %3"PRIu8"%% %"PRIu64"%%\n", bd_data->normalized_value, le64_to_cpu(*raw & 0x00FFFFFFFFFFFFFF)); } else { @@ -4427,7 +4459,7 @@ static void wdc_print_bd_ca_log_normal(void *data) } bd_data++; if (bd_data->field_id == 0x07) { - raw = (__u64*)&bd_data->raw_value[1]; + raw = (__u64*)&bd_data->raw_value[0]; printf("timed_workload_timer : %3"PRIu8"%% %"PRIu64"\n", bd_data->normalized_value, le64_to_cpu(*raw & 0x00FFFFFFFFFFFFFF)); } else { @@ -4444,7 +4476,7 @@ static void wdc_print_bd_ca_log_normal(void *data) } bd_data++; if (bd_data->field_id == 0x09) { - raw = (__u64*)&bd_data->raw_value[1]; + raw = (__u64*)&bd_data->raw_value[0]; printf("retry_buffer_overflow_count : %3"PRIu8"%% %"PRIu64"\n", bd_data->normalized_value, le64_to_cpu(*raw & 0x00FFFFFFFFFFFFFF)); } else { @@ -4452,7 +4484,7 @@ static void wdc_print_bd_ca_log_normal(void *data) } bd_data++; if (bd_data->field_id == 0x0A) { - raw = (__u64*)&bd_data->raw_value[1]; + raw = (__u64*)&bd_data->raw_value[0]; printf("pll_lock_loss_count : %3"PRIu8"%% %"PRIu64"\n", bd_data->normalized_value, le64_to_cpu(*raw & 0x00FFFFFFFFFFFFFF)); } else { @@ -4460,7 +4492,7 @@ static void wdc_print_bd_ca_log_normal(void *data) } bd_data++; if (bd_data->field_id == 0x0B) { - raw = (__u64*)&bd_data->raw_value[1]; + raw = (__u64*)&bd_data->raw_value[0]; printf("nand_bytes_written : %3"PRIu8"%% sectors: %.f\n", bd_data->normalized_value, safe_div_fp((*raw & 0x00FFFFFFFFFFFFFF), 0xFFFF)); } else { @@ -4468,7 +4500,7 @@ static void wdc_print_bd_ca_log_normal(void *data) } bd_data++; if (bd_data->field_id == 0x0C) { - raw = (__u64*)&bd_data->raw_value[1]; + raw = (__u64*)&bd_data->raw_value[0]; printf("host_bytes_written : %3"PRIu8"%% sectors: %.f\n", bd_data->normalized_value, safe_div_fp((*raw & 0x00FFFFFFFFFFFFFF), 0xFFFF)); } else { @@ -4496,7 +4528,7 @@ static void wdc_print_bd_ca_log_json(void *data) root = json_create_object(); if (bd_data->field_id == 0x00) { - raw = (__u64*)&bd_data->raw_value[1]; + raw = (__u64*)&bd_data->raw_value[0]; json_object_add_value_int(root, "program_fail_count normalized", bd_data->normalized_value); json_object_add_value_int(root, "program_fail_count raw", @@ -4506,7 +4538,7 @@ static void wdc_print_bd_ca_log_json(void *data) } bd_data++; if (bd_data->field_id == 0x01) { - raw = (__u64*)&bd_data->raw_value[1]; + raw = (__u64*)&bd_data->raw_value[0]; json_object_add_value_int(root, "erase_fail_count normalized", bd_data->normalized_value); json_object_add_value_int(root, "erase_fail_count raw", @@ -4528,7 +4560,7 @@ static void wdc_print_bd_ca_log_json(void *data) } bd_data++; if (bd_data->field_id == 0x03) { - raw = (__u64*)&bd_data->raw_value[1]; + raw = (__u64*)&bd_data->raw_value[0]; json_object_add_value_int(root, "end_to_end_error_detection_count normalized", bd_data->normalized_value); json_object_add_value_int(root, "end_to_end_error_detection_count raw", @@ -4538,7 +4570,7 @@ static void wdc_print_bd_ca_log_json(void *data) } bd_data++; if (bd_data->field_id == 0x04) { - raw = (__u64*)&bd_data->raw_value[1]; + raw = (__u64*)&bd_data->raw_value[0]; json_object_add_value_int(root, "crc_error_count normalized", bd_data->normalized_value); json_object_add_value_int(root, "crc_error_count raw", @@ -4548,7 +4580,7 @@ static void wdc_print_bd_ca_log_json(void *data) } bd_data++; if (bd_data->field_id == 0x05) { - raw = (__u64*)&bd_data->raw_value[1]; + raw = (__u64*)&bd_data->raw_value[0]; json_object_add_value_int(root, "timed_workload_media_wear normalized", bd_data->normalized_value); json_object_add_value_float(root, "timed_workload_media_wear raw", @@ -4558,7 +4590,7 @@ static void wdc_print_bd_ca_log_json(void *data) } bd_data++; if (bd_data->field_id == 0x06) { - raw = (__u64*)&bd_data->raw_value[1]; + raw = (__u64*)&bd_data->raw_value[0]; json_object_add_value_int(root, "timed_workload_host_reads normalized", bd_data->normalized_value); json_object_add_value_int(root, "timed_workload_host_reads raw", @@ -4568,7 +4600,7 @@ static void wdc_print_bd_ca_log_json(void *data) } bd_data++; if (bd_data->field_id == 0x07) { - raw = (__u64*)&bd_data->raw_value[1]; + raw = (__u64*)&bd_data->raw_value[0]; json_object_add_value_int(root, "timed_workload_timer normalized", bd_data->normalized_value); json_object_add_value_int(root, "timed_workload_timer", @@ -4589,7 +4621,7 @@ static void wdc_print_bd_ca_log_json(void *data) } bd_data++; if (bd_data->field_id == 0x09) { - raw = (__u64*)&bd_data->raw_value[1]; + raw = (__u64*)&bd_data->raw_value[0]; json_object_add_value_int(root, "retry_buffer_overflow_count normalized", bd_data->normalized_value); json_object_add_value_int(root, "retry_buffer_overflow_count raw", @@ -4599,7 +4631,7 @@ static void wdc_print_bd_ca_log_json(void *data) } bd_data++; if (bd_data->field_id == 0x0A) { - raw = (__u64*)&bd_data->raw_value[1]; + raw = (__u64*)&bd_data->raw_value[0]; json_object_add_value_int(root, "pll_lock_loss_count normalized", bd_data->normalized_value); json_object_add_value_int(root, "pll_lock_loss_count raw", @@ -4609,7 +4641,7 @@ static void wdc_print_bd_ca_log_json(void *data) } bd_data++; if (bd_data->field_id == 0x0B) { - raw = (__u64*)&bd_data->raw_value[1]; + raw = (__u64*)&bd_data->raw_value[0]; json_object_add_value_int(root, "nand_bytes_written normalized", bd_data->normalized_value); json_object_add_value_float(root, "nand_bytes_written raw", @@ -4619,7 +4651,7 @@ static void wdc_print_bd_ca_log_json(void *data) } bd_data++; if (bd_data->field_id == 0x0C) { - raw = (__u64*)&bd_data->raw_value[1]; + raw = (__u64*)&bd_data->raw_value[0]; json_object_add_value_int(root, "host_bytes_written normalized", bd_data->normalized_value); json_object_add_value_float(root, "host_bytes_written raw", @@ -4806,6 +4838,7 @@ static void wdc_print_fw_act_history_log_normal(__u8 *data, int num_entries, __u struct wdc_fw_act_history_log_format_c2 *fw_act_history_entry = (struct wdc_fw_act_history_log_format_c2 *)(data); + oldestEntryIdx = WDC_MAX_NUM_ACT_HIST_ENTRIES; if (num_entries == WDC_MAX_NUM_ACT_HIST_ENTRIES) { /* find lowest/oldest entry */ for (i = 0; i < num_entries; i++) { @@ -4889,6 +4922,7 @@ static void wdc_print_fw_act_history_log_normal(__u8 *data, int num_entries, __u struct wdc_fw_act_history_log_entry *fw_act_history_entry = (struct wdc_fw_act_history_log_entry *)(data + sizeof(struct wdc_fw_act_history_log_hdr)); + oldestEntryIdx = WDC_MAX_NUM_ACT_HIST_ENTRIES; if (num_entries == WDC_MAX_NUM_ACT_HIST_ENTRIES) { /* find lowest/oldest entry */ for (i = 0; i < num_entries; i++) { @@ -4968,6 +5002,7 @@ static void wdc_print_fw_act_history_log_json(__u8 *data, int num_entries, __u32 if(data[0] == WDC_NVME_GET_FW_ACT_HISTORY_C2_LOG_ID) { struct wdc_fw_act_history_log_format_c2 *fw_act_history_entry = (struct wdc_fw_act_history_log_format_c2 *)(data); + oldestEntryIdx = WDC_MAX_NUM_ACT_HIST_ENTRIES; if (num_entries == WDC_MAX_NUM_ACT_HIST_ENTRIES) { /* find lowest/oldest entry */ for (i = 0; i < num_entries; i++) { @@ -5038,6 +5073,7 @@ static void wdc_print_fw_act_history_log_json(__u8 *data, int num_entries, __u32 else { struct wdc_fw_act_history_log_entry *fw_act_history_entry = (struct wdc_fw_act_history_log_entry *)(data + sizeof(struct wdc_fw_act_history_log_hdr)); + oldestEntryIdx = WDC_MAX_NUM_ACT_HIST_ENTRIES; if (num_entries == WDC_MAX_NUM_ACT_HIST_ENTRIES) { /* find lowest/oldest entry */ for (i = 0; i < num_entries; i++) { @@ -8347,7 +8383,10 @@ static int wdc_fetch_log_file_from_device(int fd, __u32 fileId, __u16 spiDestn, goto end; } - wdc_get_max_transfer_len(fd, &maximumTransferLength); + if (wdc_get_max_transfer_len(fd, &maximumTransferLength) < 0) { + ret = WDC_STATUS_FAILURE; + goto end; + } /* Fetch Log File Data */ if ((fileSize >= maximumTransferLength) || (fileSize > 0xFFFFFFFF)) @@ -8407,7 +8446,8 @@ static int wdc_de_get_dump_trace(int fd, char * filePath, __u16 binFileNameLen, return ret; } - wdc_get_max_transfer_len(fd, &maximumTransferLength); + if (wdc_get_max_transfer_len(fd, &maximumTransferLength) < 0) + return WDC_STATUS_FAILURE; do { @@ -9052,8 +9092,12 @@ static int wdc_reason_identifier(int argc, char **argv, ret = -1; goto close_fd; } - - snprintf(f + strlen(f), PATH_MAX, "%s", ".bin"); + if (strlen(f) > PATH_MAX - 5) { + fprintf(stderr, "ERROR : WDC: file name overflow\n"); + ret = -1; + goto close_fd; + } + strcat(f, ".bin"); } fprintf(stderr, "%s: filename = %s\n", __func__, f); @@ -9266,7 +9310,11 @@ static int wdc_save_reason_id(int fd, __u8 *rsn_ident, int size) /* make the nvmecli dir in /usr/local if it doesn't already exist */ if (stat(reason_id_path, &st) == -1) { - mkdir(reason_id_path, 0700); + if (mkdir(reason_id_path, 0700) < 0) { + fprintf(stderr, "%s: ERROR : failed to mkdir %s : %s\n", + __func__, reason_id_path, strerror(errno)); + return -1; + } } if (asprintf(&reason_id_file, "%s/%s%s", reason_id_path,