fish: handle some out-of-memory conditions

* fish/destpaths.c (xalloc_oversized): Define.
(complete_dest_paths_generator): Use size_t as type for a few
variables, rather than int.
Don't deref NULL or undef on failed heap alloc.
Don't leak on failed realloc.
Detect theoretical overflow when count_strings returns a very
large number of strings.
Handle asprintf failure.
(APPEND_STRS_AND_FREE): Rewrite as do {...}while(0), so that each use
can/must be followed by a semicolon.  Better for auto-formatters.

diff --git a/fish/destpaths.c b/fish/destpaths.c
index 6cddafa..90970de 100644 (file)
--- a/fish/destpaths.c
+++ b/fish/destpaths.c
@@ -1,5 +1,5 @@
 /* guestfish - the filesystem interactive shell
- * Copyright (C) 2009 Red Hat Inc. 
+ * Copyright (C) 2009 Red Hat Inc.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -22,6 +22,7 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <stddef.h>
 #include <string.h>
 
 #ifdef HAVE_LIBREADLINE
@@ -32,6 +33,22 @@
 
 #include "fish.h"
 
+// From gnulib's xalloc.h:
+/* Return 1 if an array of N objects, each of size S, cannot exist due
+   to size arithmetic overflow.  S must be positive and N must be
+   nonnegative.  This is a macro, not an inline function, so that it
+   works correctly even when SIZE_MAX < N.
+
+   By gnulib convention, SIZE_MAX represents overflow in size
+   calculations, so the conservative dividend to use here is
+   SIZE_MAX - 1, since SIZE_MAX might represent an overflowed value.
+   However, malloc (SIZE_MAX) fails on all known hosts where
+   sizeof (ptrdiff_t) <= sizeof (size_t), so do not bother to test for
+   exactly-SIZE_MAX allocations on such hosts; this avoids a test and
+   branch when S is known to be 1.  */
+# define xalloc_oversized(n, s) \
+    ((size_t) (sizeof (ptrdiff_t) <= sizeof (size_t) ? -1 : -2) / (s) < (n))
+
 /* Readline completion for paths on the guest filesystem, also for
  * devices and LVM names.
  */
@@ -53,9 +70,9 @@ complete_dest_paths_generator (const char *text, int state)
 {
 #ifdef HAVE_LIBREADLINE
 
-  static int len, index;
+  static size_t len, index;
   static char **words = NULL;
-  static int nr_words = 0;
+  static size_t nr_words = 0;
   char *word;
   guestfs_error_handler_cb old_error_cb;
   void *old_error_cb_data;
@@ -73,12 +90,12 @@ complete_dest_paths_generator (const char *text, int state)
 
   if (!state) {
     char **strs;
-    int i, n;
 
     len = strlen (text);
     index = 0;
 
     if (words) {
+      size_t i;
       /* NB. 'words' array is NOT NULL-terminated. */
       for (i = 0; i < nr_words; ++i)
        free (words[i]);
@@ -90,26 +107,38 @@ complete_dest_paths_generator (const char *text, int state)
 
     SAVE_ERROR_CB
 
+/* Silently do nothing if an allocation fails */
 #define APPEND_STRS_AND_FREE                                           \
+  do {                                                                 \
     if (strs) {                                                                \
-      n = count_strings (strs);                                                \
-      words = realloc (words, sizeof (char *) * (nr_words + n));       \
-      for (i = 0; i < n; ++i)                                          \
-       words[nr_words++] = strs[i];                                    \
-      free (strs);                                                     \
-    }
+      size_t n = count_strings (strs);                                 \
+      if ( ! xalloc_oversized (nr_words + n, sizeof (char *))) {       \
+       char *w = realloc (words, sizeof (char *) * (nr_words + n));    \
+       if (w == NULL) {                                                \
+         free (words);                                                 \
+         words = NULL;                                                 \
+         nr_words = 0;                                                 \
+       } else {                                                        \
+         size_t i;                                                     \
+         for (i = 0; i < n; ++i)                                       \
+           words[nr_words++] = strs[i];                                \
+       }                                                               \
+       free (strs);                                                    \
+      }                                                                        \
+    }                                                                  \
+  } while (0)
 
     /* Is it a device? */
     if (len < 5 || strncmp (text, "/dev/", 5) == 0) {
       /* Get a list of everything that can possibly begin with /dev/ */
       strs = guestfs_list_devices (g);
-      APPEND_STRS_AND_FREE
+      APPEND_STRS_AND_FREE;
 
       strs = guestfs_list_partitions (g);
-      APPEND_STRS_AND_FREE
+      APPEND_STRS_AND_FREE;
 
       strs = guestfs_lvs (g);
-      APPEND_STRS_AND_FREE
+      APPEND_STRS_AND_FREE;
     }
 
     if (len < 1 || text[0] == '/') {
@@ -120,24 +149,28 @@ complete_dest_paths_generator (const char *text, int state)
 
       p = strrchr (text, '/');
       dir = p && p > text ? strndup (text, p - text) : strdup ("/");
-
-      strs = guestfs_ls (g, dir);
-
-      /* Prepend directory to names. */
-      if (strs) {
-       for (i = 0; strs[i]; ++i) {
-         p = NULL;
-         if (strcmp (dir, "/") == 0)
-           asprintf (&p, "/%s", strs[i]);
-         else
-           asprintf (&p, "%s/%s", dir, strs[i]);
-         free (strs[i]);
-         strs[i] = p;
+      if (dir) {
+       strs = guestfs_ls (g, dir);
+
+       /* Prepend directory to names. */
+       if (strs) {
+         size_t i;
+         for (i = 0; strs[i]; ++i) {
+           int err;
+           if (strcmp (dir, "/") == 0)
+             err = asprintf (&p, "/%s", strs[i]);
+           else
+             err = asprintf (&p, "%s/%s", dir, strs[i]);
+           if (0 <= err) {
+             free (strs[i]);
+             strs[i] = p;
+           }
+         }
        }
-      }
 
-      free (dir);
-      APPEND_STRS_AND_FREE
+       free (dir);
+       APPEND_STRS_AND_FREE;
+      }
     }
 
     /* else ...  In theory we could complete other things here such as VG