diff mbox

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

Message ID 56A6B883.2070801@cs.ucla.edu
State Superseded
Headers show

Commit Message

Paul Eggert Jan. 26, 2016, 12:06 a.m. UTC
On 01/25/2016 12:09 PM, Florian Weimer wrote:
> -  while (div * div < number && number % div != 0)
> +  while (div * (unsigned long long) div < number && number % div != 0)

Good catch. But better yet, get rid of the '*' so that we needn't worry 
about whether the multiplication overflows. On typical platforms the 
divide instruction that the '%' is already doing will give us the 
information we need, so this is faster anyway. Something like the 
attached (untested) patch.
diff mbox

Patch

diff --git a/misc/hsearch_r.c b/misc/hsearch_r.c
index f6f16ed..1fca6b3 100644
--- a/misc/hsearch_r.c
+++ b/misc/hsearch_r.c
@@ -46,15 +46,12 @@  static int
 isprime (unsigned int number)
 {
   /* no even number will be passed */
-  unsigned int div = 3;
-
-  while (div * div < number && number % div != 0)
-    div += 2;
-
-  return number % div != 0;
+  for (unsigned int div = 3; div <= number / div; div += 2)
+    if (number % div == 0)
+      return 0;
+  return 1;
 }
 
-
 /* Before using the hash table we must allocate memory for it.
    Test for an existing table are done. We allocate one element
    more as the found prime number says. This is done for more effective
@@ -71,13 +68,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 +76,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;