2009-11-10 14:28:27

by André Goddard Rosa

[permalink] [raw]
Subject: [PATCH 0/2] bsearch: fix overflow and avoid unnecessary calculation

Fix a rare overflow situation which can occur when working with large arrays.
Also, remove an unneeded calculation.

André Goddard Rosa (2):
bsearch: avoid unneeded decrement arithmetic
bsearch: prevent overflow when computing middle comparison element

lib/bsearch.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)


2009-11-10 14:28:34

by André Goddard Rosa

[permalink] [raw]
Subject: [PATCH 1/2] bsearch: avoid unneeded decrement arithmetic

Signed-off-by: André Goddard Rosa <[email protected]>
---
lib/bsearch.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/bsearch.c b/lib/bsearch.c
index 2e70664..f71ac89 100644
--- a/lib/bsearch.c
+++ b/lib/bsearch.c
@@ -33,9 +33,9 @@
void *bsearch(const void *key, const void *base, size_t num, size_t size,
int (*cmp)(const void *key, const void *elt))
{
- int start = 0, end = num - 1, mid, result;
+ int start = 0, end = num, mid, result;

- while (start <= end) {
+ while (start < end) {
mid = (start + end) / 2;
result = cmp(key, base + mid * size);
if (result < 0)
--
1.6.5.2.153.g6e31f.dirty

2009-11-10 14:28:38

by André Goddard Rosa

[permalink] [raw]
Subject: [PATCH 2/2] bsearch: prevent overflow when computing middle comparison element

It's really difficult to occur in practice because the sum of the lower
and higher limits must overflow an int variable, but it can occur when
working with large arrays. We'd better safe than sorry by avoiding this
overflow situation when computing the middle element for comparison.

See:
http://googleresearch.blogspot.com/2006/06/extra-extra-read-all-about-it-nearly.html
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=5045582

The program below demonstrates the issue:

$ ./a.out
(glibc) Element is at the last position
(patched) Element is at the last position
Segmentation fault

===
#include <search.h>
#include <stdlib.h>
#include <stdio.h>

/* A number that when doubled will be bigger than 2^31 - 1 */
#define BIG_NUMBER_TO_OVERFLOW_INT (1100000000)

static int cmp_char(const void *p1, const void *p2)
{
char v1 = (*(char *)p1);
char v2 = (*(char *)p2);

if (v1 < v2)
return -1;
else if (v1 > v2)
return 1;
else
return 0;
}

void *lib_bsearch(const void *key, const void *base, size_t num, size_t size
, int (*cmp)(const void *key, const void *elt))
{
int start = 0, end = num - 1, mid, result;

while (start <= end) {
mid = (start + end) / 2;
result = cmp(key, base + mid * size);
if (result < 0)
end = mid - 1;
else if (result > 0)
start = mid + 1;
else
return (void *)base + mid * size;
}

return NULL;
}

void *patch_lib_bsearch(const void *key, const void *base, size_t num, size_t size
, int (*cmp)(const void *key, const void *elt))
{
size_t start = 0, end = num, mid;
int result;

while (start < end) {
mid = (start + end) / 2;
result = cmp(key, base + mid * size);
if (result < 0)
end = mid - 1;
else if (result > 0)
start = mid + 1;
else
return (void *)base + mid * size;
}

return NULL;
}

int main(void)
{
char key = 1;
char *array = calloc(BIG_NUMBER_TO_OVERFLOW_INT, sizeof(char));
void *ptr;

if (!array)
{
printf("%s\n", "no memory");
return EXIT_FAILURE;
}
array[BIG_NUMBER_TO_OVERFLOW_INT - 1] = 1;

ptr = bsearch(&key, array, BIG_NUMBER_TO_OVERFLOW_INT, sizeof(char), cmp_char);
printf("(glibc) Element is%sat the last position\n"
, (ptr == &array[BIG_NUMBER_TO_OVERFLOW_INT - 1]) ? " " : " NOT ");

ptr = patch_lib_bsearch(&key, array, BIG_NUMBER_TO_OVERFLOW_INT, sizeof(char), cmp_char);
printf("(patched) Element is%sat the last position\n"
, (ptr == &array[BIG_NUMBER_TO_OVERFLOW_INT - 1]) ? " " : " NOT ");

ptr = lib_bsearch(&key, array, BIG_NUMBER_TO_OVERFLOW_INT, sizeof(char), cmp_char);
printf("(unpatched) Element is%sat the last position\n"
, (ptr == &array[BIG_NUMBER_TO_OVERFLOW_INT - 1]) ? " " : " NOT ");

free(array);

return EXIT_SUCCESS;
}

Signed-off-by: André Goddard Rosa <[email protected]>
---
lib/bsearch.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/lib/bsearch.c b/lib/bsearch.c
index f71ac89..f1aa361 100644
--- a/lib/bsearch.c
+++ b/lib/bsearch.c
@@ -33,7 +33,8 @@
void *bsearch(const void *key, const void *base, size_t num, size_t size,
int (*cmp)(const void *key, const void *elt))
{
- int start = 0, end = num, mid, result;
+ size_t start = 0, end = num, mid;
+ int result;

while (start < end) {
mid = (start + end) / 2;
--
1.6.5.2.153.g6e31f.dirty