More error checking; improve gdbm_recover

* Makefile.am (set-dist-date): New rule
(dist-hook): Catch FIXMEs in NEWS.
* NEWS: Updated.
* src/findkey.c (gdbm_bucket_element_valid_p): New function.
(_gdbm_read_entry): Validate the retrieved bucket element.
* src/gdbm.h.in (gdbm_recovery): New member: duplicate_keys.
(GDBM_BAD_HASH_TABLE): New error code.
* src/gdbmdefs.h (TYPE_WIDTH,SIGNED_TYPE_MAXIMUM)
(OFF_T_MAX): New defines.
(off_t_sum_ok): New function.
(gdbm_bucket_element_valid_p): New prototype.
* src/gdbmerrno.c: Support for GDBM_BAD_HASH_TABLE code.
* src/gdbmtool.c (recover_handler): Fix argument counting.
New argument 'summary' prints statistics summary at the end
of the run.
(export_handler,import_handler): Fix argument counting.
* src/mmap.c (SUM_FILE_SIZE): Rewrite as inlined function.
Add error checking.
(_gdbm_mapped_remap): More error checking.
* src/recover.c (run_recovery): Don't bail out on GDBM_CANNOT_REPLACE.
(gdbm_recover): Initialize duplicate_keys
* src/systems.h: Include limits.h

8 files changed, 102 insertions, 8 deletions

diff --git a/src/findkey.c b/src/findkey.c
index 7638b04..bd9fd83 100644
--- a/src/findkey.c
+++ b/src/findkey.c
@@ -19,12 +19,26 @@
 
 /* Include system configuration before all else. */
 #include "autoconf.h"
 
 #include "gdbmdefs.h"
 
+int
+gdbm_bucket_element_valid_p (GDBM_FILE dbf, int elem_loc)
+{
+  return
+    elem_loc < dbf->header->bucket_elems
+    && dbf->bucket->h_table[elem_loc].hash_value != -1
+    && dbf->bucket->h_table[elem_loc].key_size >= 0
+    && off_t_sum_ok (dbf->bucket->h_table[elem_loc].data_pointer,
+                     dbf->bucket->h_table[elem_loc].key_size)
+    && dbf->bucket->h_table[elem_loc].data_size >= 0
+    && off_t_sum_ok (dbf->bucket->h_table[elem_loc].data_pointer
+                     + dbf->bucket->h_table[elem_loc].key_size,
+                     dbf->bucket->h_table[elem_loc].data_size);
+}
 
 /* Read the data found in bucket entry ELEM_LOC in file DBF and
    return a pointer to it.  Also, cache the read value. */
 
 char *
 _gdbm_read_entry (GDBM_FILE dbf, int elem_loc)
@@ -36,12 +50,18 @@ _gdbm_read_entry (GDBM_FILE dbf, int elem_loc)
   data_cache_elem *data_ca;
   
   /* Is it already in the cache? */
   if (dbf->cache_entry->ca_data.elem_loc == elem_loc)
     return dbf->cache_entry->ca_data.dptr;
 
+  if (!gdbm_bucket_element_valid_p (dbf, elem_loc))
+    {
+      GDBM_SET_ERRNO (dbf, GDBM_BAD_HASH_TABLE, TRUE);
+      return NULL;
+    }
+  
   /* Set sizes and pointers. */
   key_size = dbf->bucket->h_table[elem_loc].key_size;
   data_size = dbf->bucket->h_table[elem_loc].data_size;
   data_ca = &dbf->cache_entry->ca_data;
   
   /* Set up the cache. */
diff --git a/src/gdbm.h.in b/src/gdbm.h.in
index 61d5707..e576c69 100644
--- a/src/gdbm.h.in
+++ b/src/gdbm.h.in
@@ -149,12 +149,13 @@ typedef struct gdbm_recovery_s
   /* Output members.
      The gdbm_recover function fills these before returning. */
   size_t recovered_keys;
   size_t recovered_buckets;
   size_t failed_keys;
   size_t failed_buckets;
+  size_t duplicate_keys;
   char *backup_name;
 } gdbm_recovery;
 
 #define GDBM_RCVR_DEFAULT              0x00  /* Default settings */
 #define GDBM_RCVR_ERRFUN               0x01  /* errfun is initialized */
 #define GDBM_RCVR_MAX_FAILED_KEYS      0x02  /* max_failed_keys is initialized */
