GCC has two warnings related to large stack frames. We were already using the -Wframe-larger-than warning, but this reduces the threshold from 10000 to 5000 bytes. However that warning only covers the static part of frames (not alloca). So this change also enables -Wstack-usage=10000 which covers both the static and dynamic usage (alloca and variable length arrays). Multiple changes are made throughout the code to reduce frames to fit within these new limits. Note that stack allocation of large strings can be a security issue. For example, we had code like: size_t len = strlen (fs->windows_systemroot) + 64; char software[len]; snprintf (software, len, "%s/system32/config/software", fs->windows_systemroot); where fs->windows_systemroot is guest controlled. It's not clear what the effects might be of allowing the guest to allocate potentially very large stack frames, but at best it allows the guest to cause libguestfs to segfault. It turns out we are very lucky that fs->windows_systemroot cannot be set arbitrarily large (see checks in is_systemroot). This commit changes those to large heap allocations instead. --- builder/index-parse.y | 7 +++++++ builder/pxzcat-c.c | 24 ++++++++++++++++------- cat/visit.c | 7 ++++++- daemon/available.c | 8 +++++--- daemon/base64.c | 10 ++++++++-- daemon/btrfs.c | 27 ++++++++++++++++++++++++++ daemon/checksum.c | 21 +++++++++++++++------ daemon/compress.c | 10 ++++++++-- daemon/copy.c | 12 +++++++++--- daemon/cpio.c | 10 ++++++++-- daemon/dd.c | 14 ++++++++++---- daemon/debug.c | 16 ++++++++++++++-- daemon/ext2.c | 30 +++++++++++++++++------------ daemon/fill.c | 17 +++++++++++++---- daemon/find.c | 8 +++++++- daemon/findfs.c | 9 ++++++--- daemon/guestfsd.c | 20 ++++++++++++++++---- daemon/hotplug.c | 24 ++++++++++++++--------- daemon/luks.c | 8 +++++--- daemon/lvm.c | 12 ++++++++---- daemon/md.c | 9 +++++++++ daemon/ntfs.c | 10 ++++++++-- daemon/ntfsclone.c | 10 ++++++++-- daemon/proto.c | 11 +++++++++-- daemon/sh.c | 9 +++++---- daemon/tar.c | 21 ++++++++++++++++----- daemon/upload.c | 21 +++++++++++++++++---- daemon/zero.c | 28 ++++++++++++++++++++------- df/domains.c | 14 ++++++++++++-- df/parallel.c | 8 ++++++-- erlang/erl-guestfs-proto.c | 18 ++++++++++++------ examples/mount-local.c | 7 ++++++- fish/events.c | 8 +++++++- fish/fish.c | 10 ++++++++-- fish/glob.c | 33 ++++++++++++++++++++++++-------- fish/hexedit.c | 19 +++++++++++++------ generator/erlang.ml | 11 ++++++++--- generator/java.ml | 12 ++++++++++-- m4/guestfs_c.m4 | 10 +++++++--- make-fs/make-fs.c | 10 ++++++++-- p2v/conversion.c | 8 +++++++- src/appliance.c | 47 +++++++++++++++++----------------------------- src/command.c | 6 +++--- src/conn-socket.c | 4 ++-- src/copy-in-out.c | 6 +++--- src/errors.c | 2 +- src/inspect-apps.c | 7 ++----- src/inspect-fs-unix.c | 4 ++-- src/inspect-fs-windows.c | 20 +++++++++----------- src/osinfo.c | 6 ++---- src/proto.c | 4 ++-- tests/qemu/qemu-boot.c | 31 +++++++++++++++++++++++------- 52 files changed, 511 insertions(+), 207 deletions(-) diff --git a/builder/index-parse.y b/builder/index-parse.y index 82ea9d2..635b555 100644 --- a/builder/index-parse.y +++ b/builder/index-parse.y @@ -26,6 +26,13 @@ #include "index-struct.h" #include "index-parse.h" +/* The generated code uses frames > 5000 bytes. */ +#if defined(__GNUC__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wframe-larger-than=5000" +#pragma GCC diagnostic ignored "-Wstack-usage=10000" +#endif + #define YY_EXTRA_TYPE struct parse_context * extern void yyerror (YYLTYPE * yylloc, yyscan_t scanner, struct parse_context *context, const char *msg); diff --git a/builder/pxzcat-c.c b/builder/pxzcat-c.c index 4b49e74..1f5ceeb 100644 --- a/builder/pxzcat-c.c +++ b/builder/pxzcat-c.c @@ -250,8 +250,8 @@ parse_indexes (value filenamev, int fd) { lzma_ret r; off_t pos, index_size; - uint8_t footer[LZMA_STREAM_HEADER_SIZE]; - uint8_t header[LZMA_STREAM_HEADER_SIZE]; + CLEANUP_FREE uint8_t *footer = NULL; + CLEANUP_FREE uint8_t *header = NULL; lzma_stream_flags footer_flags; lzma_stream_flags header_flags; lzma_stream strm = LZMA_STREAM_INIT; @@ -260,6 +260,13 @@ parse_indexes (value filenamev, int fd) lzma_index *this_index = NULL; lzma_vli stream_padding = 0; size_t nr_streams = 0; + CLEANUP_FREE uint8_t *buf = NULL; + + footer = malloc (sizeof (uint8_t) * LZMA_STREAM_HEADER_SIZE); + header = malloc (sizeof (uint8_t) * LZMA_STREAM_HEADER_SIZE); + buf = malloc (sizeof (uint8_t) * BUFSIZ); + if (footer == NULL || header == NULL || buf == NULL) + caml_raise_out_of_memory (); /* Check file size is a multiple of 4 bytes. */ pos = lseek (fd, 0, SEEK_END); @@ -322,13 +329,11 @@ parse_indexes (value filenamev, int fd) } do { - uint8_t buf[BUFSIZ]; - strm.avail_in = index_size; if (strm.avail_in > BUFSIZ) strm.avail_in = BUFSIZ; - n = read (fd, &buf, strm.avail_in); + n = read (fd, buf, strm.avail_in); if (n == -1) unix_error (errno, (char *) "read", filenamev); @@ -454,12 +459,17 @@ iter_blocks (lzma_index *idx, unsigned nr_threads, value filenamev, int fd, value outputfilev, int ofd) { struct global_state global; - struct per_thread_state per_thread[nr_threads]; - pthread_t thread[nr_threads]; + CLEANUP_FREE struct per_thread_state *per_thread = NULL; + CLEANUP_FREE pthread_t *thread = NULL; unsigned u, nr_errors; int err; void *status; + per_thread = malloc (sizeof (struct per_thread_state) * nr_threads); + thread = malloc (sizeof (pthread_t) * nr_threads); + if (per_thread == NULL || thread == NULL) + caml_raise_out_of_memory (); + lzma_index_iter_init (&global.iter, idx); global.iter_finished = 0; err = pthread_mutex_init (&global.iter_mutex, NULL); diff --git a/cat/visit.c b/cat/visit.c index 5dfe1b6..2e08ccc 100644 --- a/cat/visit.c +++ b/cat/visit.c @@ -89,6 +89,7 @@ _visit (guestfs_h *g, int depth, const char *dir, /* Call function on everything in this directory. */ for (i = 0, xattrp = 0; names[i] != NULL; ++i, ++xattrp) { CLEANUP_FREE char *path = NULL; + CLEANUP_FREE char *attrval = NULL; struct guestfs_xattr_list file_xattrs; size_t nr_xattrs; @@ -104,7 +105,11 @@ _visit (guestfs_h *g, int depth, const char *dir, return -1; } /* attrval is not \0-terminated. */ - char attrval[xattrs->val[xattrp].attrval_len+1]; + attrval = malloc (xattrs->val[xattrp].attrval_len + 1); + if (attrval == NULL) { + perror ("malloc"); + return -1; + } memcpy (attrval, xattrs->val[xattrp].attrval, xattrs->val[xattrp].attrval_len); attrval[xattrs->val[xattrp].attrval_len] = '\0'; diff --git a/daemon/available.c b/daemon/available.c index d50c737..9716796 100644 --- a/daemon/available.c +++ b/daemon/available.c @@ -70,12 +70,14 @@ do_available_all_groups (void) static int test_proc_filesystems (const char *filesystem) { - size_t len = strlen (filesystem) + 32; - char regex[len]; + CLEANUP_FREE char *regex = NULL; CLEANUP_FREE char *err = NULL; int r; - snprintf (regex, len, "^[[:space:]]*%s$", filesystem); + if (asprintf (&regex, "^[[:space:]]*%s$", filesystem) == -1) { + perror ("asprintf"); + return -1; + } r = commandr (NULL, &err, str_grep, regex, "/proc/filesystems", NULL); if (r == -1 || r >= 2) { diff --git a/daemon/base64.c b/daemon/base64.c index 35a5d2f..3f7a630 100644 --- a/daemon/base64.c +++ b/daemon/base64.c @@ -106,7 +106,13 @@ do_base64_out (const char *file) int r; FILE *fp; CLEANUP_FREE char *cmd = NULL; - char buffer[GUESTFS_MAX_CHUNK_SIZE]; + CLEANUP_FREE char *buffer = NULL; + + buffer = malloc (GUESTFS_MAX_CHUNK_SIZE); + if (buffer == NULL) { + reply_with_perror ("malloc"); + return -1; + } /* Check the filename exists and is not a directory (RHBZ#908322). */ buf = sysroot_path (file); @@ -146,7 +152,7 @@ do_base64_out (const char *file) */ reply (NULL, NULL); - while ((r = fread (buffer, 1, sizeof buffer, fp)) > 0) { + while ((r = fread (buffer, 1, GUESTFS_MAX_CHUNK_SIZE, fp)) > 0) { if (send_file_write (buffer, r) < 0) { pclose (fp); return -1; diff --git a/daemon/btrfs.c b/daemon/btrfs.c index 3155a74..e027fb9 100644 --- a/daemon/btrfs.c +++ b/daemon/btrfs.c @@ -94,6 +94,11 @@ btrfs_set_label (const char *device, const char *label) return 0; } +#if defined(__GNUC__) && GUESTFS_GCC_VERSION >= 40800 /* gcc >= 4.8.0 */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wstack-usage=10000" +#endif + /* Takes optional arguments, consult optargs_bitmask. */ int do_btrfs_filesystem_resize (const char *filesystem, int64_t size) @@ -259,6 +264,10 @@ do_mkfs_btrfs (char *const *devices, return 0; } +#if defined(__GNUC__) && GUESTFS_GCC_VERSION >= 40800 /* gcc >= 4.8.0 */ +#pragma GCC diagnostic pop +#endif + int do_btrfs_subvolume_snapshot (const char *source, const char *dest, int ro, const char *qgroupid) @@ -715,6 +724,11 @@ test_btrfs_device_add_needs_force (void) return strstr (out, "--force") != NULL; } +#if defined(__GNUC__) && GUESTFS_GCC_VERSION >= 40800 /* gcc >= 4.8.0 */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wstack-usage=10000" +#endif + int do_btrfs_device_add (char *const *devices, const char *fs) { @@ -805,6 +819,10 @@ do_btrfs_device_delete (char *const *devices, const char *fs) } +#if defined(__GNUC__) && GUESTFS_GCC_VERSION >= 40800 /* gcc >= 4.8.0 */ +#pragma GCC diagnostic pop +#endif + /* btrfstune command add two new options * -U UUID change fsid to UUID * -u change fsid, use a random one @@ -2099,6 +2117,11 @@ do_btrfstune_enable_skinny_metadata_extent_refs (const char *device) return 0; } +#if defined(__GNUC__) && GUESTFS_GCC_VERSION >= 40800 /* gcc >= 4.8.0 */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wstack-usage=10000" +#endif + int do_btrfs_image (char *const *sources, const char *image, int compresslevel) @@ -2140,6 +2163,10 @@ do_btrfs_image (char *const *sources, const char *image, return 0; } +#if defined(__GNUC__) && GUESTFS_GCC_VERSION >= 40800 /* gcc >= 4.8.0 */ +#pragma GCC diagnostic pop +#endif + int do_btrfs_replace (const char *srcdev, const char *targetdev, const char* mntpoint) diff --git a/daemon/checksum.c b/daemon/checksum.c index eec45e2..3045948 100644 --- a/daemon/checksum.c +++ b/daemon/checksum.c @@ -132,12 +132,23 @@ do_checksums_out (const char *csumtype, const char *dir) { struct stat statbuf; int r; + const char *program; + CLEANUP_FREE char *str = NULL; + CLEANUP_FREE char *sysrootdir = NULL; + CLEANUP_FREE char *cmd = NULL; + FILE *fp; - const char *program = program_of_csum (csumtype); + str = malloc (GUESTFS_MAX_CHUNK_SIZE); + if (str == NULL) { + reply_with_perror ("malloc"); + return -1; + } + + program = program_of_csum (csumtype); if (program == NULL) return -1; - CLEANUP_FREE char *sysrootdir = sysroot_path (dir); + sysrootdir = sysroot_path (dir); if (!sysrootdir) { reply_with_perror ("malloc"); return -1; @@ -153,7 +164,7 @@ do_checksums_out (const char *csumtype, const char *dir) return -1; } - CLEANUP_FREE char *cmd = NULL; + cmd = NULL; if (asprintf_nowarn (&cmd, "cd %Q && %s -type f -print0 | %s -0 %s", sysrootdir, str_find, str_xargs, program) == -1) { reply_with_perror ("asprintf"); @@ -163,7 +174,7 @@ do_checksums_out (const char *csumtype, const char *dir) if (verbose) fprintf (stderr, "%s\n", cmd); - FILE *fp = popen (cmd, "r"); + fp = popen (cmd, "r"); if (fp == NULL) { reply_with_perror ("%s", cmd); return -1; @@ -175,8 +186,6 @@ do_checksums_out (const char *csumtype, const char *dir) */ reply (NULL, NULL); - char str[GUESTFS_MAX_CHUNK_SIZE]; - while ((r = fread (str, 1, GUESTFS_MAX_CHUNK_SIZE, fp)) > 0) { if (send_file_write (str, r) < 0) { pclose (fp); diff --git a/daemon/compress.c b/daemon/compress.c index b00be3e..36f4e66 100644 --- a/daemon/compress.c +++ b/daemon/compress.c @@ -40,7 +40,13 @@ do_compressX_out (const char *file, const char *filter, int is_device) int r; FILE *fp; CLEANUP_FREE char *cmd = NULL; - char buf[GUESTFS_MAX_CHUNK_SIZE]; + CLEANUP_FREE char *buf = NULL; + + buf = malloc (GUESTFS_MAX_CHUNK_SIZE); + if (buf == NULL) { + reply_with_perror ("malloc"); + return -1; + } /* The command will look something like: * gzip -c /sysroot%s # file @@ -80,7 +86,7 @@ do_compressX_out (const char *file, const char *filter, int is_device) */ reply (NULL, NULL); - while ((r = fread (buf, 1, sizeof buf, fp)) > 0) { + while ((r = fread (buf, 1, GUESTFS_MAX_CHUNK_SIZE, fp)) > 0) { if (send_file_write (buf, r) < 0) { pclose (fp); return -1; diff --git a/daemon/copy.c b/daemon/copy.c index 6f3e844..2a6720b 100644 --- a/daemon/copy.c +++ b/daemon/copy.c @@ -50,11 +50,17 @@ copy (const char *src, const char *src_display, { int64_t saved_size = size; int src_fd, dest_fd; - char buf[BUFSIZ]; + CLEANUP_FREE char *buf = NULL; size_t n; ssize_t r; int err; + buf = malloc (BUFSIZ); + if (buf == NULL) { + reply_with_perror ("malloc"); + return -1; + } + if ((optargs_bitmask & GUESTFS_COPY_DEVICE_TO_DEVICE_SRCOFFSET_BITMASK)) { if (srcoffset < 0) { reply_with_error ("srcoffset is negative"); @@ -119,8 +125,8 @@ copy (const char *src, const char *src_display, while (size != 0) { /* Calculate bytes to copy. */ - if (size == -1 || size > (int64_t) sizeof buf) - n = sizeof buf; + if (size == -1 || size > BUFSIZ) + n = BUFSIZ; else n = size; diff --git a/daemon/cpio.c b/daemon/cpio.c index e33e47b..d1459b1 100644 --- a/daemon/cpio.c +++ b/daemon/cpio.c @@ -45,7 +45,13 @@ do_cpio_out (const char *dir, const char *format) int r; FILE *fp; CLEANUP_FREE char *cmd = NULL; - char buffer[GUESTFS_MAX_CHUNK_SIZE]; + CLEANUP_FREE char *buffer = NULL; + + buffer = malloc (GUESTFS_MAX_CHUNK_SIZE); + if (buffer == NULL) { + reply_with_perror ("malloc"); + return -1; + } /* Check the filename exists and is a directory (RHBZ#908322). */ buf = sysroot_path (dir); @@ -97,7 +103,7 @@ do_cpio_out (const char *dir, const char *format) */ reply (NULL, NULL); - while ((r = fread (buffer, 1, sizeof buffer, fp)) > 0) { + while ((r = fread (buffer, 1, GUESTFS_MAX_CHUNK_SIZE, fp)) > 0) { if (send_file_write (buffer, r) < 0) { pclose (fp); return -1; diff --git a/daemon/dd.c b/daemon/dd.c index d29c527..461df61 100644 --- a/daemon/dd.c +++ b/daemon/dd.c @@ -106,15 +106,21 @@ do_copy_size (const char *src, const char *dest, int64_t ssize) } uint64_t position = 0, size = (uint64_t) ssize; + CLEANUP_FREE char *buf; + buf = malloc (BUFSIZ); + if (buf == NULL) { + reply_with_perror ("malloc"); + close (src_fd); + close (dest_fd); + return -1; + } while (position < size) { - char buf[BUFSIZ]; - /* Calculate bytes to copy. */ uint64_t n64 = size - position; size_t n; - if (n64 > sizeof buf) - n = sizeof buf; + if (n64 > BUFSIZ) + n = BUFSIZ; else n = (size_t) n64; /* safe because of if condition */ diff --git a/daemon/debug.c b/daemon/debug.c index 5637939..6059603 100644 --- a/daemon/debug.c +++ b/daemon/debug.c @@ -437,12 +437,18 @@ static char * debug_ls (const char *subcmd, size_t argc, char *const *const argv) { size_t len = count_strings (argv); - const char *cargv[len+3]; + CLEANUP_FREE const char **cargv; size_t i; int r; char *out; CLEANUP_FREE char *err = NULL; + cargv = malloc (sizeof (char *) * (len+3)); + if (cargv == NULL) { + reply_with_perror ("malloc"); + return NULL; + } + cargv[0] = str_ls; cargv[1] = "-a"; for (i = 0; i < len; ++i) @@ -464,12 +470,18 @@ static char * debug_ll (const char *subcmd, size_t argc, char *const *const argv) { size_t len = count_strings (argv); - const char *cargv[len+3]; + CLEANUP_FREE const char **cargv = NULL; size_t i; int r; char *out; CLEANUP_FREE char *err = NULL; + cargv = malloc (sizeof (char *) * (len+3)); + if (cargv == NULL) { + reply_with_perror ("malloc"); + return NULL; + } + cargv[0] = str_ls; cargv[1] = "-la"; for (i = 0; i < len; ++i) diff --git a/daemon/ext2.c b/daemon/ext2.c index 9ba4f09..5dd67c7 100644 --- a/daemon/ext2.c +++ b/daemon/ext2.c @@ -498,6 +498,8 @@ do_mke2fs_J (const char *fstype, int blocksize, const char *device, const char *journal) { CLEANUP_FREE char *err = NULL; + char blocksize_s[32]; + CLEANUP_FREE char *jdev = NULL; int r; if (!fstype_is_extfs (fstype)) { @@ -505,12 +507,12 @@ do_mke2fs_J (const char *fstype, int blocksize, const char *device, return -1; } - char blocksize_s[32]; snprintf (blocksize_s, sizeof blocksize_s, "%d", blocksize); - size_t len = strlen (journal); - char jdev[len+32]; - snprintf (jdev, len+32, "device=%s", journal); + if (asprintf (&jdev, "device=%s", journal) == -1) { + reply_with_perror ("asprintf"); + return -1; + } wipe_device_before_mkfs (device); @@ -530,6 +532,8 @@ do_mke2fs_JL (const char *fstype, int blocksize, const char *device, const char *label) { CLEANUP_FREE char *err = NULL; + char blocksize_s[32]; + CLEANUP_FREE char *jdev = NULL; int r; if (!fstype_is_extfs (fstype)) { @@ -543,12 +547,12 @@ do_mke2fs_JL (const char *fstype, int blocksize, const char *device, return -1; } - char blocksize_s[32]; snprintf (blocksize_s, sizeof blocksize_s, "%d", blocksize); - size_t len = strlen (label); - char jdev[len+32]; - snprintf (jdev, len+32, "device=LABEL=%s", label); + if (asprintf (&jdev, "device=LABEL=%s", label) == -1) { + reply_with_perror ("asprintf"); + return -1; + } wipe_device_before_mkfs (device); @@ -568,6 +572,8 @@ do_mke2fs_JU (const char *fstype, int blocksize, const char *device, const char *uuid) { CLEANUP_FREE char *err = NULL; + char blocksize_s[32]; + CLEANUP_FREE char *jdev = NULL; int r; if (!fstype_is_extfs (fstype)) { @@ -575,12 +581,12 @@ do_mke2fs_JU (const char *fstype, int blocksize, const char *device, return -1; } - char blocksize_s[32]; snprintf (blocksize_s, sizeof blocksize_s, "%d", blocksize); - size_t len = strlen (uuid); - char jdev[len+32]; - snprintf (jdev, len+32, "device=UUID=%s", uuid); + if (asprintf (&jdev, "device=UUID=%s", uuid) == -1) { + reply_with_perror ("asprintf"); + return -1; + } wipe_device_before_mkfs (device); diff --git a/daemon/fill.c b/daemon/fill.c index a9a3aa7..b1d2180 100644 --- a/daemon/fill.c +++ b/daemon/fill.c @@ -35,12 +35,18 @@ do_fill (int c, int len, const char *path) ssize_t r; size_t len_sz; size_t n; - char buf[BUFSIZ]; + CLEANUP_FREE char *buf; if (c < 0 || c > 255) { reply_with_error ("%d: byte number must be in range 0..255", c); return -1; } + + buf = malloc (BUFSIZ); + if (buf == NULL) { + reply_with_perror ("malloc"); + return -1; + } memset (buf, c, BUFSIZ); if (len < 0) { reply_with_error ("%d: length is < 0", len); @@ -127,13 +133,16 @@ do_fill_pattern (const char *pattern, int len, const char *path) int do_fill_dir (const char *dir, int n) { - size_t len = strlen (dir); - char filename[len+10]; int fd; int i; for (i = 0; i < n; ++i) { - snprintf (filename, len+10, "%s/%08d", dir, i); + CLEANUP_FREE char *filename; + + if (asprintf (&filename, "%s/%08d", dir, i) == -1) { + reply_with_perror ("asprintf"); + return -1; + } CHROOT_IN; fd = open (filename, O_WRONLY|O_CREAT|O_NOCTTY|O_CLOEXEC, 0666); diff --git a/daemon/find.c b/daemon/find.c index a603054..b47caa8 100644 --- a/daemon/find.c +++ b/daemon/find.c @@ -61,7 +61,13 @@ do_find0 (const char *dir) CLEANUP_FREE char *cmd = NULL; CLEANUP_FREE char *sysrootdir = NULL; size_t sysrootdirlen; - char str[GUESTFS_MAX_CHUNK_SIZE]; + CLEANUP_FREE char *str = NULL; + + str = malloc (GUESTFS_MAX_CHUNK_SIZE); + if (str == NULL) { + reply_with_perror ("malloc"); + return -1; + } sysrootdir = sysroot_path (dir); if (!sysrootdir) { diff --git a/daemon/findfs.c b/daemon/findfs.c index a5047cb..f441370 100644 --- a/daemon/findfs.c +++ b/daemon/findfs.c @@ -33,7 +33,9 @@ findfs (const char *tag, const char *label_or_uuid) { char *out; CLEANUP_FREE char *err = NULL; + CLEANUP_FREE char *arg = NULL; int r; + size_t len; /* Kill the cache file, forcing blkid to reread values from the * original filesystems. In blkid there is a '-p' option which is @@ -43,9 +45,10 @@ findfs (const char *tag, const char *label_or_uuid) unlink ("/etc/blkid/blkid.tab"); unlink ("/run/blkid/blkid.tab"); - size_t len = strlen (tag) + strlen (label_or_uuid) + 2; - char arg[len]; - snprintf (arg, len, "%s=%s", tag, label_or_uuid); + if (asprintf (&arg, "%s=%s", tag, label_or_uuid) == -1) { + reply_with_perror ("asprintf"); + return NULL; + } r = command (&out, &err, str_findfs, arg, NULL); if (r == -1) { diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c index 4bc5fd4..aa4c728 100644 --- a/daemon/guestfsd.c +++ b/daemon/guestfsd.c @@ -992,8 +992,7 @@ device_name_translation (const char *device) int parse_btrfsvol (const char *desc_orig, mountable_t *mountable) { - size_t len = strlen (desc_orig); - char desc[len+1]; + CLEANUP_FREE char *desc = NULL; CLEANUP_FREE char *device = NULL; const char *volume = NULL; char *slash; @@ -1001,10 +1000,14 @@ parse_btrfsvol (const char *desc_orig, mountable_t *mountable) mountable->type = MOUNTABLE_BTRFSVOL; - memcpy (desc, desc_orig, len+1); + if (!STRPREFIX (desc_orig, "/dev/")) + return -1; - if (! STRPREFIX (desc, "/dev/")) + desc = strdup (desc_orig); + if (desc == NULL) { + perror ("strdup"); return -1; + } slash = desc + strlen ("/dev/") - 1; while ((slash = strchr (slash + 1, '/'))) { @@ -1073,6 +1076,11 @@ mountable_to_string (const mountable_t *mountable) } } +#if defined(__GNUC__) && GUESTFS_GCC_VERSION >= 40800 /* gcc >= 4.8.0 */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wstack-usage=10000" +#endif + /* Check program exists and is executable on $PATH. */ int prog_exists (const char *prog) @@ -1105,6 +1113,10 @@ prog_exists (const char *prog) return 0; } +#if defined(__GNUC__) && GUESTFS_GCC_VERSION >= 40800 /* gcc >= 4.8.0 */ +#pragma GCC diagnostic pop +#endif + /* Pass a template such as "/sysroot/XXXXXXXX.XXX". This updates the * template to contain a randomly named file. Any 'X' characters * after the final '/' are replaced with random characters. diff --git a/daemon/hotplug.c b/daemon/hotplug.c index 2f159c6..234f51e 100644 --- a/daemon/hotplug.c +++ b/daemon/hotplug.c @@ -48,12 +48,14 @@ hotplug_error (const char *op, const char *path, const char *verb, int do_internal_hot_add_drive (const char *label) { + CLEANUP_FREE char *path = NULL; time_t start_t, now_t; - size_t len = strlen (label); - char path[len+64]; int r; - snprintf (path, len+64, "/dev/disk/guestfs/%s", label); + if (asprintf (&path, "/dev/disk/guestfs/%s", label) == -1) { + reply_with_perror ("asprintf"); + return -1; + } time (&start_t); @@ -81,8 +83,7 @@ GUESTFSD_EXT_CMD(str_fuser, fuser); int do_internal_hot_remove_drive_precheck (const char *label) { - size_t len = strlen (label); - char path[len+64]; + CLEANUP_FREE char *path = NULL; int r; CLEANUP_FREE char *out = NULL, *err = NULL; @@ -90,7 +91,10 @@ do_internal_hot_remove_drive_precheck (const char *label) udev_settle (); sync_disks (); - snprintf (path, len+64, "/dev/disk/guestfs/%s", label); + if (asprintf (&path, "/dev/disk/guestfs/%s", label) == -1) { + reply_with_perror ("asprintf"); + return -1; + } r = commandr (&out, &err, str_fuser, "-v", "-m", path, NULL); if (r == -1) { @@ -124,12 +128,14 @@ do_internal_hot_remove_drive_precheck (const char *label) int do_internal_hot_remove_drive (const char *label) { + CLEANUP_FREE char *path = NULL; time_t start_t, now_t; - size_t len = strlen (label); - char path[len+64]; int r; - snprintf (path, len+64, "/dev/disk/guestfs/%s", label); + if (asprintf (&path, "/dev/disk/guestfs/%s", label) == -1) { + reply_with_perror ("asprintf"); + return -1; + } time (&start_t); diff --git a/daemon/luks.c b/daemon/luks.c index a720e25..b8530c5 100644 --- a/daemon/luks.c +++ b/daemon/luks.c @@ -91,9 +91,11 @@ luks_open (const char *device, const char *key, const char *mapname, * that the device-mapper control device (/dev/mapper/control) is * always there, so you can't ever have mapname == "control". */ - size_t len = strlen (mapname); - char devmapper[len+32]; - snprintf (devmapper, len+32, "/dev/mapper/%s", mapname); + CLEANUP_FREE char *devmapper; + if (asprintf (&devmapper, "/dev/mapper/%s", mapname) == -1) { + reply_with_perror ("asprintf"); + return -1; + } if (access (devmapper, F_OK) == 0) { reply_with_error ("%s: device already exists", devmapper); return -1; diff --git a/daemon/lvm.c b/daemon/lvm.c index 529e20d..2b61357 100644 --- a/daemon/lvm.c +++ b/daemon/lvm.c @@ -901,6 +901,8 @@ do_list_dm_devices (void) } while (1) { + CLEANUP_FREE char *devname = NULL; + errno = 0; d = readdir (dir); if (d == NULL) break; @@ -913,10 +915,12 @@ do_list_dm_devices (void) if (STREQ (d->d_name, "control")) continue; - size_t len = strlen (d->d_name); - char devname[len+64]; - - snprintf (devname, len+64, "/dev/mapper/%s", d->d_name); + if (asprintf (&devname, "/dev/mapper/%s", d->d_name) == -1) { + reply_with_perror ("asprintf"); + free_stringslen (ret.argv, ret.size); + closedir (dir); + return NULL; + } /* Ignore dm devices which are LVs. */ r = lv_canonical (devname, NULL); diff --git a/daemon/md.c b/daemon/md.c index 8bb4b54..50b9a81 100644 --- a/daemon/md.c +++ b/daemon/md.c @@ -58,6 +58,11 @@ count_bits (uint64_t bitmap) return c + count_bits (bitmap); } +#if defined(__GNUC__) && GUESTFS_GCC_VERSION >= 40800 /* gcc >= 4.8.0 */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wstack-usage=10000" +#endif + /* Takes optional arguments, consult optargs_bitmask. */ int do_md_create (const char *name, char *const *devices, @@ -177,6 +182,10 @@ do_md_create (const char *name, char *const *devices, return 0; } +#if defined(__GNUC__) && GUESTFS_GCC_VERSION >= 40800 /* gcc >= 4.8.0 */ +#pragma GCC diagnostic pop +#endif + static int glob_errfunc (const char *epath, int eerrno) { diff --git a/daemon/ntfs.c b/daemon/ntfs.c index e555c63..e191a19 100644 --- a/daemon/ntfs.c +++ b/daemon/ntfs.c @@ -273,7 +273,13 @@ do_ntfscat_i (const mountable_t *mountable, int64_t inode) int r; FILE *fp; CLEANUP_FREE char *cmd = NULL; - char buffer[GUESTFS_MAX_CHUNK_SIZE]; + CLEANUP_FREE char *buffer = NULL; + + buffer = malloc (GUESTFS_MAX_CHUNK_SIZE); + if (buffer == NULL) { + reply_with_perror ("malloc"); + return -1; + } /* Inode must be greater than 0 */ if (inode < 0) { @@ -303,7 +309,7 @@ do_ntfscat_i (const mountable_t *mountable, int64_t inode) */ reply (NULL, NULL); - while ((r = fread (buffer, 1, sizeof buffer, fp)) > 0) { + while ((r = fread (buffer, 1, GUESTFS_MAX_CHUNK_SIZE, fp)) > 0) { if (send_file_write (buffer, r) < 0) { pclose (fp); return -1; diff --git a/daemon/ntfsclone.c b/daemon/ntfsclone.c index 732ab79..a239111 100644 --- a/daemon/ntfsclone.c +++ b/daemon/ntfsclone.c @@ -152,7 +152,13 @@ do_ntfsclone_out (const char *device, int r; FILE *fp; CLEANUP_FREE char *cmd = NULL; - char buf[GUESTFS_MAX_CHUNK_SIZE]; + CLEANUP_FREE char *buf = NULL; + + buf = malloc (GUESTFS_MAX_CHUNK_SIZE); + if (buf == NULL) { + reply_with_perror ("malloc"); + return -1; + } /* Construct the ntfsclone command. */ if (asprintf (&cmd, "%s -o - --save-image%s%s%s%s%s %s", @@ -182,7 +188,7 @@ do_ntfsclone_out (const char *device, */ reply (NULL, NULL); - while ((r = fread (buf, 1, sizeof buf, fp)) > 0) { + while ((r = fread (buf, 1, GUESTFS_MAX_CHUNK_SIZE, fp)) > 0) { if (send_file_write (buf, r) < 0) { pclose (fp); return -1; diff --git a/daemon/proto.c b/daemon/proto.c index 3d45dd5..61d376e 100644 --- a/daemon/proto.c +++ b/daemon/proto.c @@ -596,12 +596,19 @@ send_file_end (int cancel) static int send_chunk (const guestfs_chunk *chunk) { - char buf[GUESTFS_MAX_CHUNK_SIZE + 48]; + const size_t buf_len = GUESTFS_MAX_CHUNK_SIZE + 48; + CLEANUP_FREE char *buf = NULL; char lenbuf[4]; XDR xdr; uint32_t len; - xdrmem_create (&xdr, buf, sizeof buf, XDR_ENCODE); + buf = malloc (buf_len); + if (buf == NULL) { + perror ("malloc"); + return -1; + } + + xdrmem_create (&xdr, buf, buf_len, XDR_ENCODE); if (!xdr_guestfs_chunk (&xdr, (guestfs_chunk *) chunk)) { fprintf (stderr, "guestfsd: send_chunk: failed to encode chunk\n"); xdr_destroy (&xdr); diff --git a/daemon/sh.c b/daemon/sh.c index c4efa5b..94f348b 100644 --- a/daemon/sh.c +++ b/daemon/sh.c @@ -160,6 +160,7 @@ static int set_up_etc_resolv_conf (struct resolver_state *rs) { struct stat statbuf; + CLEANUP_FREE char *buf = NULL; rs->sysroot_etc_resolv_conf_old = NULL; @@ -174,11 +175,11 @@ set_up_etc_resolv_conf (struct resolver_state *rs) * that on Ubuntu it's a dangling symlink. */ if (lstat (rs->sysroot_etc_resolv_conf, &statbuf) == 0) { - size_t len = sysroot_len + 32; - char buf[len]; - /* Make a random name for the backup file. */ - snprintf (buf, len, "%s/etc/XXXXXXXX", sysroot); + if (asprintf (&buf, "%s/etc/XXXXXXXX", sysroot) == -1) { + reply_with_perror ("asprintf"); + goto error; + } if (random_name (buf) == -1) { reply_with_perror ("random_name"); goto error; diff --git a/daemon/tar.c b/daemon/tar.c index 255a870..baa5403 100644 --- a/daemon/tar.c +++ b/daemon/tar.c @@ -45,12 +45,17 @@ optgroup_xz_available (void) static int is_chown_supported (const char *dir) { - size_t len = sysroot_len + strlen (dir) + 64; - char buf[len]; + CLEANUP_FREE char *buf = NULL; int fd, r, err, saved_errno; /* Create a randomly named file. */ - snprintf (buf, len, "%s%s/XXXXXXXX.XXX", sysroot, dir); + if (asprintf (&buf, "%s%s/XXXXXXXX.XXX", sysroot, dir) == -1) { + err = errno; + r = cancel_receive (); + errno = err; + reply_with_perror ("asprintf"); + return -1; + } if (random_name (buf) == -1) { err = errno; r = cancel_receive (); @@ -332,7 +337,13 @@ do_tar_out (const char *dir, const char *compress, int numericowner, FILE *fp; CLEANUP_UNLINK_FREE char *exclude_from_file = NULL; CLEANUP_FREE char *cmd = NULL; - char buffer[GUESTFS_MAX_CHUNK_SIZE]; + CLEANUP_FREE char *buffer = NULL; + + buffer = malloc (GUESTFS_MAX_CHUNK_SIZE); + if (buffer == NULL) { + reply_with_perror ("malloc"); + return -1; + } if ((optargs_bitmask & GUESTFS_TAR_OUT_COMPRESS_BITMASK)) { if (STREQ (compress, "compress")) @@ -416,7 +427,7 @@ do_tar_out (const char *dir, const char *compress, int numericowner, */ reply (NULL, NULL); - while ((r = fread (buffer, 1, sizeof buffer, fp)) > 0) { + while ((r = fread (buffer, 1, GUESTFS_MAX_CHUNK_SIZE, fp)) > 0) { if (send_file_write (buffer, r) < 0) { pclose (fp); return -1; diff --git a/daemon/upload.c b/daemon/upload.c index efbb427..bb6da39 100644 --- a/daemon/upload.c +++ b/daemon/upload.c @@ -152,7 +152,13 @@ int do_download (const char *filename) { int fd, r, is_dev; - char buf[GUESTFS_MAX_CHUNK_SIZE]; + CLEANUP_FREE char *buf; + + buf = malloc (GUESTFS_MAX_CHUNK_SIZE); + if (buf == NULL) { + reply_with_perror ("malloc"); + return -1; + } is_dev = STRPREFIX (filename, "/dev/"); @@ -195,7 +201,7 @@ do_download (const char *filename) */ reply (NULL, NULL); - while ((r = read (fd, buf, sizeof buf)) > 0) { + while ((r = read (fd, buf, GUESTFS_MAX_CHUNK_SIZE)) > 0) { if (send_file_write (buf, r) < 0) { close (fd); return -1; @@ -229,7 +235,13 @@ int do_download_offset (const char *filename, int64_t offset, int64_t size) { int fd, r, is_dev; - char buf[GUESTFS_MAX_CHUNK_SIZE]; + CLEANUP_FREE char *buf = NULL; + + buf = malloc (GUESTFS_MAX_CHUNK_SIZE); + if (buf == NULL) { + reply_with_perror ("malloc"); + return -1; + } if (offset < 0) { reply_with_perror ("%s: offset in file is negative", filename); @@ -274,7 +286,8 @@ do_download_offset (const char *filename, int64_t offset, int64_t size) reply (NULL, NULL); while (usize > 0) { - r = read (fd, buf, usize > sizeof buf ? sizeof buf : usize); + r = read (fd, buf, + usize > GUESTFS_MAX_CHUNK_SIZE ? GUESTFS_MAX_CHUNK_SIZE : usize); if (r == -1) { fprintf (stderr, "read: %s: %m\n", filename); send_file_end (1); /* Cancel. */ diff --git a/daemon/zero.c b/daemon/zero.c index 542211c..c545753 100644 --- a/daemon/zero.c +++ b/daemon/zero.c @@ -197,9 +197,15 @@ int do_is_zero (const char *path) { int fd; - char buf[BUFSIZ]; + CLEANUP_FREE char *buf = NULL; ssize_t r; + buf = malloc (BUFSIZ); + if (buf == NULL) { + reply_with_perror ("malloc"); + return -1; + } + CHROOT_IN; fd = open (path, O_RDONLY|O_CLOEXEC); CHROOT_OUT; @@ -209,7 +215,7 @@ do_is_zero (const char *path) return -1; } - while ((r = read (fd, buf, sizeof buf)) > 0) { + while ((r = read (fd, buf, BUFSIZ)) > 0) { if (!is_zero (buf, r)) { close (fd); return 0; @@ -234,16 +240,22 @@ int do_is_zero_device (const char *device) { int fd; - char buf[BUFSIZ]; + CLEANUP_FREE char *buf = NULL; ssize_t r; + buf = malloc (BUFSIZ); + if (buf == NULL) { + reply_with_perror ("malloc"); + return -1; + } + fd = open (device, O_RDONLY|O_CLOEXEC); if (fd == -1) { reply_with_perror ("open: %s", device); return -1; } - while ((r = read (fd, buf, sizeof buf)) > 0) { + while ((r = read (fd, buf, BUFSIZ)) > 0) { if (!is_zero (buf, r)) { close (fd); return 0; @@ -276,8 +288,7 @@ do_is_zero_device (const char *device) int do_zero_free_space (const char *dir) { - size_t len = strlen (dir); - char filename[sysroot_len+len+14]; /* sysroot + dir + "/" + 8.3 + "\0" */ + CLEANUP_FREE char *filename = NULL; int fd; unsigned skip = 0; struct statvfs statbuf; @@ -287,7 +298,10 @@ do_zero_free_space (const char *dir) * this won't conflict with existing files, and it should be * compatible with any filesystem type inc. FAT. */ - snprintf (filename, sysroot_len+len+14, "%s%s/XXXXXXXX.XXX", sysroot, dir); + if (asprintf (&filename, "%s%s/XXXXXXXX.XXX", sysroot, dir) == -1) { + reply_with_perror ("asprintf"); + return -1; + } if (random_name (filename) == -1) { reply_with_perror ("random_name"); return -1; diff --git a/df/domains.c b/df/domains.c index aa0f449..9cb0a09 100644 --- a/df/domains.c +++ b/df/domains.c @@ -75,6 +75,8 @@ get_all_libvirt_domains (const char *libvirt_uri) virErrorPtr err; int n; size_t i; + CLEANUP_FREE int *ids = NULL; + CLEANUP_FREE char **names = NULL; /* Get the list of all domains. */ conn = virConnectOpenAuth (libvirt_uri, virConnectAuthPtrDefault, @@ -96,7 +98,11 @@ get_all_libvirt_domains (const char *libvirt_uri) exit (EXIT_FAILURE); } - int ids[n]; + ids = malloc (sizeof (int) * n); + if (ids == NULL) { + perror ("malloc"); + exit (EXIT_FAILURE); + } n = virConnectListDomains (conn, ids, n); if (n == -1) { err = virGetLastError (); @@ -117,7 +123,11 @@ get_all_libvirt_domains (const char *libvirt_uri) exit (EXIT_FAILURE); } - char *names[n]; + names = malloc (sizeof (char *) * n); + if (names == NULL) { + perror ("malloc"); + exit (EXIT_FAILURE); + } n = virConnectListDefinedDomains (conn, names, n); if (n == -1) { err = virGetLastError (); diff --git a/df/parallel.c b/df/parallel.c index 114cc27..aa37bc3 100644 --- a/df/parallel.c +++ b/df/parallel.c @@ -85,6 +85,8 @@ start_threads (size_t option_P, guestfs_h *options_handle, work_fn work) size_t i, nr_threads; int err, errors; void *status; + CLEANUP_FREE struct thread_data *thread_data = NULL; + CLEANUP_FREE pthread_t *threads = NULL; if (nr_domains == 0) /* Nothing to do. */ return 0; @@ -98,8 +100,10 @@ start_threads (size_t option_P, guestfs_h *options_handle, work_fn work) if (verbose) fprintf (stderr, "parallel: creating %zu threads\n", nr_threads); - struct thread_data thread_data[nr_threads]; - pthread_t threads[nr_threads]; + thread_data = malloc (sizeof (struct thread_data) * nr_threads); + threads = malloc (sizeof (pthread_t) * nr_threads); + if (thread_data == NULL || threads == NULL) + error (EXIT_FAILURE, errno, "malloc"); for (i = 0; i < nr_threads; ++i) { thread_data[i].thread_num = i; diff --git a/erlang/erl-guestfs-proto.c b/erlang/erl-guestfs-proto.c index 658a0ef..0c2f545 100644 --- a/erlang/erl-guestfs-proto.c +++ b/erlang/erl-guestfs-proto.c @@ -201,11 +201,14 @@ ETERM * make_string_list (char **r) { size_t i, size; + ETERM **t; for (size = 0; r[size] != NULL; ++size) ; - ETERM *t[size]; + t = malloc (sizeof (ETERM *) * size); + if (t == NULL) + return make_error ("make_string_list"); for (i = 0; r[i] != NULL; ++i) t[i] = erl_mk_string (r[i]); @@ -220,13 +223,16 @@ ETERM * make_table (char **r) { size_t i, size; - - for (size = 0; r[size] != NULL; ++size) - ; - - ETERM *t[size/2]; + ETERM **t; ETERM *a[2]; + for (size = 0; r[size] != NULL; ++size) + ; + + t = malloc (sizeof (ETERM *) * (size/2)); + if (t == NULL) + return make_error ("make_table"); + for (i = 0; r[i] != NULL; i += 2) { a[0] = erl_mk_string (r[i]); a[1] = erl_mk_string (r[i+1]); diff --git a/examples/mount-local.c b/examples/mount-local.c index 291cb26..8bd6401 100644 --- a/examples/mount-local.c +++ b/examples/mount-local.c @@ -142,8 +142,13 @@ main (int argc, char *argv[]) p = strrchr (shell, '/'); if (p && strcmp (p+1, "bash") == 0) { size_t len = 64 + strlen (shell); - char buf[len]; + char *buf; + buf = malloc (len); + if (buf == NULL) { + perror ("malloc"); + _exit (EXIT_FAILURE); + } snprintf (buf, len, "PS1='mount-local-shell> ' %s --norc -i", shell); r = system (buf); } else diff --git a/fish/events.c b/fish/events.c index 667efdc..0349c55 100644 --- a/fish/events.c +++ b/fish/events.c @@ -99,12 +99,18 @@ do_event_handler (guestfs_h *g, const uint64_t *array, size_t array_len) { pid_t pid; - const char *argv[8 + array_len]; + CLEANUP_FREE const char **argv = NULL; const char *shell; struct entry *entry = opaque; size_t i, j; char *s; + argv = malloc (sizeof (const char *) * (8 + array_len)); + if (argv == NULL) { + perror ("malloc"); + return; + } + pid = fork (); if (pid == -1) { perror ("event handler: fork"); diff --git a/fish/fish.c b/fish/fish.c index d26f8b3..1e29639 100644 --- a/fish/fish.c +++ b/fish/fish.c @@ -1843,9 +1843,15 @@ file_in_heredoc (const char *endmarker) CLEANUP_FREE char *tmpdir = guestfs_get_tmpdir (g), *template = NULL; int fd; size_t markerlen; - char buffer[BUFSIZ]; + CLEANUP_FREE char *buffer = NULL; int write_error = 0; + buffer = malloc (BUFSIZ); + if (buffer == NULL) { + perror ("malloc"); + return NULL; + } + if (asprintf (&template, "%s/guestfishXXXXXX", tmpdir) == -1) { perror ("asprintf"); return NULL; @@ -1865,7 +1871,7 @@ file_in_heredoc (const char *endmarker) markerlen = strlen (endmarker); - while (fgets (buffer, sizeof buffer, stdin) != NULL) { + while (fgets (buffer, BUFSIZ, stdin) != NULL) { /* Look for "END"<EOF> or "END\n" in input. */ size_t blen = strlen (buffer); if (STREQLEN (buffer, endmarker, markerlen) && diff --git a/fish/glob.c b/fish/glob.c index 9fba42e..01b7c6d 100644 --- a/fish/glob.c +++ b/fish/glob.c @@ -37,7 +37,7 @@ static char **expand_devicename (guestfs_h *g, const char *device); static int add_strings_matching (char **pp, const char *glob, char ***ret, size_t *size_r); static int add_string (const char *str, char ***ret, size_t *size_r); static char **single_element_list (const char *element); -static void glob_issue (char *cmd, size_t argc, char ***globs, size_t *posn, size_t *count, int *r); +static int glob_issue (char *cmd, size_t argc, char ***globs, size_t *posn, size_t *count, int *r); int run_glob (const char *cmd, size_t argc, char *argv[]) @@ -52,12 +52,20 @@ run_glob (const char *cmd, size_t argc, char *argv[]) * and then we call every combination (ie. 1x3x3) of * argv[1-]. */ - char **globs[argc]; - size_t posn[argc]; - size_t count[argc]; + CLEANUP_FREE char ***globs = NULL; + CLEANUP_FREE size_t *posn = NULL; + CLEANUP_FREE size_t *count = NULL; size_t i; int r = 0; + globs = malloc (sizeof (char **) * argc); + posn = malloc (sizeof (size_t) * argc); + count = malloc (sizeof (size_t) * argc); + if (globs == NULL || posn == NULL || count == NULL) { + perror ("malloc"); + return -1; + } + if (argc < 1) { fprintf (stderr, _("use 'glob command [args...]'\n")); return -1; @@ -107,7 +115,10 @@ run_glob (const char *cmd, size_t argc, char *argv[]) } /* Issue the commands. */ - glob_issue (argv[0], argc, globs, posn, count, &r); + if (glob_issue (argv[0], argc, globs, posn, count, &r) == -1) { + r = -1; + goto error; + } /* Free resources. */ error: @@ -285,13 +296,19 @@ single_element_list (const char *element) return pp; } -static void +static int glob_issue (char *cmd, size_t argc, char ***globs, size_t *posn, size_t *count, int *r) { size_t i; - char *argv[argc+1]; + CLEANUP_FREE char **argv; + + argv = malloc (sizeof (char *) * (argc+1)); + if (argv == NULL) { + perror ("malloc"); + return -1; + } argv[0] = cmd; argv[argc] = NULL; @@ -310,7 +327,7 @@ glob_issue (char *cmd, size_t argc, posn[i] = 0; } if (i == 0) /* All done. */ - return; + return 0; goto again; } diff --git a/fish/hexedit.c b/fish/hexedit.c index 9f051aa..b8c0a91 100644 --- a/fish/hexedit.c +++ b/fish/hexedit.c @@ -100,7 +100,8 @@ run_hexedit (const char *cmd, size_t argc, char *argv[]) const char *editor; int r; struct stat oldstat, newstat; - char buf[BUFSIZ]; + CLEANUP_FREE char *tmpfd = NULL; + CLEANUP_FREE char *editcmd = NULL; CLEANUP_FREE char *tmpdir = guestfs_get_tmpdir (g), *tmp = NULL; if (asprintf (&tmp, "%s/guestfishXXXXXX", tmpdir) == -1) { @@ -119,9 +120,12 @@ run_hexedit (const char *cmd, size_t argc, char *argv[]) if (editor == NULL) editor = "hexedit"; - snprintf (buf, sizeof buf, "/dev/fd/%d", fd); + if (asprintf (&tmpfd, "/dev/fd/%d", fd) == -1) { + perror ("asprintf"); + return -1; + } - if (guestfs_download_offset (g, filename, buf, start, max) == -1) { + if (guestfs_download_offset (g, filename, tmpfd, start, max) == -1) { unlink (tmp); close (fd); return -1; @@ -140,11 +144,14 @@ run_hexedit (const char *cmd, size_t argc, char *argv[]) } /* Edit it. */ - snprintf (buf, sizeof buf, "%s %s", editor, tmp); + if (asprintf (&editcmd, "%s %s", editor, tmp) == -1) { + perror ("asprintf"); + return -1; + } - r = system (buf); + r = system (editcmd); if (r != 0) { - perror (buf); + perror (editcmd); unlink (tmp); return -1; } diff --git a/generator/erlang.ml b/generator/erlang.ml index 4a18938..a2facec 100644 --- a/generator/erlang.ml +++ b/generator/erlang.ml @@ -227,13 +227,18 @@ extern int64_t get_int64 (ETERM *term); pr "static ETERM *\n"; pr "make_%s_list (const struct guestfs_%s_list *%ss)\n" typ typ typ; pr "{\n"; - pr " ETERM *t[%ss->len];\n" typ; + pr " size_t len = %ss->len;\n" typ; pr " size_t i;\n"; + pr " CLEANUP_FREE ETERM **t;\n"; pr "\n"; - pr " for (i = 0; i < %ss->len; ++i)\n" typ; + pr " t = malloc (sizeof (ETERM *) * len);\n"; + pr " if (t == NULL)\n"; + pr " return make_error (\"make_%s_list\");\n" typ; + pr "\n"; + pr " for (i = 0; i < len; ++i)\n"; pr " t[i] = make_%s (&%ss->val[i]);\n" typ typ; pr "\n"; - pr " return erl_mk_list (t, %ss->len);\n" typ; + pr " return erl_mk_list (t, len);\n"; pr "}\n"; pr "\n"; in diff --git a/generator/java.ml b/generator/java.ml index 2a41a5f..aa7ed04 100644 --- a/generator/java.ml +++ b/generator/java.ml @@ -1218,7 +1218,11 @@ and generate_java_struct_return typ jtyp cols = | name, FBuffer -> pr " {\n"; pr " size_t len = r->%s_len;\n" name; - pr " char s[len+1];\n"; + pr " CLEANUP_FREE char *s = malloc (len + 1);\n"; + pr " if (s == NULL) {\n"; + pr " throw_out_of_memory (env, \"malloc\");\n"; + pr " goto ret_error;\n"; + pr " }\n"; pr " memcpy (s, r->%s, len);\n" name; pr " s[len] = 0;\n"; pr " fl = (*env)->GetFieldID (env, cl, \"%s\", \"Ljava/lang/String;\");\n" name; @@ -1275,7 +1279,11 @@ and generate_java_struct_list_return typ jtyp cols = | FBuffer -> pr " {\n"; pr " size_t len = r->val[i].%s_len;\n" name; - pr " char s[len+1];\n"; + pr " CLEANUP_FREE char *s = malloc (len);\n"; + pr " if (s == NULL) {\n"; + pr " throw_out_of_memory (env, \"malloc\");\n"; + pr " goto ret_error;\n"; + pr " }\n"; pr " memcpy (s, r->val[i].%s, len);\n" name; pr " s[len] = 0;\n"; pr " (*env)->SetObjectField (env, jfl, fl,\n"; diff --git a/m4/guestfs_c.m4 b/m4/guestfs_c.m4 index 002c32f..7be006d 100644 --- a/m4/guestfs_c.m4 +++ b/m4/guestfs_c.m4 @@ -87,9 +87,13 @@ gl_WARN_ADD([-fdiagnostics-show-option]) dnl Now some warnings we want to enable and/or customize ... -dnl Warn about large stack allocations. 10000 happens to be the -dnl same size as Coverity warns about. -gl_WARN_ADD([-Wframe-larger-than=10000]) +dnl Warn about large stack frames. This does not include alloca and +dnl variable length arrays. Coverity warns about 10000 byte frames. +gl_WARN_ADD([-Wframe-larger-than=5000]) + +dnl Warn about large stack frames, including estimates for alloca +dnl and variable length arrays. +gl_WARN_ADD([-Wstack-usage=10000]) AC_SUBST([WARN_CFLAGS]) diff --git a/make-fs/make-fs.c b/make-fs/make-fs.c index 561c6ae..2778772 100644 --- a/make-fs/make-fs.c +++ b/make-fs/make-fs.c @@ -281,9 +281,15 @@ exec_command_count_output (char **argv, uint64_t *bytes_rtn) pid_t pid; int status; int fd[2]; - char buffer[BUFSIZ]; + CLEANUP_FREE char *buffer = NULL; ssize_t r; + buffer = malloc (BUFSIZ); + if (buffer == NULL) { + perror ("malloc"); + return -1; + } + if (pipe (fd) == -1) { perror ("pipe"); return -1; @@ -297,7 +303,7 @@ exec_command_count_output (char **argv, uint64_t *bytes_rtn) close (fd[1]); /* Read output from the subprocess and count the length. */ - while ((r = read (fd[0], buffer, sizeof buffer)) > 0) { + while ((r = read (fd[0], buffer, BUFSIZ)) > 0) { *bytes_rtn += r; } if (r == -1) { diff --git a/p2v/conversion.c b/p2v/conversion.c index f5c518a..8e90ef5 100644 --- a/p2v/conversion.c +++ b/p2v/conversion.c @@ -167,7 +167,7 @@ start_conversion (struct config *config, int status; size_t i, len; size_t nr_disks = guestfs_int_count_strings (config->disks); - struct data_conn data_conns[nr_disks]; + CLEANUP_FREE struct data_conn *data_conns; CLEANUP_FREE char *remote_dir = NULL, *libvirt_xml = NULL; time_t now; struct tm tm; @@ -184,6 +184,12 @@ start_conversion (struct config *config, set_running (1); set_cancel_requested (0); + data_conns = malloc (sizeof (struct data_conn) * nr_disks); + if (data_conns == NULL) { + perror ("malloc"); + exit (EXIT_FAILURE); + } + for (i = 0; config->disks[i] != NULL; ++i) { data_conns[i].h = NULL; data_conns[i].nbd_pid = 0; diff --git a/src/appliance.c b/src/appliance.c index dbde35e..d7cc60b 100644 --- a/src/appliance.c +++ b/src/appliance.c @@ -39,8 +39,8 @@ static const char *initrd_name = "initramfs." host_cpu ".img"; static int build_appliance (guestfs_h *g, char **kernel, char **initrd, char **appliance); static int find_path (guestfs_h *g, int (*pred) (guestfs_h *g, const char *pelem, void *data), void *data, char **pelem); -static int dir_contains_file (const char *dir, const char *file); -static int dir_contains_files (const char *dir, ...); +static int dir_contains_file (guestfs_h *g, const char *dir, const char *file); +static int dir_contains_files (guestfs_h *g, const char *dir, ...); static int contains_old_style_appliance (guestfs_h *g, const char *path, void *data); static int contains_fixed_appliance (guestfs_h *g, const char *path, void *data); static int contains_supermin_appliance (guestfs_h *g, const char *path, void *data); @@ -170,13 +170,13 @@ build_appliance (guestfs_h *g, static int contains_old_style_appliance (guestfs_h *g, const char *path, void *data) { - return dir_contains_files (path, kernel_name, initrd_name, NULL); + return dir_contains_files (g, path, kernel_name, initrd_name, NULL); } static int contains_fixed_appliance (guestfs_h *g, const char *path, void *data) { - return dir_contains_files (path, + return dir_contains_files (g, path, "README.fixed", "kernel", "initrd", "root", NULL); } @@ -184,7 +184,7 @@ contains_fixed_appliance (guestfs_h *g, const char *path, void *data) static int contains_supermin_appliance (guestfs_h *g, const char *path, void *data) { - return dir_contains_files (path, "supermin.d", NULL); + return dir_contains_files (g, path, "supermin.d", NULL); } /* Build supermin appliance from supermin_path to $TMPDIR/.guestfs-$UID. @@ -201,19 +201,12 @@ build_supermin_appliance (guestfs_h *g, char **appliance) { CLEANUP_FREE char *tmpdir = guestfs_get_cachedir (g); + CLEANUP_FREE char *cachedir = NULL, *lockfile = NULL, *appliancedir = NULL; struct stat statbuf; - size_t len; - /* len must be longer than the length of any pathname we can - * generate in this function. - */ - len = strlen (tmpdir) + 128; - char cachedir[len]; - snprintf (cachedir, len, "%s/.guestfs-%ju", tmpdir, (uintmax_t) uid); - char lockfile[len]; - snprintf (lockfile, len, "%s/lock", cachedir); - char appliancedir[len]; - snprintf (appliancedir, len, "%s/appliance.d", cachedir); + cachedir = safe_asprintf (g, "%s/.guestfs-%ju", tmpdir, (uintmax_t) uid); + lockfile = safe_asprintf (g, "%s/lock", cachedir); + appliancedir = safe_asprintf (g, "%s/appliance.d", cachedir); ignore_value (mkdir (cachedir, 0755)); ignore_value (chmod (cachedir, 0755)); /* RHBZ#921292 */ @@ -254,12 +247,9 @@ build_supermin_appliance (guestfs_h *g, guestfs_int_print_timestamped_message (g, "finished building supermin appliance"); /* Return the appliance filenames. */ - *kernel = safe_malloc (g, len); - *initrd = safe_malloc (g, len); - *appliance = safe_malloc (g, len); - snprintf (*kernel, len, "%s/kernel", appliancedir); - snprintf (*initrd, len, "%s/initrd", appliancedir); - snprintf (*appliance, len, "%s/root", appliancedir); + *kernel = safe_asprintf (g, "%s/kernel", appliancedir); + *initrd = safe_asprintf (g, "%s/initrd", appliancedir); + *appliance = safe_asprintf (g, "%s/root", appliancedir); /* Touch the files so they don't get deleted (as they are in /var/tmp). */ (void) utimes (*kernel, NULL); @@ -396,27 +386,24 @@ find_path (guestfs_h *g, /* Returns true iff file is contained in dir. */ static int -dir_contains_file (const char *dir, const char *file) +dir_contains_file (guestfs_h *g, const char *dir, const char *file) { - size_t dirlen = strlen (dir); - size_t filelen = strlen (file); - size_t len = dirlen + filelen + 2; - char path[len]; + CLEANUP_FREE char *path; - snprintf (path, len, "%s/%s", dir, file); + path = safe_asprintf (g, "%s/%s", dir, file); return access (path, F_OK) == 0; } /* Returns true iff every listed file is contained in 'dir'. */ static int -dir_contains_files (const char *dir, ...) +dir_contains_files (guestfs_h *g, const char *dir, ...) { va_list args; const char *file; va_start (args, dir); while ((file = va_arg (args, const char *)) != NULL) { - if (!dir_contains_file (dir, file)) { + if (!dir_contains_file (g, dir, file)) { va_end (args); return 0; } diff --git a/src/command.c b/src/command.c index e9f357a..866847d 100644 --- a/src/command.c +++ b/src/command.c @@ -607,7 +607,7 @@ loop (struct command *cmd) fd_set rset, rset2; int maxfd = -1, r; size_t nr_fds = 0; - char buf[BUFSIZ]; + CLEANUP_FREE char *buf = safe_malloc (cmd->g, BUFSIZ); ssize_t n; FD_ZERO (&rset); @@ -636,7 +636,7 @@ loop (struct command *cmd) if (cmd->errorfd >= 0 && FD_ISSET (cmd->errorfd, &rset2)) { /* Read output and send it to the log. */ - n = read (cmd->errorfd, buf, sizeof buf); + n = read (cmd->errorfd, buf, BUFSIZ); if (n > 0) guestfs_int_call_callbacks_message (cmd->g, GUESTFS_EVENT_APPLIANCE, buf, n); @@ -660,7 +660,7 @@ loop (struct command *cmd) /* Read the output, buffer it up to the end of the line, then * pass it to the callback. */ - n = read (cmd->outfd, buf, sizeof buf); + n = read (cmd->outfd, buf, BUFSIZ); if (n > 0) { if (cmd->outbuf.add_data) cmd->outbuf.add_data (cmd, buf, n); diff --git a/src/conn-socket.c b/src/conn-socket.c index 05177ca..5b6b80e 100644 --- a/src/conn-socket.c +++ b/src/conn-socket.c @@ -312,7 +312,7 @@ static int handle_log_message (guestfs_h *g, struct connection_socket *conn) { - char buf[BUFSIZ]; + CLEANUP_FREE char *buf = safe_malloc (g, BUFSIZ); ssize_t n; /* Carried over from ancient proto.c code. The comment there was: @@ -329,7 +329,7 @@ handle_log_message (guestfs_h *g, */ usleep (1000); - n = read (conn->console_sock, buf, sizeof buf); + n = read (conn->console_sock, buf, BUFSIZ); if (n == 0) return 0; diff --git a/src/copy-in-out.c b/src/copy-in-out.c index 5059d8d..50831be 100644 --- a/src/copy-in-out.c +++ b/src/copy-in-out.c @@ -43,7 +43,7 @@ guestfs_impl_copy_in (guestfs_h *g, int r; char fdbuf[64]; size_t buf_len = strlen (localpath) + 1; - char buf[buf_len]; + CLEANUP_FREE char *buf = safe_malloc (g, buf_len); const char *dirname, *basename; struct stat statbuf; @@ -153,7 +153,7 @@ guestfs_impl_copy_out (guestfs_h *g, if (r == 1) { /* is file */ CLEANUP_FREE char *filename = NULL; size_t buf_len = strlen (remotepath) + 1; - char buf[buf_len]; + CLEANUP_FREE char *buf = safe_malloc (g, buf_len); const char *basename; if (split_path (g, buf, buf_len, remotepath, NULL, &basename) == -1) @@ -181,7 +181,7 @@ guestfs_impl_copy_out (guestfs_h *g, } size_t buf_len = strlen (remotepath) + 1; - char buf[buf_len]; + CLEANUP_FREE char *buf = safe_malloc (g, buf_len); const char *basename; if (split_path (g, buf, buf_len, remotepath, NULL, &basename) == -1) return -1; diff --git a/src/errors.c b/src/errors.c index 6162083..c85aed5 100644 --- a/src/errors.c +++ b/src/errors.c @@ -328,7 +328,7 @@ guestfs_int_external_command_failed (guestfs_h *g, int status, const char *cmd_name, const char *extra) { size_t len = 80 + strlen (cmd_name); - char status_string[len]; + CLEANUP_FREE char *status_string = safe_malloc (g, len); guestfs_int_exit_status_to_string (status, cmd_name, status_string, len); diff --git a/src/inspect-apps.c b/src/inspect-apps.c index b54cf07..71efa75 100644 --- a/src/inspect-apps.c +++ b/src/inspect-apps.c @@ -660,14 +660,11 @@ static void list_applications_windows_from_path (guestfs_h *g, struct guestfs_ap static struct guestfs_application2_list * list_applications_windows (guestfs_h *g, struct inspect_fs *fs) { - size_t len = strlen (fs->windows_systemroot) + 64; - char software[len]; + CLEANUP_FREE char *software = + safe_asprintf (g, "%s/system32/config/software", fs->windows_systemroot); CLEANUP_FREE char *software_path; struct guestfs_application2_list *ret = NULL; - snprintf (software, len, "%s/system32/config/software", - fs->windows_systemroot); - software_path = guestfs_case_sensitive_path (g, software); if (!software_path) return NULL; diff --git a/src/inspect-fs-unix.c b/src/inspect-fs-unix.c index c209fc3..4120d31 100644 --- a/src/inspect-fs-unix.c +++ b/src/inspect-fs-unix.c @@ -231,8 +231,8 @@ parse_os_release (guestfs_h *g, struct inspect_fs *fs, const char *filename) if (minor_version == -1) return -1; } else { - char buf[value_len + 1]; - snprintf (buf, sizeof buf, "%*s", (int) value_len, value); + CLEANUP_FREE char *buf = + safe_asprintf (g, "%*s", (int) value_len, value); major_version = guestfs_int_parse_unsigned_int (g, buf); /* Handle cases where VERSION_ID is not a number. */ if (major_version != -1) diff --git a/src/inspect-fs-windows.c b/src/inspect-fs-windows.c index a0e6feb..3ac9107 100644 --- a/src/inspect-fs-windows.c +++ b/src/inspect-fs-windows.c @@ -232,9 +232,8 @@ guestfs_int_check_windows_root (guestfs_h *g, struct inspect_fs *fs, static int check_windows_arch (guestfs_h *g, struct inspect_fs *fs) { - size_t len = strlen (fs->windows_systemroot) + 32; - char cmd_exe[len]; - snprintf (cmd_exe, len, "%s/system32/cmd.exe", fs->windows_systemroot); + CLEANUP_FREE char *cmd_exe = + safe_asprintf (g, "%s/system32/cmd.exe", fs->windows_systemroot); /* Should exist because of previous check above in get_windows_systemroot. */ CLEANUP_FREE char *cmd_exe_path = guestfs_case_sensitive_path (g, cmd_exe); @@ -260,10 +259,8 @@ check_windows_software_registry (guestfs_h *g, struct inspect_fs *fs) int ret = -1; int r; - size_t len = strlen (fs->windows_systemroot) + 64; - char software[len]; - snprintf (software, len, "%s/system32/config/software", - fs->windows_systemroot); + CLEANUP_FREE char *software = + safe_asprintf (g, "%s/system32/config/software", fs->windows_systemroot); CLEANUP_FREE char *software_path = guestfs_case_sensitive_path (g, software); if (!software_path) @@ -387,11 +384,11 @@ static int check_windows_system_registry (guestfs_h *g, struct inspect_fs *fs) { int r; - size_t len = strlen (fs->windows_systemroot) + 64; - char system[len]; static const char gpt_prefix[] = "DMIO:ID:"; - snprintf (system, len, "%s/system32/config/system", - fs->windows_systemroot); + + CLEANUP_FREE char *system = + safe_asprintf (g, "%s/system32/config/system", + fs->windows_systemroot); CLEANUP_FREE char *system_path = guestfs_case_sensitive_path (g, system); if (!system_path) @@ -495,6 +492,7 @@ check_windows_system_registry (guestfs_h *g, struct inspect_fs *fs) char *device; int64_t type; bool is_gpt; + size_t len; type = guestfs_hivex_value_type (g, v); blob = guestfs_hivex_value_value (g, v, &len); diff --git a/src/osinfo.c b/src/osinfo.c index b1cfa09..4a4cbfc 100644 --- a/src/osinfo.c +++ b/src/osinfo.c @@ -229,9 +229,7 @@ static int read_os_node (guestfs_h *g, xmlXPathContextPtr xpathCtx, xmlNodePtr o static int read_osinfo_db_xml (guestfs_h *g, const char *filename) { - const size_t pathname_len = - strlen (LIBOSINFO_DB_OS_PATH) + strlen (filename) + 2; - char pathname[pathname_len]; + CLEANUP_FREE char *pathname = NULL; CLEANUP_XMLFREEDOC xmlDocPtr doc = NULL; CLEANUP_XMLXPATHFREECONTEXT xmlXPathContextPtr xpathCtx = NULL; CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpathObj = NULL; @@ -240,7 +238,7 @@ read_osinfo_db_xml (guestfs_h *g, const char *filename) struct osinfo *osinfo; size_t i; - snprintf (pathname, pathname_len, "%s/%s", LIBOSINFO_DB_OS_PATH, filename); + pathname = safe_asprintf (g, "%s/%s", LIBOSINFO_DB_OS_PATH, filename); doc = xmlReadFile (pathname, NULL, XML_PARSE_NONET); if (doc == NULL) { diff --git a/src/proto.c b/src/proto.c index 5ac7e67..5213856 100644 --- a/src/proto.c +++ b/src/proto.c @@ -316,7 +316,7 @@ static int send_file_complete (guestfs_h *g); int guestfs_int_send_file (guestfs_h *g, const char *filename) { - char buf[GUESTFS_MAX_CHUNK_SIZE]; + CLEANUP_FREE char *buf = safe_malloc (g, GUESTFS_MAX_CHUNK_SIZE); int fd, r = 0, err; g->user_cancel = 0; @@ -332,7 +332,7 @@ guestfs_int_send_file (guestfs_h *g, const char *filename) /* Send file in chunked encoding. */ while (!g->user_cancel) { - r = read (fd, buf, sizeof buf); + r = read (fd, buf, GUESTFS_MAX_CHUNK_SIZE); if (r == -1 && (errno == EINTR || errno == EAGAIN)) continue; if (r <= 0) break; diff --git a/tests/qemu/qemu-boot.c b/tests/qemu/qemu-boot.c index 446ae4b..94d2f97 100644 --- a/tests/qemu/qemu-boot.c +++ b/tests/qemu/qemu-boot.c @@ -67,6 +67,7 @@ struct thread_data { int r; }; +static void run_test (size_t P); static void *start_thread (void *thread_data_vp); static void message_callback (guestfs_h *g, void *opaque, uint64_t event, int event_handle, int flags, const char *buf, size_t buf_len, const uint64_t *array, size_t array_len); @@ -102,10 +103,8 @@ main (int argc, char *argv[]) { "verbose", 0, 0, 'v' }, { 0, 0, 0, 0 } }; - size_t P = 0, i, errors; + size_t P = 0, i; int c, option_index; - int err; - void *status; for (;;) { c = getopt_long (argc, argv, options, long_options, &option_index); @@ -183,10 +182,27 @@ main (int argc, char *argv[]) else P = MIN (n, MIN (MAX_THREADS, estimate_max_threads ())); + run_test (P); + exit (EXIT_SUCCESS); +} + +static void +run_test (size_t P) +{ + void *status; + int err; + size_t i, errors; + CLEANUP_FREE struct thread_data *thread_data = NULL; + CLEANUP_FREE pthread_t *threads = NULL; + + thread_data = malloc (sizeof (struct thread_data) * P); + threads = malloc (sizeof (pthread_t) * P); + if (thread_data == NULL || threads == NULL) { + perror ("malloc"); + exit (EXIT_FAILURE); + } + /* Start the worker threads. */ - struct thread_data thread_data[P]; - pthread_t threads[P]; - for (i = 0; i < P; ++i) { thread_data[i].thread_num = i; err = pthread_create (&threads[i], NULL, start_thread, &thread_data[i]); @@ -210,7 +226,8 @@ main (int argc, char *argv[]) errors++; } - exit (errors == 0 ? EXIT_SUCCESS : EXIT_FAILURE); + if (errors > 0) + exit (EXIT_FAILURE); } /* Worker thread. */ -- 2.5.0