[Asterisk-code-review] pbx: Fix hints deadlock between reload and ExtensionState. (asterisk[18])

[George Joseph]

George Joseph has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/14836 )

Change subject: pbx: Fix hints deadlock between reload and ExtensionState.
......................................................................

pbx: Fix hints deadlock between reload and ExtensionState.

When the ExtensionState AMI action is executed on a pattern matched
hint it can end up adding a new hint if one does not already exist.
This results in a locking order of contexts -> hints -> contexts.

If at the same time a reload is occurring and adding its own hint
it will have a locking order of hints -> contexts.

This results in a deadlock as one thread wants a lock on contexts
that the other has, and the other thread wants a lock on hints
that the other has.

This change enforces a hints -> contexts locking order by explicitly
locking hints in the places where a hint is added when queried for.
This matches the order seen through normal adding of hints.

ASTERISK-29046

Change-Id: I49f027f4aab5d2d50855ae937bcf5e2fd8bfc504
---
M main/pbx.c
1 file changed, 10 insertions(+), 1 deletion(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved; Approved for Submit



diff --git a/main/pbx.c b/main/pbx.c
index bbc6df9..b520f5f 100644
--- a/main/pbx.c
+++ b/main/pbx.c
@@ -3148,10 +3148,15 @@
 	}
 
 	if (e->exten[0] == '_') {
-		/* Create this hint on-the-fly */
+		/* Create this hint on-the-fly, we explicitly lock hints here to ensure the
+		 * same locking order as if this were done through configuration file - that is
+		 * hints is locked first and then (if needed) contexts is locked
+		 */
+		ao2_lock(hints);
 		ast_add_extension(e->parent->name, 0, exten, e->priority, e->label,
 			e->matchcid ? e->cidmatch : NULL, e->app, ast_strdup(e->data), ast_free_ptr,
 			e->registrar);
+		ao2_unlock(hints);
 		if (!(e = ast_hint_extension(c, context, exten))) {
 			/* Improbable, but not impossible */
 			return -1;
@@ -3228,9 +3233,11 @@
 
 	if (e->exten[0] == '_') {
 		/* Create this hint on-the-fly */
+		ao2_lock(hints);
 		ast_add_extension(e->parent->name, 0, exten, e->priority, e->label,
 			e->matchcid ? e->cidmatch : NULL, e->app, ast_strdup(e->data), ast_free_ptr,
 			e->registrar);
+		ao2_unlock(hints);
 		if (!(e = ast_hint_extension(c, context, exten))) {
 			/* Improbable, but not impossible */
 			return -1;
@@ -3766,9 +3773,11 @@
 	 * individual extension, because the pattern will no longer match first.
 	 */
 	if (e->exten[0] == '_') {
+		ao2_lock(hints);
 		ast_add_extension(e->parent->name, 0, exten, e->priority, e->label,
 			e->matchcid ? e->cidmatch : NULL, e->app, ast_strdup(e->data), ast_free_ptr,
 			e->registrar);
+		ao2_unlock(hints);
 		e = ast_hint_extension(NULL, context, exten);
 		if (!e || e->exten[0] == '_') {
 			return -1;

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/14836
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 18
Gerrit-Change-Id: I49f027f4aab5d2d50855ae937bcf5e2fd8bfc504
Gerrit-Change-Number: 14836
Gerrit-PatchSet: 2
Gerrit-Owner: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200828/9356b266/attachment-0001.html>