[Asterisk-code-review] Astobj2: Correctly treat hash fn returning INT MIN (asterisk[master])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged.

Change subject: Astobj2: Correctly treat hash_fn returning INT_MIN
......................................................................


Astobj2: Correctly treat hash_fn returning INT_MIN

The code in astobj2_hash.c wrongly assumed that abs(int) is always > 0.
However, abs(INT_MIN) = INT_MIN and is still negative, as well as
abs(INT_MIN) % num_buckets, and as a result this led to a crash.

One way to trigger the bug is using host=::80 or 0.0.0.128 in peer
configuration section in chan_sip or chan_iax.

This patch takes the remainder before applying abs, so that bucket
number is always in range.

ASTERISK-25100 #close
Reported by: Mark Petersen

Change-Id: Id6981400ad526f47e10bcf7b847b62bd2785e899
---
M main/astobj2_hash.c
1 file changed, 5 insertions(+), 6 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, but someone else must approve
  Richard Mudgett: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved; Verified



diff --git a/main/astobj2_hash.c b/main/astobj2_hash.c
index 91ad2d2..b036911 100644
--- a/main/astobj2_hash.c
+++ b/main/astobj2_hash.c
@@ -215,8 +215,7 @@
 		return NULL;
 	}
 
-	i = abs(self->hash_fn(obj_new, OBJ_SEARCH_OBJECT));
-	i %= self->n_buckets;
+	i = abs(self->hash_fn(obj_new, OBJ_SEARCH_OBJECT) % self->n_buckets);
 
 	__ao2_ref(obj_new, +1, tag ?: "Container node creation", file, line, func);
 	node->common.obj = obj_new;
@@ -362,8 +361,8 @@
 	case OBJ_SEARCH_OBJECT:
 	case OBJ_SEARCH_KEY:
 		/* we know hash can handle this case */
-		bucket_cur = abs(self->hash_fn(arg, flags & OBJ_SEARCH_MASK));
-		bucket_cur %= self->n_buckets;
+		bucket_cur = abs(self->hash_fn(arg, flags & OBJ_SEARCH_MASK)
+				% self->n_buckets);
 		state->sort_fn = self->common.sort_fn;
 		break;
 	case OBJ_SEARCH_PARTIAL_KEY:
@@ -960,8 +959,8 @@
 			++count_obj;
 
 			/* Check container hash key for expected bucket. */
-			bucket_exp = abs(self->hash_fn(node->common.obj, OBJ_SEARCH_OBJECT));
-			bucket_exp %= self->n_buckets;
+			bucket_exp = abs(self->hash_fn(node->common.obj, OBJ_SEARCH_OBJECT)
+					% self->n_buckets);
 			if (bucket != bucket_exp) {
 				ast_log(LOG_ERROR, "Bucket %d node hashes to bucket %d!\n",
 					bucket, bucket_exp);

-- 
To view, visit https://gerrit.asterisk.org/530
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Id6981400ad526f47e10bcf7b847b62bd2785e899
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Ivan Poddubny <ivan.poddubny at gmail.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>