10:51 a.m.
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.
There is a gnulib macro we can use to make this simpler for integers.
It requires a new gnulib module (intprops), but it turns out that we
were already pulling that in through dependencies, so the change to
bootstrap is a no-op. (thanks: Dan Berrange)
---
bootstrap | 1 +
cat/filesystems.c | 3 ++-
daemon/9p.c | 10 +++++++---
daemon/debug.c | 12 ++++++++++--
daemon/devsparts.c | 16 ++++++++++++----
daemon/sync.c | 7 +++++--
generator/bindtests.ml | 14 ++++++++------
7 files changed, 45 insertions(+), 18 deletions(-)
diff --git a/bootstrap b/bootstrap
index 071682c..037d07e 100755
--- a/bootstrap
+++ b/bootstrap
@@ -66,6 +66,7 @@ hash-pjw
human
iconv
ignore-value
+intprops
lock
maintainer-makefile
manywarnings
diff --git a/cat/filesystems.c b/cat/filesystems.c
index 1036c6f..0c7748d 100644
--- a/cat/filesystems.c
+++ b/cat/filesystems.c
@@ -33,6 +33,7 @@
#include "c-ctype.h"
#include "human.h"
+#include "intprops.h"
#include "getprogname.h"
#include "guestfs.h"
@@ -866,7 +867,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[INT_BUFSIZE_BOUND (mbr_id)];
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..8ac5031 100644
--- a/generator/bindtests.ml
+++ b/generator/bindtests.ml
@@ -48,6 +48,8 @@ let rec generate_bindtests () =
#include \"guestfs-internal-actions.h\"
#include \"guestfs_protocol.h\"
+#include \"intprops.h\"
+
int
guestfs_impl_internal_test_set_output (guestfs_h *g, const char *filename)
{
@@ -258,8 +260,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, INT_BUFSIZE_BOUND (i));\n";
+ pr " snprintf (strs[i], INT_BUFSIZE_BOUND (i), \"%%zu\",
i);\n";
pr " }\n";
pr " strs[n] = NULL;\n";
pr " return strs;\n"
@@ -290,10 +292,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, INT_BUFSIZE_BOUND (i));\n";
+ pr " strs[i*2+1] = safe_malloc (g, INT_BUFSIZE_BOUND (i));\n";
+ pr " snprintf (strs[i*2], INT_BUFSIZE_BOUND (i), \"%%zu\",
i);\n";
+ pr " snprintf (strs[i*2+1], INT_BUFSIZE_BOUND (i),
\"%%zu\", i);\n";
pr " }\n";
pr " strs[n*2] = NULL;\n";
pr " return strs;\n"
--
2.10.2