@@ -219,15 +220,16 @@ extern int gdbm_copy_meta (GDBM_FILE dst, GDBM_FILE src);
 # define GDBM_NEED_RECOVERY             29
 # define GDBM_BACKUP_FAILED             30
 # define GDBM_DIR_OVERFLOW              31
 # define GDBM_BAD_BUCKET                32
 # define GDBM_BAD_HEADER                33
 # define GDBM_BAD_AVAIL                 34
+# define GDBM_BAD_HASH_TABLE            35
 
         # define _GDBM_MIN_ERRNO0
-        # define _GDBM_MAX_ERRNOGDBM_BAD_AVAIL
+        # define _GDBM_MAX_ERRNOGDBM_BAD_HASH_TABLE
 
 /* This one was never used and will be removed in the future */
 # define GDBM_UNKNOWN_UPDATE GDBM_UNKNOWN_ERROR
   
 typedef int gdbm_error;
 extern int *gdbm_errno_location (void);
diff --git a/src/gdbmdefs.h b/src/gdbmdefs.h
index 5305b0d..1bb519b 100644
--- a/src/gdbmdefs.h
+++ b/src/gdbmdefs.h
@@ -23,12 +23,28 @@
 #define DEFAULT_TEXT_DOMAIN PACKAGE
 #include "gettext.h"
 
 #define _(s) gettext (s)
 #define N_(s) s
 
+/* The width in bits of the integer type or expression T. */
+#define TYPE_WIDTH(t) (sizeof (t) * CHAR_BIT)
+
+#define SIGNED_TYPE_MAXIMUM(t) \
+  ((t) ((((t) 1 << (TYPE_WIDTH (t) - 2)) - 1) * 2 + 1))
+
+/* Maximum value for off_t */
+#define OFF_T_MAX SIGNED_TYPE_MAXIMUM (off_t)
+
+/* Return true if A can be added to B without integer overflow */
+static inline off_t
+off_t_sum_ok (off_t a, off_t b)
+{
+  return OFF_T_MAX - a >= b;
+}
+
 /* The type definitions are next.  */
 
 /* The available file space is stored in an "avail" table.  The one with
    most activity is contained in the file header. (See below.)  When that
    one filles up, it is split in half and half is pushed on an "avail
    stack."  When the active avail table is empty and the "avail stack" is
@@ -90,12 +106,13 @@ typedef struct
   off_t data_pointer;     /* The file address of the key record. The
                              data record directly follows the key.  */
   int   key_size;         /* Size of key data in the file. */
   int   data_size;        /* Size of associated data in the file. */
 } bucket_element;
 
