diff mbox

Improve check against integer wraparound in hcreate_r [BZ #18240]

Message ID 56A42D78.1030506@cs.ucla.edu
State Superseded
Headers

Commit Message

Paul Eggert Jan. 24, 2016, 1:48 a.m. UTC 
  Florian Weimer wrote:

> -  if (nel >= SIZE_MAX / sizeof (_ENTRY))
> +  /* This limit is sufficient to avoid unsigned wraparound below,
> +     possibly after truncation to unsigned int.  (struct hsearch_data
> +     is part of the public API and uses usigned ints.)  */
> +  if (nel >= INT_MAX / sizeof (_ENTRY))

This patch doesn't look right. nel should be bounded by UINT_MAX - 2, not by 
INT_MAX / sizeof (anything). (Not by UINT_MAX, since the code computes nel + 1 
later; and not by UINT_MAX - 1 since that cannot be prime.) Furthermore, calloc 
will check for size overflow on multiplication so hcreate_r need not worry about 
dividing by sizeof (anything). Also, "unsigned" is misspelled in the comment.

How about something like the attached (untested) patch instead?
diff mbox

Patch

diff --git a/misc/hsearch_r.c b/misc/hsearch_r.c
index f6f16ed..c258397 100644
--- a/misc/hsearch_r.c
+++ b/misc/hsearch_r.c
@@ -71,13 +71,6 @@  __hcreate_r (size_t nel, struct hsearch_data *htab)
       return 0;
     }
 
-  if (nel >= SIZE_MAX / sizeof (_ENTRY))
-    {
-      __set_errno (ENOMEM);
-      return 0;
-    }
-
-
   /* There is still another table active. Return with error. */
   if (htab->table != NULL)
     return 0;
@@ -86,10 +79,19 @@  __hcreate_r (size_t nel, struct hsearch_data *htab)
      use will not work.  */
   if (nel < 3)
     nel = 3;
-  /* Change nel to the first prime number not smaller as nel. */
-  nel |= 1;      /* make odd */
-  while (!isprime (nel))
-    nel += 2;
+
+  /* Change nel to the first prime number in the range [nel, UINT_MAX - 2],
+     The '- 2' means 'nel += 2' cannot overflow.  */
+  for (nel |= 1; ; nel += 2)
+    {
+      if (UINT_MAX - 2 < nel)
+	{
+	  __set_errno (ENOMEM);
+	  return 0;
+	}
+      if (isprime (nel))
+	break;
+    }
 
   htab->size = nel;
   htab->filled = 0;