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

[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.c
1 file changed, 2 insertions(+), 3 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.c b/main/astobj2.c
index b49ed60..c434209 100644
--- a/main/astobj2.c
+++ b/main/astobj2.c
@@ -900,7 +900,7 @@
 		return NULL;
 	}
 
-	i = abs(c->hash_fn(user_data, OBJ_POINTER));
+	i = abs(c->hash_fn(user_data, OBJ_POINTER) % c->n_buckets);
 
 	if (flags & OBJ_NOLOCK) {
 		orig_lock = adjust_lock(c, AO2_LOCK_REQ_WRLOCK, 1);
@@ -909,7 +909,6 @@
 		orig_lock = AO2_LOCK_REQ_MUTEX;
 	}
 
-	i %= c->n_buckets;
 	p->astobj = obj;
 	p->version = ast_atomic_fetchadd_int(&c->version, 1);
 	AST_LIST_INSERT_TAIL(&c->buckets[i], p, entry);
@@ -1065,7 +1064,7 @@
 	 */
 	if ((flags & (OBJ_POINTER | OBJ_KEY))) {
 		/* we know hash can handle this case */
-		start = i = c->hash_fn(arg, flags & (OBJ_POINTER | OBJ_KEY)) % c->n_buckets;
+		start = i = abs(c->hash_fn(arg, flags & (OBJ_POINTER | OBJ_KEY)) % c->n_buckets);
 	} else {
 		/* don't know, let's scan all buckets */
 		start = i = -1;		/* XXX this must be fixed later. */

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Id6981400ad526f47e10bcf7b847b62bd2785e899
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: 11
Gerrit-Owner: Ivan Poddubny <ivan.poddubny at gmail.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: 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>