GCC 7.0.1 can determine if there is likely to be sufficient space in the output buffer when using sprintf/snprintf, based on the format string. The errors were all either of this form: bindtests.c:717:29: error: '%zu' directive output may be truncated writing between 1 and 19 bytes into a region of size 16 [-Werror=format-truncation=] snprintf (strs[i], 16, "%zu", i); ^~~ bindtests.c:717:28: note: directive argument in the range [0, 2305843009213693951] snprintf (strs[i], 16, "%zu", i); ^~~~~ or this form: sync.c: In function 'fsync_devices': sync.c:108:50: error: '%s' directive output may be truncated writing up to 255 bytes into a region of size 251 [-Werror=format-truncation=] snprintf (dev_path, sizeof dev_path, "/dev/%s", d->d_name); ^~ Fixed by converting these into dynamic allocation, or making the output buffer larger, whichever was easier. --- cat/filesystems.c | 2 +- daemon/9p.c | 10 +++++++--- daemon/debug.c | 12 ++++++++++-- daemon/devsparts.c | 16 ++++++++++++---- daemon/sync.c | 7 +++++-- generator/bindtests.ml | 12 ++++++------ 6 files changed, 41 insertions(+), 18 deletions(-) diff --git a/cat/filesystems.c b/cat/filesystems.c index 1036c6f..4264b0f 100644 --- a/cat/filesystems.c +++ b/cat/filesystems.c @@ -866,7 +866,7 @@ write_row (const char *name, const char *type, size_t len = 0; char hum[LONGEST_HUMAN_READABLE]; char num[256]; - char mbr_id_str[3]; + char mbr_id_str[9]; if ((columns & COLUMN_NAME)) strings[len++] = name; diff --git a/daemon/9p.c b/daemon/9p.c index a9e36d1..f72c8dd 100644 --- a/daemon/9p.c +++ b/daemon/9p.c @@ -71,9 +71,13 @@ do_list_9p (void) if (d == NULL) break; if (STRPREFIX (d->d_name, "virtio")) { - char mount_tag_path[256]; - snprintf (mount_tag_path, sizeof mount_tag_path, - BUS_PATH "/%s/mount_tag", d->d_name); + CLEANUP_FREE char *mount_tag_path; + if (asprintf (&mount_tag_path, BUS_PATH "/%s/mount_tag", + d->d_name) == -1) { + reply_with_perror ("asprintf"); + closedir (dir); + return NULL; + } /* A bit unclear, but it looks like the virtio transport allows * the mount tag length to be unlimited (or up to 65536 bytes). diff --git a/daemon/debug.c b/daemon/debug.c index 06f0729..b18d87c 100644 --- a/daemon/debug.c +++ b/daemon/debug.c @@ -161,7 +161,7 @@ debug_fds (const char *subcmd, size_t argc, char *const *const argv) FILE *fp; DIR *dir; struct dirent *d; - char fname[256], link[256]; + char link[256]; struct stat statbuf; fp = open_memstream (&out, &size); @@ -178,10 +178,18 @@ debug_fds (const char *subcmd, size_t argc, char *const *const argv) } while ((d = readdir (dir)) != NULL) { + CLEANUP_FREE char *fname = NULL; + if (STREQ (d->d_name, ".") || STREQ (d->d_name, "..")) continue; - snprintf (fname, sizeof fname, "/proc/self/fd/%s", d->d_name); + if (asprintf (&fname, "/proc/self/fd/%s", d->d_name) == -1) { + reply_with_perror ("asprintf"); + fclose (fp); + free (out); + closedir (dir); + return NULL; + } r = lstat (fname, &statbuf); if (r == -1) { diff --git a/daemon/devsparts.c b/daemon/devsparts.c index f15d2c3..96e867f 100644 --- a/daemon/devsparts.c +++ b/daemon/devsparts.c @@ -43,7 +43,6 @@ foreach_block_device (block_dev_func_t func, bool return_md) DIR *dir; int err = 0; struct dirent *d; - char dev_path[256]; int fd; dir = opendir ("/sys/block"); @@ -64,7 +63,12 @@ foreach_block_device (block_dev_func_t func, bool return_md) STREQLEN (d->d_name, "sr", 2) || (return_md && STREQLEN (d->d_name, "md", 2) && c_isdigit (d->d_name[2]))) { - snprintf (dev_path, sizeof dev_path, "/dev/%s", d->d_name); + CLEANUP_FREE char *dev_path; + if (asprintf (&dev_path, "/dev/%s", d->d_name) == -1) { + reply_with_perror ("asprintf"); + closedir (dir); + return NULL; + } /* Ignore the root device. */ if (is_root_device (dev_path)) @@ -167,8 +171,12 @@ add_partitions (const char *device, struct stringsbuf *r) struct dirent *d; while ((d = readdir (dir)) != NULL) { if (STREQLEN (d->d_name, device, strlen (device))) { - char part[256]; - snprintf (part, sizeof part, "/dev/%s", d->d_name); + CLEANUP_FREE char *part; + if (asprintf (&part, "/dev/%s", d->d_name) == -1) { + perror ("asprintf"); + closedir (dir); + return -1; + } if (add_string (r, part) == -1) { closedir (dir); diff --git a/daemon/sync.c b/daemon/sync.c index 581fa0f..b64f6e3 100644 --- a/daemon/sync.c +++ b/daemon/sync.c @@ -86,7 +86,6 @@ fsync_devices (void) { DIR *dir; struct dirent *d; - char dev_path[256]; int fd; dir = opendir ("/sys/block"); @@ -105,7 +104,11 @@ fsync_devices (void) STREQLEN (d->d_name, "ubd", 3) || STREQLEN (d->d_name, "vd", 2) || STREQLEN (d->d_name, "sr", 2)) { - snprintf (dev_path, sizeof dev_path, "/dev/%s", d->d_name); + CLEANUP_FREE char *dev_path; + if (asprintf (&dev_path, "/dev/%s", d->d_name) == -1) { + perror ("asprintf"); + continue; + } /* Ignore the root device. */ if (is_root_device (dev_path)) diff --git a/generator/bindtests.ml b/generator/bindtests.ml index 22d0c37..2fde425 100644 --- a/generator/bindtests.ml +++ b/generator/bindtests.ml @@ -258,8 +258,8 @@ fill_lvm_pv (guestfs_h *g, struct guestfs_lvm_pv *pv, size_t i) pr " }\n"; pr " strs = safe_malloc (g, (n+1) * sizeof (char *));\n"; pr " for (i = 0; i < n; ++i) {\n"; - pr " strs[i] = safe_malloc (g, 16);\n"; - pr " snprintf (strs[i], 16, \"%%zu\", i);\n"; + pr " strs[i] = safe_malloc (g, 20);\n"; + pr " snprintf (strs[i], 20, \"%%zu\", i);\n"; pr " }\n"; pr " strs[n] = NULL;\n"; pr " return strs;\n" @@ -290,10 +290,10 @@ fill_lvm_pv (guestfs_h *g, struct guestfs_lvm_pv *pv, size_t i) pr " }\n"; pr " strs = safe_malloc (g, (n*2+1) * sizeof (*strs));\n"; pr " for (i = 0; i < n; ++i) {\n"; - pr " strs[i*2] = safe_malloc (g, 16);\n"; - pr " strs[i*2+1] = safe_malloc (g, 16);\n"; - pr " snprintf (strs[i*2], 16, \"%%zu\", i);\n"; - pr " snprintf (strs[i*2+1], 16, \"%%zu\", i);\n"; + pr " strs[i*2] = safe_malloc (g, 20);\n"; + pr " strs[i*2+1] = safe_malloc (g, 20);\n"; + pr " snprintf (strs[i*2], 20, \"%%zu\", i);\n"; + pr " snprintf (strs[i*2+1], 20, \"%%zu\", i);\n"; pr " }\n"; pr " strs[n*2] = NULL;\n"; pr " return strs;\n" -- 2.10.2