nta: assertion `sq' failed in outgoing_answer_a() after nameserver SERVFAIL (v1.13.17/v1.13.18)

[mmeyerlein]

Summary

FreeSWITCH aborts inside libsofia-sip-ua when the nameserver returns SERVFAIL for outbound registrations. The abort is the assert(sq) at the top of outgoing_answer_a() (nta.c:11022 in v1.13.18; the same assert exists in outgoing_answer_aaaa()). A transient DNS failure should end in a 503 and a registration retry, not in a core dump of the whole SIP stack.

This looks like the same report as signalwire/freeswitch#1008 (2020, closed as not reproducible).

Environment

• FreeSWITCH 1.11.1 (Debian bookworm packages from the SignalWire repo)
• libsofia-sip-ua0 1.13.17-27770626046-7333682a29~bookworm (i.e. commit 7333682, one commit before the v1.13.18 tag, so the "Fix hang on unresolvable DNS resolution error" change is already in this build)
• Four registered gateways (two providers, one of them with three accounts), /etc/resolv.conf with a single nameserver and a search domain
• The assert is unchanged in v1.13.18, and master has no nta.c commits since the tag.

What happened

All gateways lost DNS at the same moment (the nameserver answered SERVFAIL for ~1 s), FreeSWITCH logged the usual unregister/retry sequence and aborted two seconds later:

[WARNING] sofia.c:6473 Unregister gw-a
[WARNING] sofia.c:6488 Ping failed gw-a with code 503 - count 1/0/1, state DOWN
[WARNING] sofia.c:6473 Unregister gw-b
[WARNING] sofia.c:6488 Ping failed gw-b with code 503 - count 1/0/1, state DOWN
... (same for gw-c, gw-d)
[WARNING] sofia_reg.c:520 gw-a Failed Registration [0], setting retry to 30 seconds.
[WARNING] sofia_reg.c:520 gw-b Failed Registration [0], setting retry to 30 seconds.
[WARNING] sofia_reg.c:520 gw-c Failed Registration [0], setting retry to 30 seconds.
[WARNING] sofia_reg.c:520 gw-d Failed Registration [0], setting retry to 30 seconds.
[ERR] sofia_reg.c:2661 gw-a Failed Registration with status DNS Error [503]. failure #1
[WARNING] sofia_reg.c:520 gw-a Failed Registration [503], setting retry to 30 seconds.
--- 2 s later ---
systemd: freeswitch.service: Main process exited, code=dumped, status=6/ABRT

Backtrace (from the core)

#0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
#1  0x00007f8a926a8f4f in __pthread_kill_internal (signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:78
#2  0x00007f8a92659fb2 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#3  0x00007f8a92644472 in __GI_abort () at ./stdlib/abort.c:79
#4  0x00007f8a92644395 in __assert_fail_base (fmt=0x7f8a927b9a90 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=assertion@entry=0x7f8a9225aa49 "sq", file=file@entry=0x7f8a9225a30b "nta.c", line=line@entry=11022, function=function@entry=0x7f8a9225b930 "outgoing_answer_a") at ./assert/assert.c:94
#5  0x00007f8a92652ec2 in __GI___assert_fail (assertion=0x7f8a9225aa49 "sq", file=0x7f8a9225a30b "nta.c", line=11022, function=0x7f8a9225b930 "outgoing_answer_a") at ./assert/assert.c:103
#6  0x00007f8a921bf9b8 in ?? () from /lib/libsofia-sip-ua.so.0
#7  0x00007f8a92213654 in ?? () from /lib/libsofia-sip-ua.so.0

(The distro package has no debug symbols; #6 is outgoing_answer_a, #7 is the resolver's error-report path, matching the backtrace in signalwire/freeswitch#1008 where #5 was sres_query_report_error at sres.c.)

So sr->sr_current is NULL when the A-record callback fires with an error answer. I have not pinned down the exact interleaving (candidates: an answer arriving after outgoing_query_results() already cleared sr_current for the previous query, or a search-domain subquery reporting the error for the top query).

Proposed change

Regardless of the exact race, a late or stray DNS answer must not abort the process. The patch below replaces the two asserts with a guard: drop the records, clear the query handle if it is ours, and fail the transaction with 503 "DNS Error" if it has not been resolved yet. That is the same outcome the code produces for an empty answer with no further queries, and mod_sofia already handles the 503 with a registration retry.

I am running this on the production box that crashed (rebuilt v1.13.18 package). Happy to turn it into a PR if the approach is acceptable.

--- a/libsofia-sip-ua/nta/nta.c
+++ b/libsofia-sip-ua/nta/nta.c
@@ -10932,7 +10932,17 @@
   size_t i, j, found = 0;
   char *result, **results = NULL;
 
-  assert(sq); assert(sq->sq_type == sres_type_aaaa);
+  if (sq == NULL || sq->sq_type != sres_type_aaaa) {
+    /* Stray or late answer: no matching query in flight. Never abort. */
+    SU_DEBUG_1(("nta(%p): ignoring stray AAAA answer for \"%s\"\n",
+		(void *)orq, orq->orq_tpn->tpn_host));
+    sres_free_answers(orq->orq_agent->sa_resolver, answers);
+    if (sr->sr_query == q)
+      sr->sr_query = NULL;
+    if (!orq->orq_resolved)
+      outgoing_resolving_error(orq, SIPDNS_503_ERROR);
+    return;
+  }
 
   sr->sr_query = NULL;
 
@@ -11019,7 +11029,17 @@
   int i, j, found = 0;
   char *result, **results = NULL;
 
-  assert(sq); assert(sq->sq_type == sres_type_a);
+  if (sq == NULL || sq->sq_type != sres_type_a) {
+    /* Stray or late answer: no matching query in flight. Never abort. */
+    SU_DEBUG_1(("nta(%p): ignoring stray A answer for \"%s\"\n",
+		(void *)orq, orq->orq_tpn->tpn_host));
+    sres_free_answers(orq->orq_agent->sa_resolver, answers);
+    if (sr->sr_query == q)
+      sr->sr_query = NULL;
+    if (!orq->orq_resolved)
+      outgoing_resolving_error(orq, SIPDNS_503_ERROR);
+    return;
+  }
 
   sr->sr_query = NULL;