From: Mauro Carvalho Chehab Date: Thu, 18 Jul 2024 11:02:30 +0000 (+0200) Subject: rasdaemon: don't use unsafe strcpy, strcat and sprintf X-Git-Tag: v0.8.2~52 X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=86aabc578ed6abc00a5f0223d738cdd2ccd18bef;p=users%2Fmchehab%2Frasdaemon.git rasdaemon: don't use unsafe strcpy, strcat and sprintf Remove all occurrences of those calls. While here, also fix a couple missing whitespace warnings. Signed-off-by: Mauro Carvalho Chehab --- diff --git a/mce-amd-k8.c b/mce-amd-k8.c index a937516..d76d49b 100644 --- a/mce-amd-k8.c +++ b/mce-amd-k8.c @@ -297,7 +297,8 @@ int parse_amd_k8_event(struct ras_events *ras, struct mce_event *e) decode_k8_threashold(e); break; default: - strcpy(e->error_msg, "Don't know how to decode this bank"); + strscpy(e->error_msg, "Don't know how to decode this bank", + sizeof(e->error_msg)); } /* IP doesn't matter on memory errors */ diff --git a/mce-amd-smca.c b/mce-amd-smca.c index f3d4171..3e39e94 100644 --- a/mce-amd-smca.c +++ b/mce-amd-smca.c @@ -952,17 +952,20 @@ void decode_smca_error(struct mce_event *e, struct mce_priv *m) } if (i >= MAX_NR_BANKS) { - strcpy(e->mcastatus_msg, "Couldn't find bank type with IPID"); + strscpy(e->mcastatus_msg, "Couldn't find bank type with IPID", + sizeof(e->mcastatus_msg)); return; } if (bank_type >= N_SMCA_BANK_TYPES) { - strcpy(e->mcastatus_msg, "Don't know how to decode this bank"); + strscpy(e->mcastatus_msg, "Don't know how to decode this bank", + sizeof(e->mcastatus_msg)); return; } if (bank_type == SMCA_RESERVED) { - strcpy(e->mcastatus_msg, "Bank 4 is reserved.\n"); + strscpy(e->mcastatus_msg, "Bank 4 is reserved.\n", + sizeof(e->mcastatus_msg)); return; } diff --git a/mce-amd.c b/mce-amd.c index 028a329..4ab6329 100644 --- a/mce-amd.c +++ b/mce-amd.c @@ -80,16 +80,21 @@ void decode_amd_errcode(struct mce_event *e) if (e->status & MCI_STATUS_UC) { if (e->status & MCI_STATUS_PCC) - strcpy(e->error_msg, "System Fatal error."); + strscpy(e->error_msg, "System Fatal error.", + sizeof(e->error_msg)); if (e->mcgstatus & MCG_STATUS_RIPV) - strcpy(e->error_msg, - "Uncorrected, software restartable error."); - strcpy(e->error_msg, - "Uncorrected, software containable error."); + strscpy(e->error_msg, + "Uncorrected, software restartable error.", + sizeof(e->error_msg)); + strscpy(e->error_msg, + "Uncorrected, software containable error.", + sizeof(e->error_msg)); } else if (e->status & MCI_STATUS_DEFERRED) { - strcpy(e->error_msg, "Deferred error, no action required."); + strscpy(e->error_msg, "Deferred error, no action required.", + sizeof(e->error_msg)); } else { - strcpy(e->error_msg, "Corrected error, no action required."); + strscpy(e->error_msg, "Corrected error, no action required.", + sizeof(e->error_msg)); } if (!(e->status & MCI_STATUS_VAL)) diff --git a/mce-intel.c b/mce-intel.c index bec84ec..b399467 100644 --- a/mce-intel.c +++ b/mce-intel.c @@ -151,9 +151,9 @@ static void decode_memory_controller(struct mce_event *e, uint32_t status) char channel[30]; if ((status & 0xf) == 0xf) - sprintf(channel, "unspecified"); + snprintf(channel, sizeof(channel), "unspecified"); else - sprintf(channel, "%u", status & 0xf); + snprintf(channel, sizeof(channel), "%u", status & 0xf); mce_snprintf(e->error_msg, "MEMORY CONTROLLER %s_CHANNEL%s_ERR", mmm_mnemonic[(status >> 4) & 7], channel); @@ -195,14 +195,14 @@ static void decode_mcg(struct mce_event *e) static void bank_name(struct mce_event *e) { - char *buf = e->bank_name; - switch (e->bank) { case MCE_THERMAL_BANK: - strcpy(buf, "THERMAL EVENT"); + strscpy(e->bank_name, "THERMAL EVENT", sizeof(e->bank_name)); break; case MCE_TIMEOUT_BANK: - strcpy(buf, "Timeout waiting for exception on other CPUs"); + strscpy(e->bank_name, + "Timeout waiting for exception on other CPUs", + sizeof(e->bank_name)); break; default: break; @@ -435,7 +435,7 @@ static int domsr(int cpu, int msr, int bit) unsigned long long data; int fd; - sprintf(fpath, "/dev/cpu/%d/msr", cpu); + snprintf(fpath, sizeof(fpath), "/dev/cpu/%d/msr", cpu); fd = open(fpath, O_RDWR); if (fd == -1) { switch (errno) { diff --git a/ras-aer-handler.c b/ras-aer-handler.c index 138abe2..e5fe23a 100644 --- a/ras-aer-handler.c +++ b/ras-aer-handler.c @@ -188,9 +188,9 @@ int ras_aer_event_handler(struct trace_seq *s, sel_data[3] = bus; sel_data[4] = (((dev & 0x1f) << 3) | (fn & 0x7)); - sprintf(ipmi_add_sel, - "ipmitool raw 0x0a 0x44 0x00 0x00 0xc0 0x00 0x00 0x00 0x00 0x3a 0xcd 0x00 0xc0 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x", - sel_data[0], sel_data[1], sel_data[2], sel_data[3], sel_data[4]); + snprintf(ipmi_add_sel, sizeof(ipmi_add_sel), + "ipmitool raw 0x0a 0x44 0x00 0x00 0xc0 0x00 0x00 0x00 0x00 0x3a 0xcd 0x00 0xc0 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x", + sel_data[0], sel_data[1], sel_data[2], sel_data[3], sel_data[4]); rc = system(ipmi_add_sel); if (rc) diff --git a/ras-arm-handler.c b/ras-arm-handler.c index c090943..77f6358 100644 --- a/ras-arm-handler.c +++ b/ras-arm-handler.c @@ -40,7 +40,7 @@ #define ARM_INFO_FLAGS_PROPAGATED BIT(2) #define ARM_INFO_FLAGS_OVERFLOW BIT(3) -#define ARM_ERR_TYPE_MASK GENMASK(4,1) +#define ARM_ERR_TYPE_MASK GENMASK(4, 1) #define ARM_CACHE_ERROR BIT(1) #define ARM_TLB_ERROR BIT(2) #define ARM_BUS_ERROR BIT(3) diff --git a/ras-cpu-isolation.c b/ras-cpu-isolation.c index 0daa63a..90200c5 100644 --- a/ras-cpu-isolation.c +++ b/ras-cpu-isolation.c @@ -255,14 +255,13 @@ void cpu_infos_free(void) static int do_cpu_offline(unsigned int cpu) { int fd, rc; - char buf[2] = ""; + char buf[2] = "0"; cpu_infos[cpu].state = CPU_OFFLINE_FAILED; fd = open_sys_file(cpu, O_RDWR, cpu_path_format); if (fd == -1) return HANDLE_FAILED; - strcpy(buf, "0"); rc = write(fd, buf, strlen(buf)); if (rc < 0) { log(TERM, LOG_ERR, "cpu%u offline failed, errno:%d\n", cpu, errno); diff --git a/ras-cxl-handler.c b/ras-cxl-handler.c index dd3cfeb..e49c128 100644 --- a/ras-cxl-handler.c +++ b/ras-cxl-handler.c @@ -40,7 +40,7 @@ static void convert_timestamp(unsigned long long ts, char *ts_ptr, uint16_t size strftime(ts_ptr, size, "%Y-%m-%d %H:%M:%S %z", tm); if (!ts || !tm) - strncpy(ts_ptr, "1970-01-01 00:00:00 +0000", + strscpy(ts_ptr, "1970-01-01 00:00:00 +0000", size); } @@ -55,7 +55,7 @@ static void get_timestamp(struct trace_seq *s, struct tep_record *record, if (tm) strftime(ts_ptr, size, "%Y-%m-%d %H:%M:%S %z", tm); else - strncpy(ts_ptr, "1970-01-01 00:00:00 +0000", size); + strscpy(ts_ptr, "1970-01-01 00:00:00 +0000", size); } struct cxl_event_flags { @@ -85,7 +85,7 @@ static char *uuid_be(const char *uu) static const unsigned char be[16] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}; for (i = 0; i < 16; i++) { - p += sprintf(p, "%.2x", (unsigned char)uu[be[i]]); + p += snprintf(p, sizeof(uuid), "%.2x", (unsigned char)uu[be[i]]); switch (i) { case 3: case 5: @@ -249,7 +249,7 @@ int ras_cxl_poison_event_handler(struct trace_seq *s, return -1; convert_timestamp(val, ev.overflow_ts, sizeof(ev.overflow_ts)); } else { - strncpy(ev.overflow_ts, "1970-01-01 00:00:00 +0000", + strscpy(ev.overflow_ts, "1970-01-01 00:00:00 +0000", sizeof(ev.overflow_ts)); } diff --git a/ras-events.c b/ras-events.c index 53e15de..81535e8 100644 --- a/ras-events.c +++ b/ras-events.c @@ -100,8 +100,7 @@ static int get_debugfs_dir(char *tracing_dir, size_t len) if (!strcmp(type, "debugfs")) { fclose(fp); - strncpy(tracing_dir, dir, len - 1); - tracing_dir[len - 1] = '\0'; + strscpy(tracing_dir, dir, len - 1); return 0; } } while (1); @@ -114,10 +113,17 @@ static int get_debugfs_dir(char *tracing_dir, size_t len) static int open_trace(struct ras_events *ras, char *name, int flags) { char fname[MAX_PATH + 1]; + int rc; - strcpy(fname, ras->tracing); - strcat(fname, "/"); - strcat(fname, name); + rc = strscpy(fname, ras->tracing, sizeof(fname)); + if (rc < 0) + return rc; + rc = strscat(fname, "/", sizeof(fname)); + if (rc < 0) + return rc; + rc = strscat(fname, name, sizeof(fname)); + if (rc < 0) + return rc; return open(fname, flags); } @@ -131,8 +137,13 @@ static int get_tracing_dir(struct ras_events *ras) get_debugfs_dir(ras->debugfs, sizeof(ras->debugfs)); - strcpy(fname, ras->debugfs); - strcat(fname, "/tracing"); + rc = strscpy(fname, ras->debugfs, sizeof(fname)); + if (rc < 0) + return rc; + rc = strscat(fname, "/tracing", sizeof(fname)); + if (rc < 0) + return rc; + dir = opendir(fname); if (!dir) return -1; @@ -145,10 +156,14 @@ static int get_tracing_dir(struct ras_events *ras) } closedir(dir); - strcpy(ras->tracing, ras->debugfs); - strcat(ras->tracing, "/tracing"); + strscpy(ras->tracing, ras->debugfs, sizeof(ras->tracing)); + strscat(ras->tracing, "/tracing", sizeof(ras->tracing)); if (has_instances) { - strcat(ras->tracing, "/instances/" TOOL_NAME); + rc = strscat(ras->tracing, "/instances/" TOOL_NAME, + sizeof(ras->tracing)); + if (rc < 0) + return rc; + rc = mkdir(ras->tracing, 0700); if (rc < 0 && errno != EEXIST) { log(ALL, LOG_INFO, diff --git a/ras-extlog-handler.c b/ras-extlog-handler.c index 8b5680f..8d4739e 100644 --- a/ras-extlog-handler.c +++ b/ras-extlog-handler.c @@ -108,40 +108,85 @@ static char *err_cper_data(const char *c) { const struct cper_mem_err_compact *cpd = (struct cper_mem_err_compact *)c; static char buf[256]; + unsigned int rc, size = sizeof(buf); char *p = buf; if (cpd->validation_bits == 0) return ""; - p += sprintf(p, " ("); - if (cpd->validation_bits & CPER_MEM_VALID_NODE) - p += sprintf(p, "node: %d ", cpd->node); - if (cpd->validation_bits & CPER_MEM_VALID_CARD) - p += sprintf(p, "card: %d ", cpd->card); - if (cpd->validation_bits & CPER_MEM_VALID_MODULE) - p += sprintf(p, "module: %d ", cpd->module); - if (cpd->validation_bits & CPER_MEM_VALID_BANK) - p += sprintf(p, "bank: %d ", cpd->bank); - if (cpd->validation_bits & CPER_MEM_VALID_DEVICE) - p += sprintf(p, "device: %d ", cpd->device); - if (cpd->validation_bits & CPER_MEM_VALID_ROW) - p += sprintf(p, "row: %d ", cpd->row); - if (cpd->validation_bits & CPER_MEM_VALID_COLUMN) - p += sprintf(p, "column: %d ", cpd->column); - if (cpd->validation_bits & CPER_MEM_VALID_BIT_POSITION) - p += sprintf(p, "bit_pos: %d ", cpd->bit_pos); - if (cpd->validation_bits & CPER_MEM_VALID_REQUESTOR_ID) - p += sprintf(p, "req_id: 0x%llx ", cpd->requestor_id); - if (cpd->validation_bits & CPER_MEM_VALID_RESPONDER_ID) - p += sprintf(p, "resp_id: 0x%llx ", cpd->responder_id); - if (cpd->validation_bits & CPER_MEM_VALID_TARGET_ID) - p += sprintf(p, "tgt_id: 0x%llx ", cpd->target_id); - if (cpd->validation_bits & CPER_MEM_VALID_RANK_NUMBER) - p += sprintf(p, "rank: %d ", cpd->rank); - if (cpd->validation_bits & CPER_MEM_VALID_CARD_HANDLE) - p += sprintf(p, "card_handle: %d ", cpd->mem_array_handle); - if (cpd->validation_bits & CPER_MEM_VALID_MODULE_HANDLE) - p += sprintf(p, "module_handle: %d ", cpd->mem_dev_handle); - p += sprintf(p - 1, ")"); + rc = snprintf(p, size, " ("); + p += rc; + size -= rc; + if (cpd->validation_bits & CPER_MEM_VALID_NODE) { + rc = snprintf(p, size, "node: %d ", cpd->node); + p += rc; + size -= rc; + } + if (cpd->validation_bits & CPER_MEM_VALID_CARD) { + rc = snprintf(p, size, "card: %d ", cpd->card); + p += rc; + size -= rc; + } + if (cpd->validation_bits & CPER_MEM_VALID_MODULE) { + rc = snprintf(p, size, "module: %d ", cpd->module); + p += rc; + size -= rc; + } + if (cpd->validation_bits & CPER_MEM_VALID_BANK) { + rc = snprintf(p, size, "bank: %d ", cpd->bank); + p += rc; + size -= rc; + } + if (cpd->validation_bits & CPER_MEM_VALID_DEVICE) { + rc = snprintf(p, size, "device: %d ", cpd->device); + p += rc; + size -= rc; + } + if (cpd->validation_bits & CPER_MEM_VALID_ROW) { + rc = snprintf(p, size, "row: %d ", cpd->row); + p += rc; + size -= rc; + } + if (cpd->validation_bits & CPER_MEM_VALID_COLUMN) { + rc = snprintf(p, size, "column: %d ", cpd->column); + p += rc; + size -= rc; + } + if (cpd->validation_bits & CPER_MEM_VALID_BIT_POSITION) { + rc = snprintf(p, size, "bit_pos: %d ", cpd->bit_pos); + p += rc; + size -= rc; + } + if (cpd->validation_bits & CPER_MEM_VALID_REQUESTOR_ID) { + rc = snprintf(p, size, "req_id: 0x%llx ", cpd->requestor_id); + p += rc; + size -= rc; + } + if (cpd->validation_bits & CPER_MEM_VALID_RESPONDER_ID) { + rc = snprintf(p, size, "resp_id: 0x%llx ", cpd->responder_id); + p += rc; + size -= rc; + } + if (cpd->validation_bits & CPER_MEM_VALID_TARGET_ID) { + rc = snprintf(p, size, "tgt_id: 0x%llx ", cpd->target_id); + p += rc; + size -= rc; + } + if (cpd->validation_bits & CPER_MEM_VALID_RANK_NUMBER) { + rc = snprintf(p, size, "rank: %d ", cpd->rank); + p += rc; + size -= rc; + } + if (cpd->validation_bits & CPER_MEM_VALID_CARD_HANDLE) { + rc = snprintf(p, size, "card_handle: %d ", cpd->mem_array_handle); + p += rc; + size -= rc; + } + if (cpd->validation_bits & CPER_MEM_VALID_MODULE_HANDLE) { + rc = snprintf(p, size, "module_handle: %d ", cpd->mem_dev_handle); + p += rc; + size -= rc; + } + rc = snprintf(p - 1, size, ")"); return buf; } @@ -154,7 +199,7 @@ static char *uuid_le(const char *uu) static const unsigned char le[16] = {3, 2, 1, 0, 5, 4, 7, 6, 8, 9, 10, 11, 12, 13, 14, 15}; for (i = 0; i < 16; i++) { - p += sprintf(p, "%.2x", (unsigned char)uu[le[i]]); + p += snprintf(p, sizeof(uuid), "%.2x", (unsigned char)uu[le[i]]); switch (i) { case 3: case 5: diff --git a/ras-memory-failure-handler.c b/ras-memory-failure-handler.c index 1a4d17a..358bc9a 100644 --- a/ras-memory-failure-handler.c +++ b/ras-memory-failure-handler.c @@ -197,12 +197,12 @@ int ras_memory_failure_event_handler(struct trace_seq *s, strftime(ev.timestamp, sizeof(ev.timestamp), "%Y-%m-%d %H:%M:%S %z", tm); else - strncpy(ev.timestamp, "1970-01-01 00:00:00 +0000", sizeof(ev.timestamp)); + strscpy(ev.timestamp, "1970-01-01 00:00:00 +0000", sizeof(ev.timestamp)); trace_seq_printf(s, "%s ", ev.timestamp); if (tep_get_field_val(s, event, "pfn", record, &val, 1) < 0) return -1; - sprintf(ev.pfn, "0x%llx", val); + snprintf(ev.pfn, sizeof(ev.pfn), "0x%llx", val); trace_seq_printf(s, "pfn=0x%llx ", val); if (tep_get_field_val(s, event, "type", record, &val, 1) < 0) diff --git a/ras-non-standard-handler.c b/ras-non-standard-handler.c index 7d3cecc..6d830fd 100644 --- a/ras-non-standard-handler.c +++ b/ras-non-standard-handler.c @@ -40,7 +40,7 @@ static char *uuid_le(const char *uu) static const unsigned char le[16] = {3, 2, 1, 0, 5, 4, 7, 6, 8, 9, 10, 11, 12, 13, 14, 15}; for (i = 0; i < 16; i++) { - p += sprintf(p, "%.2x", (unsigned char)uu[le[i]]); + p += snprintf(p, sizeof(uuid), "%.2x", (unsigned char)uu[le[i]]); switch (i) { case 3: case 5: diff --git a/ras-page-isolation.c b/ras-page-isolation.c index 0589492..d3706c1 100644 --- a/ras-page-isolation.c +++ b/ras-page-isolation.c @@ -225,7 +225,7 @@ static int do_page_offline(unsigned long long addr, enum otype type) return -1; } - sprintf(buf, "%#llx", addr); + snprintf(buf, sizeof(buf), "%#llx", addr); rc = write(fd, buf, strlen(buf)); if (rc < 0) log(TERM, LOG_ERR, diff --git a/ras-record.c b/ras-record.c index ed5bd48..5cc81ea 100644 --- a/ras-record.c +++ b/ras-record.c @@ -1107,9 +1107,9 @@ static int __ras_mc_prepare_stmt(struct sqlite3_priv *priv, for (i = 1; i < db_tab->num_fields; i++) { if (i < db_tab->num_fields - 1) - strcat(sql, "?, "); + strscat(sql, "?, ", sizeof(sql)); else - strcat(sql, "?)"); + strscat(sql, "?)", sizeof(sql)); } #ifdef DEBUG_SQL diff --git a/ras-report.c b/ras-report.c index 3148315..3898041 100644 --- a/ras-report.c +++ b/ras-report.c @@ -33,7 +33,7 @@ static int setup_report_socket(void) memset(&addr, 0, sizeof(struct sockaddr_un)); addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, ABRT_SOCKET, sizeof(addr.sun_path)); + strscpy(addr.sun_path, ABRT_SOCKET, sizeof(addr.sun_path)); addr.sun_path[sizeof(addr.sun_path) - 1] = '\0'; rc = connect(sockfd, (struct sockaddr *)&addr, sizeof(struct sockaddr_un)); diff --git a/types.h b/types.h index 5e77967..d0b4ee3 100644 --- a/types.h +++ b/types.h @@ -101,7 +101,7 @@ /* BIT handling */ -#define _AC(X,Y) (X##Y) +#define _AC(X, Y) (X##Y) #define _UL(x) (_AC(x, UL)) #define _ULL(x) (_AC(x, ULL))