Skip to content

Commit

Permalink
usrloc: Fix locking for contact_id-based deletion
Browse files Browse the repository at this point in the history
The delete_ucontact_from_id(), get_ucontact_from_id() pair of functions
were masking possible code execution paths which would lead to unsafe memory
access. This, in turn, would cause the ping_checker_timer() job of nathelper to
occasionally crash and burn.

Reported by solo@astercc.org.

(cherry picked from commit 18a93c8)
  • Loading branch information
liviuchircu committed Jan 17, 2018
1 parent 8d139f3 commit 99432b1
Showing 1 changed file with 11 additions and 2 deletions.
13 changes: 11 additions & 2 deletions modules/usrloc/dlist.c
Expand Up @@ -824,6 +824,10 @@ int find_domain(str* _d, udomain_t** _p)

/*
* retrieve the ucontact from a domain using the contact id
*
* Returns:
* NULL, if contact not found
* contact, *with grabbed ulslot lock*
*/
ucontact_t* get_ucontact_from_id(udomain_t *d, uint64_t contact_id, urecord_t **_r)
{
Expand Down Expand Up @@ -866,7 +870,6 @@ ucontact_t* get_ucontact_from_id(udomain_t *d, uint64_t contact_id, urecord_t **
for (c = r->contacts; c != NULL; c = c->next)
if ((unsigned short)c->label == clabel) {
*_r = r;
unlock_ulslot(d, sl);
return c;
}
}
Expand All @@ -879,7 +882,8 @@ int delete_ucontact_from_id(udomain_t *d, uint64_t contact_id, char is_replicate
{
ucontact_t *c, virt_c;
urecord_t *r;

unsigned int sl, rlabel;
unsigned short aorhash, clabel;

/* if contact only in database */
if (db_mode == DB_ONLY) {
Expand Down Expand Up @@ -917,6 +921,11 @@ int delete_ucontact_from_id(udomain_t *d, uint64_t contact_id, char is_replicate
mem_delete_ucontact(r, c);
}

unpack_indexes(contact_id, &aorhash, &rlabel, &clabel);

sl = aorhash & (d->size - 1);
unlock_ulslot(d, sl);

return 0;
}

Expand Down

0 comments on commit 99432b1

Please sign in to comment.