From 07c496c53c7835c0db882ef6761309db4952ebf6 Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Sun, 6 Mar 2016 18:37:40 +0000 Subject: [PATCH] Use less stack. 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 | 22 ++++++++++++++---- 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 | 12 +++++++--- examples/mount-local.c | 8 ++++++- fish/events.c | 8 ++++++- fish/fish.c | 10 ++++++-- fish/glob.c | 33 +++++++++++++++++++------- fish/hexedit.c | 16 ++++++++----- 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, 507 insertions(+), 205 deletions(-) diff --git a/builder/index-parse.y b/builder/index-parse.y index 82ea9d291..ffc3f50f8 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=" +#pragma GCC diagnostic ignored "-Wstack-usage=" +#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 4b49e744b..1f5ceebcd 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 5dfe1b647..2e08cccf4 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 d50c737e1..971679626 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 (®ex, "^[[: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 35a5d2fa8..3f7a63054 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 3155a7438..2a20cb01e 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=" +#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=" +#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=" +#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 eec45e2da..304594868 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 b00be3e37..36f4e66f7 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 6f3e84429..2a6720bef 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 e33e47bd7..d1459b15d 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 d29c52778..d0390d8b9 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 = NULL; + 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 5637939c1..dac64820f 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 = 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] = "-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 9ba4f095b..5dd67c7e0 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 a9a3aa786..2d19df5db 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 = NULL; 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 = NULL; + + 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 a603054a0..b47caa887 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 a5047cb89..f44137038 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 4bc5fd449..f4e0e560a 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,11 +1000,15 @@ parse_btrfsvol (const char *desc_orig, mountable_t *mountable) mountable->type = MOUNTABLE_BTRFSVOL; - memcpy (desc, desc_orig, len+1); - - if (! STRPREFIX (desc, "/dev/")) + if (!STRPREFIX (desc_orig, "/dev/")) return -1; + desc = strdup (desc_orig); + if (desc == NULL) { + perror ("strdup"); + return -1; + } + slash = desc + strlen ("/dev/") - 1; while ((slash = strchr (slash + 1, '/'))) { *slash = '\0'; @@ -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=" +#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 2f159c635..234f51ed2 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 a720e2539..53bb820d6 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 = NULL; + 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 529e20d75..2b6135745 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 8bb4b5473..5d60b62ba 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=" +#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 e555c63c8..e191a197c 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 732ab798c..a2391110a 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 3d45dd5d4..61d376e5f 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 c4efa5b7b..94f348b31 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 255a87091..baa5403c1 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 efbb42718..aeb730666 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 = NULL; + + 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 542211cdd..c54575350 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 aa0f44940..9cb0a091b 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 114cc2765..aa37bc334 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 658a0eff9..24295ef68 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; + CLEANUP_FREE ETERM **t = NULL; 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,12 +223,15 @@ ETERM * make_table (char **r) { size_t i, size; + CLEANUP_FREE ETERM **t = NULL; + ETERM *a[2]; for (size = 0; r[size] != NULL; ++size) ; - ETERM *t[size/2]; - ETERM *a[2]; + 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]); diff --git a/examples/mount-local.c b/examples/mount-local.c index 291cb2680..ce12ffe9b 100644 --- a/examples/mount-local.c +++ b/examples/mount-local.c @@ -142,10 +142,16 @@ 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); + free (buf); } else r = system (shell); } diff --git a/fish/events.c b/fish/events.c index 667efdcaf..0349c5544 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 d26f8b3ab..1e2963974 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" 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 9fba42e4e..5bf1ad654 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 = NULL; + + 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 9f051aa1f..e30352f57 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]; + char tmpfd[sizeof "/dev/fd/" + 3 * sizeof (int)]; + 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,9 @@ run_hexedit (const char *cmd, size_t argc, char *argv[]) if (editor == NULL) editor = "hexedit"; - snprintf (buf, sizeof buf, "/dev/fd/%d", fd); + snprintf (tmpfd, sizeof tmpfd, "/dev/fd/%d", fd); - 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 +141,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 4a18938b3..a2facec49 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 2a41a5f2b..aa7ed044e 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 002c32f95..7be006d92 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 561c6ae31..2778772b9 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 f5c518a0d..d076718d0 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 = NULL; 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 dbde35e93..de088e9e5 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 = NULL; - 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 e9f357a95..866847dd8 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 05177ca63..5b6b80e2c 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 5059d8d75..50831bec3 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 616208386..c85aed54a 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 78c32bfd4..e53d87c0d 100644 --- a/src/inspect-apps.c +++ b/src/inspect-apps.c @@ -781,14 +781,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 c209fc3d8..4120d31cf 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 a0e6febc2..3ac9107af 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 b1cfa091c..4a4cbfca6 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 5ac7e6728..521385682 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 446ae4b43..94d2f971a 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 ())); - /* Start the worker threads. */ - struct thread_data thread_data[P]; - pthread_t threads[P]; + 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. */ 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. */