+extern int gdbm_bucket_element_valid_p (GDBM_FILE dbf, int elem_loc);
 
 /* A bucket is a small hash table.  This one consists of a number of
    bucket elements plus some bookkeeping fields.  The number of elements
    depends on the optimum blocksize for the storage device and on a
    parameter given at file creation time.  This bucket takes one block.
    When one of these tables gets full, it is split into two hash buckets.
diff --git a/src/gdbmerrno.c b/src/gdbmerrno.c
index 896bf70..52cfe30 100644
--- a/src/gdbmerrno.c
+++ b/src/gdbmerrno.c
@@ -131,13 +131,14 @@ const char * const gdbm_errlist[_GDBM_MAX_ERRNO+1] = {
   [GDBM_ERR_FILE_MODE]          = N_("Failed to restore file mode"),
   [GDBM_NEED_RECOVERY]          = N_("Database needs recovery"),
   [GDBM_BACKUP_FAILED]          = N_("Failed to create backup copy"),
   [GDBM_DIR_OVERFLOW]           = N_("Bucket directory overflow"),
   [GDBM_BAD_BUCKET]             = N_("Malformed bucket header"),
   [GDBM_BAD_HEADER]             = N_("Malformed database file header"),
-  [GDBM_BAD_AVAIL]              = N_("Malforemd avail_block")
+  [GDBM_BAD_AVAIL]              = N_("Malformed avail_block"),
+  [GDBM_BAD_HASH_TABLE]         = N_("Malformed hash table")
 };
 
 const char *
 gdbm_strerror (gdbm_error error)
 {
   if (error < _GDBM_MIN_ERRNO || error > _GDBM_MAX_ERRNO)
diff --git a/src/gdbmtool.c b/src/gdbmtool.c
index 33bdf93..9c6eebe 100644
--- a/src/gdbmtool.c
+++ b/src/gdbmtool.c
@@ -549,30 +549,35 @@ err_printer (void *data GDBM_ARG_UNUSED, char const *fmt, ...)
   va_start (ap, fmt);
   vfprintf (stderr, fmt, ap);
   va_end (ap);
   fprintf (stderr, "\n");
 }
 
-/* recover verbose backup max-failed-keys=N max-failed-buckets=N max-failures=N */
+/* recover sumamry verbose backup max-failed-keys=N max-failed-buckets=N max-failures=N */
 void
 recover_handler (struct handler_param *param)
 {
   gdbm_recovery rcvr;
   int flags = 0;
   int rc;
   int i;
   char *p;
+  int summary = 0;
   
-  for (i = 1; i < param->argc; i++)
+  for (i = 0; i < param->argc; i++)
     {
       char *arg = PARAM_STRING (param, i);
       if (strcmp (arg, "verbose") == 0)
         {
           rcvr.errfun = err_printer;
           flags |= GDBM_RCVR_ERRFUN;
         }
+      else if (strcmp (arg, "summary") == 0)
+        {
+          summary = 1;
+        }
       else if (strcmp (arg, "backup") == 0)
         {
           rcvr.errfun = err_printer;
           flags |= GDBM_RCVR_BACKUP;
         }
       else if (strncmp (arg, "max-failures=", 13) == 0)
@@ -614,12 +619,25 @@ recover_handler (struct handler_param *param)
 
   rc = gdbm_recover (gdbm_file, &rcvr, flags);
 
   if (rc == 0)
     {
       fprintf (param->fp, _("Recovery succeeded.\n"));
+      if (summary)
+        {
+          fprintf (param->fp,
+                   _("Keys recovered: %lu, failed: %lu, duplicate: %lu\n"),
+                   (unsigned long) rcvr.recovered_keys,
+                   (unsigned long) rcvr.failed_keys,
+                   (unsigned long) rcvr.duplicate_keys);
+          fprintf (param->fp,
+                   _("Buckets recovered: %lu, failed: %lu\n"),
+                   (unsigned long) rcvr.recovered_buckets,
+                   (unsigned long) rcvr.failed_buckets);
+        }
+      
       if (rcvr.backup_name)
         {
           fprintf (param->fp,
                    _("Original database preserved in file %s"),
                    rcvr.backup_name);
           free (rcvr.backup_name);
@@ -922,13 +940,13 @@ export_handler (struct handler_param *param)
 {
   int format = GDBM_DUMP_FMT_ASCII;
   int flags = GDBM_WRCREAT;
   int i;
   int filemode;
 
-  for (i = 1; i < param->argc; i++)
+  for (i = 0; i < param->argc; i++)
     {
       if (strcmp (PARAM_STRING (param, i), "truncate") == 0)
          flags = GDBM_NEWDB;
       else if (strcmp (PARAM_STRING (param, i), "binary") == 0)
          format = GDBM_DUMP_FMT_BINARY;
       else if (strcmp (PARAM_STRING (param, i), "ascii") == 0)
@@ -956,13 +974,13 @@ import_handler (struct handler_param *param)
   int flag = GDBM_INSERT;
   unsigned long err_line;
   int meta_mask = 0;
   int i;
   int rc;
 
-  for (i = 1; i < param->argc; i++)
+  for (i = 0; i < param->argc; i++)
     {
       if (strcmp (PARAM_STRING (param, i), "replace") == 0)
          flag = GDBM_REPLACE;
       else if (strcmp (PARAM_STRING (param, i), "nometa") == 0)
          meta_mask = GDBM_META_MASK_MODE | GDBM_META_MASK_OWNER;
       else
@@ -1217,12 +1235,13 @@ struct command command_tab[] = {
     FALSE,
     REPEAT_NEVER,
     N_("reorganize") },
   { S(recover), T_CMD,
     checkdb, recover_handler, NULL,
     { { "[verbose]", GDBM_ARG_STRING },
+      { "[summary]", GDBM_ARG_STRING },
       { "[backup]",  GDBM_ARG_STRING },
       { "[max-failed-keys=N]", GDBM_ARG_STRING },
       { "[max-failed-buckets=N]", GDBM_ARG_STRING },
       { "[max-failures=N]", GDBM_ARG_STRING },
       { NULL } },
     FALSE,
diff --git a/src/mmap.c b/src/mmap.c
index 114d8b2..24ede29 100644
--- a/src/mmap.c
+++ b/src/mmap.c
@@ -41,14 +41,21 @@
   ((off) >= (dbf)->mapped_off \
    && ((off) - (dbf)->mapped_off) < (dbf)->mapped_size)
 /* Return true if the current region needs to be remapped */
 # define _GDBM_NEED_REMAP(dbf) \
   (!(dbf)->mapped_region || (dbf)->mapped_pos == (dbf)->mapped_size)
 /* Return the sum of the currently mapped size and DELTA */
-# define SUM_FILE_SIZE(dbf, delta) \
-  ((dbf)->mapped_off + (dbf)->mapped_size + (delta))
+static inline off_t
+SUM_FILE_SIZE (GDBM_FILE dbf, off_t delta)
+{
+  if (delta >= 0
+      && off_t_sum_ok (dbf->mapped_off, dbf->mapped_size)
+      && off_t_sum_ok (dbf->mapped_off + dbf->mapped_size, delta))
+    return dbf->mapped_off + dbf->mapped_size + delta;
+  return -1;
+}
 
 /* Store the size of the GDBM file DBF in *PSIZE.
    Return 0 on success and -1 on failure. */
 int
 _gdbm_file_size (GDBM_FILE dbf, off_t *psize)
 {
@@ -179,12 +186,23 @@ _gdbm_file_extend (GDBM_FILE dbf, off_t size)
    The function returns 0 on success, -1 on failure. */
 int
 _gdbm_mapped_remap (GDBM_FILE dbf, off_t size, int flag)
 {
   off_t file_size, pos;
 
+  if (size < 0)
+    {
+      errno = EINVAL;
+      GDBM_SET_ERRNO (dbf, GDBM_FILE_SEEK_ERROR, TRUE);
+      return -1;
+    }
+
+  if (size < dbf->mapped_size)
+    /* Nothing to do */
+    return 0;
+  
   if (_gdbm_file_size (dbf, &file_size))
     {
       SAVE_ERRNO (_gdbm_mapped_unmap (dbf));
       return -1; 
     }
 
diff --git a/src/recover.c b/src/recover.c
index d6d4ff9..721c23f 100644
--- a/src/recover.c
+++ b/src/recover.c
@@ -309,12 +309,26 @@ run_recovery (GDBM_FILE dbf, GDBM_FILE new_dbf, gdbm_recovery *rcvr, int flags)
 
               data.dptr  = dptr + key.dsize;
               data.dsize = dbf->bucket->h_table[i].data_size;
             
               if (gdbm_store (new_dbf, key, data, GDBM_INSERT) != 0)
                 {
+                  switch (gdbm_last_errno (new_dbf))
+                    {
+                    case GDBM_CANNOT_REPLACE:
+                      rcvr->duplicate_keys++;
+                      if (flags & GDBM_RCVR_ERRFUN)
+                        rcvr->errfun (rcvr->data,
+                          _("ignoring duplicate key %d:%d (%lu:%d)"),
+                          bucket_dir, i,
+                          (unsigned long) dbf->bucket->h_table[i].data_pointer,
+                          dbf->bucket->h_table[i].key_size
+                                      + dbf->bucket->h_table[i].data_size);
+                      break;
+                      
+                    default:
                   if (flags & GDBM_RCVR_ERRFUN)
                     rcvr->errfun (rcvr->data,
                                   _("fatal: can't store element %d:%d (%lu:%d): %s"),
                                   bucket_dir, i,
                                   (unsigned long) dbf->bucket->h_table[i].data_pointer,
                                   dbf->bucket->h_table[i].key_size
@@ -322,12 +336,13 @@ run_recovery (GDBM_FILE dbf, GDBM_FILE new_dbf, gdbm_recovery *rcvr, int flags)
                                   gdbm_db_strerror (new_dbf));
                   return -1;
                         }
             }
         }
     }
+    }
   
   return 0;
 }
 
 int
 gdbm_recover (GDBM_FILE dbf, gdbm_recovery *rcvr, int flags)
@@ -353,12 +368,13 @@ gdbm_recover (GDBM_FILE dbf, gdbm_recovery *rcvr, int flags)
       flags = 0;
     }
   rcvr->recovered_keys = 0;
   rcvr->recovered_buckets = 0;
   rcvr->failed_keys = 0;
   rcvr->failed_buckets = 0;
+  rcvr->duplicate_keys = 0;
   rcvr->backup_name = NULL;
 
   rc = 0;
   if ((flags & GDBM_RCVR_FORCE) || check_db (dbf))
     {
       len = strlen (dbf->name);
diff --git a/src/systems.h b/src/systems.h
index 66955dd..c678573 100644
--- a/src/systems.h
+++ b/src/systems.h
@@ -30,12 +30,13 @@
 #else
 # include <strings.h>
 #endif
 #include <unistd.h>
 #include <fcntl.h>
 #include <errno.h>
+#include <limits.h>
 
 #ifndef SEEK_SET
 # define SEEK_SET        0
 #endif
 
 #ifndef O_CLOEXEC