From 4bdd9cf512845de38efcd983d06803615da9ae62 Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Wed, 26 Dec 2018 15:08:41 +0900 Subject: [PATCH] ubsan: remove most sprintf calls sprintf is implemented as snprintf(..., INT_MAX, ...) which will overflow the argument pointer for the end, then fix the end to be -1. This technically works but we know the actual buffer size in all these call sites, might as well do this properly Change-Id: I807d09f46a0221f539063fda515e1c504e658d40 --- kernel/debug.c | 8 +++++--- kernel/mem.c | 8 +++++--- kernel/procfs.c | 28 ++++++++++++++++++---------- 3 files changed, 28 insertions(+), 16 deletions(-) diff --git a/kernel/debug.c b/kernel/debug.c index 0328eb93..aecd165c 100644 --- a/kernel/debug.c +++ b/kernel/debug.c @@ -25,7 +25,7 @@ struct ihk_kmsg_buf *kmsg_buf; extern int vsnprintf(char *buf, size_t size, const char *fmt, va_list args); -extern int sprintf(char * buf, const char *fmt, ...); +extern int snprintf(char *buf, size_t size, const char *fmt, ...); extern void eventfd(int type); static ihk_spinlock_t kmsg_lock; extern char *find_command_line(char *name); @@ -110,7 +110,8 @@ int __kprintf(const char *format, ...) } /* Copy into the local buf */ - len = sprintf(buf, "[%3d]: ", ihk_mc_get_processor_id()); + len = snprintf(buf, KPRINTF_LOCAL_BUF_LEN, "[%3d]: ", + ihk_mc_get_processor_id()); va_start(va, format); len += vsnprintf(buf + len, KPRINTF_LOCAL_BUF_LEN - len - 2, format, va); @@ -149,7 +150,8 @@ int kprintf(const char *format, ...) } /* Copy into the local buf */ - len = sprintf(buf, "[%3d]: ", ihk_mc_get_processor_id()); + len = snprintf(buf, KPRINTF_LOCAL_BUF_LEN, "[%3d]: ", + ihk_mc_get_processor_id()); va_start(va, format); len += vsnprintf(buf + len, KPRINTF_LOCAL_BUF_LEN - len - 2, format, va); diff --git a/kernel/mem.c b/kernel/mem.c index c199f3a6..717b147f 100644 --- a/kernel/mem.c +++ b/kernel/mem.c @@ -1474,9 +1474,10 @@ static void numa_distances_init() char buf[1024]; char *pbuf = buf; - pbuf += sprintf(pbuf, "NUMA %d distances: ", i); + pbuf += snprintf(pbuf, 1024, "NUMA %d distances: ", i); for (j = 0; j < ihk_mc_get_nr_numa_nodes(); ++j) { - pbuf += sprintf(pbuf, "%d (%d), ", + pbuf += snprintf(pbuf, 1024 - (pbuf - buf), + "%d (%d), ", memory_nodes[i].nodes_by_distance[j].id, memory_nodes[i].nodes_by_distance[j].distance); } @@ -1519,7 +1520,8 @@ void numa_sysfs_setup(void) { char path[PATH_MAX]; for (i = 0; i < ihk_mc_get_nr_numa_nodes(); ++i) { - sprintf(path, "/sys/devices/system/node/node%d/meminfo", i); + snprintf(path, PATH_MAX, + "/sys/devices/system/node/node%d/meminfo", i); error = sysfs_createf(&numa_sysfs_meminfo, &memory_nodes[i], 0444, path); diff --git a/kernel/procfs.c b/kernel/procfs.c index 577aa329..dc7f344d 100644 --- a/kernel/procfs.c +++ b/kernel/procfs.c @@ -35,8 +35,7 @@ #define dprintf(...) #endif -extern int snprintf(char * buf, size_t size, const char *fmt, ...); -extern int sprintf(char * buf, const char *fmt, ...); +extern int snprintf(char *buf, size_t size, const char *fmt, ...); extern int sscanf(const char * buf, const char * fmt, ...); extern int scnprintf(char * buf, size_t size, const char *fmt, ...); @@ -324,6 +323,8 @@ int process_procfs_request(struct ikc_scd_packet *rpacket) for (cpu = 0; cpu < num_processors; ++cpu) { ans = snprintf(buf, count, "cpu%d\n", cpu); + if (ans < 0 || ans > count) + goto err; if (buf_add(&buf_top, &buf_cur, buf, ans) < 0) goto err; } @@ -333,7 +334,7 @@ int process_procfs_request(struct ikc_scd_packet *rpacket) #ifdef POSTK_DEBUG_ARCH_DEP_42 /* /proc/cpuinfo support added. */ else if (!strcmp(p, "cpuinfo")) { /* "/proc/cpuinfo" */ ans = ihk_mc_show_cpuinfo(buf, count, 0, &eof); - if (ans < 0) + if (ans < 0 || ans > count) goto err; if (buf_add(&buf_top, &buf_cur, buf, ans) < 0) goto err; @@ -452,7 +453,8 @@ int process_procfs_request(struct ikc_scd_packet *rpacket) "" ); - if (buf_add(&buf_top, &buf_cur, buf, ans) < 0) { + if (ans < 0 || ans > count || + buf_add(&buf_top, &buf_cur, buf, ans) < 0) { ihk_mc_spinlock_unlock_noirq(&vm->memory_range_lock); goto err; } @@ -573,30 +575,35 @@ int process_procfs_request(struct ikc_scd_packet *rpacket) proc->rgid, proc->egid, proc->sgid, proc->fsgid, state, (lockedsize + 1023) >> 10); - if (buf_add(&buf_top, &buf_cur, buf, ans) < 0) { + if (ans < 0 || ans > count || + buf_add(&buf_top, &buf_cur, buf, ans) < 0) { goto err; } ans = snprintf(buf, count, "Cpus_allowed:\t%s\n", cpu_bitmask); - if (buf_add(&buf_top, &buf_cur, buf, ans) < 0) { + if (ans < 0 || ans > count || + buf_add(&buf_top, &buf_cur, buf, ans) < 0) { kfree(bitmasks); goto err; } ans = snprintf(buf, count, "Cpus_allowed_list:\t%s\n", cpu_list); - if (buf_add(&buf_top, &buf_cur, buf, ans) < 0) { + if (ans < 0 || ans > count || + buf_add(&buf_top, &buf_cur, buf, ans) < 0) { kfree(bitmasks); goto err; } ans = snprintf(buf, count, "Mems_allowed:\t%s\n", numa_bitmask); - if (buf_add(&buf_top, &buf_cur, buf, ans) < 0) { + if (ans < 0 || ans > count || + buf_add(&buf_top, &buf_cur, buf, ans) < 0) { kfree(bitmasks); goto err; } ans = snprintf(buf, count, "Mems_allowed_list:\t%s\n", numa_list); - if (buf_add(&buf_top, &buf_cur, buf, ans) < 0) { + if (ans < 0 || ans > count || + buf_add(&buf_top, &buf_cur, buf, ans) < 0) { kfree(bitmasks); goto err; } @@ -701,7 +708,8 @@ int process_procfs_request(struct ikc_scd_packet *rpacket) 0, 0LL, 0L, 0L // policy... ); - if (buf_add(&buf_top, &buf_cur, buf, ans) < 0) + if (ans < 0 || ans > count || + buf_add(&buf_top, &buf_cur, buf, ans) < 0) goto err; ans = 0; goto end;