]> git.ipfire.org Git - thirdparty/asterisk.git/commit
bridging: Fix multiple bridging issues causing SEGVs and FRACKs.
authorGeorge Joseph <gjoseph@sangoma.com>
Wed, 22 Jan 2025 20:52:33 +0000 (13:52 -0700)
committerGeorge Joseph <gjoseph@sangoma.com>
Thu, 20 Feb 2025 18:34:26 +0000 (18:34 +0000)
commit46c9f7db8e6f529c242d2f8f43294743b8777258
tree811326fb66f0c849e9ea0548170e31bcb60a6d6b
parent1a7c9e0a041bf14978a1a4659a4bd18be51c66a5
bridging: Fix multiple bridging issues causing SEGVs and FRACKs.

Issues:

* The bridging core allowed multiple bridges to be created with the same
  unique bridgeId at the same time.  Only the last bridge created with the
  duplicate name was actually saved to the core bridges container.

* The bridging core was creating a stasis topic for the bridge and saving it
  in the bridge->topic field but not increasing its reference count.  In the
  case where two bridges were created with the same uniqueid (which is also
  the topic name), the second bridge would get the _existing_ topic the first
  bridge created.  When the first bridge was destroyed, it would take the
  topic with it so when the second bridge attempted to publish a message to
  it it either FRACKed or SEGVd.

* The bridge destructor, which also destroys the bridge topic, is run from the
  bridge manager thread not the caller's thread.  This makes it possible for
  an ARI developer to create a new one with the same uniqueid believing the
  old one was destroyed when, in fact, the old one's destructor hadn't
  completed. This could cause the new bridge to get the old one's topic just
  before the topic was destroyed.  When the new bridge attempted to publish
  a message on that topic, asterisk could either FRACK or SEGV.

* The ARI bridges resource also allowed multiple bridges to be created with
  the same uniqueid but it kept the duplicate bridges in its app_bridges
  container.  This created a situation where if you added two bridges with
  the same "bridge1" uniqueid, all operations on "bridge1" were performed on
  the first bridge created and the second was basically orphaned.  If you
  attempted to delete what you thought was the second bridge, you actually
  deleted the first one created.

Changes:

* A new API `ast_bridge_topic_exists(uniqueid)` was created to determine if
  a topic already exists for a bridge.

* `bridge_base_init()` in bridge.c and `ast_ari_bridges_create()` in
  resource_bridges.c now call `ast_bridge_topic_exists(uniqueid)` to check
  if a bridge with the requested uniqueid already exists and will fail if it
  does.

* `bridge_register()` in bridges.c now checks the core bridges container to
  make sure a bridge doesn't already exist with the requested uniqueid.
  Although most callers of `bridge_register()` will have already called
  `bridge_base_init()`, which will now fail on duplicate bridges, there
  is no guarantee of this so we must check again.

* The core bridges container allocation was changed to reject duplicate
  uniqueids instead of silently replacing an existing one. This is a "belt
  and suspenders" check.

* A global mutex was added to bridge.c to prevent concurrent calls to
  `bridge_base_init()` and `bridge_register()`.

* Even though you can no longer create multiple bridges with the same uniqueid
  at the same time, it's still possible that the bridge topic might be
  destroyed while a second bridge with the same uniqueid was trying to use
  it. To address this, the bridging core now increments the reference count
  on bridge->topic when a bridge is created and decrements it when the
  bridge is destroyed.

* `bridge_create_common()` in res_stasis.c now checks the stasis app_bridges
  container to make sure a bridge with the requested uniqueid doesn't already
  exist.  This may seem like overkill but there are so many entrypoints to
  bridge creation that we need to be safe and catch issues as soon in the
  process as possible.

* The stasis app_bridges container allocation was changed to reject duplicate
  uniqueids instead of adding them. This is a "belt and suspenders" check.

* The `bridge show all` CLI command now shows the bridge name as well as the
  bridge id.

* Response code 409 "Conflict" was added as a possible response from the ARI
  bridge create resources to signal that a bridge with the requested uniqueid
  already exists.

* Additional debugging was added to multiple bridging and stasis files.

Resolves: #211
include/asterisk/bridge.h
include/asterisk/stasis_bridges.h
main/bridge.c
main/stasis_bridges.c
res/ari/resource_bridges.c
res/ari/resource_bridges.h
res/res_ari_bridges.c
res/res_stasis.c
res/stasis/stasis_bridge.c
rest-api/api-docs/bridges.json