res_fax_spandsp: Always init T.38 session to avoid crashes during state change
authorMatthew Jordan <mjordan@digium.com>
Mon, 9 Dec 2013 03:21:56 +0000 (03:21 +0000)
committerMatthew Jordan <mjordan@digium.com>
Mon, 9 Dec 2013 03:21:56 +0000 (03:21 +0000)
Prior to this patch, res_fax_spandsp was conservative with how it initialized
the spandsp T.38 context. It would only initialize it if the driver thought
the current state was a T.38 fax. While this works fine in nominal situations,
in certain off nominal situations, res_fax_spandsp can believe that a T.38
fax will not occur when in fact one has started. In particular, this was
discovered when res_fax would fall back to audio after timing out on a T.38
upgrade. The SIP channel driver would continue to retry the re-INVITE and -
if the remote end responded after res_fax timed out with a 200 OK - a T.38
frame would be delivered to the res_fax stack when it no longer expected it.

As it turns out, there does not appear to be any downside to always
initializing the T.38 context, other than the actual memory allocation.
Since that avoids this off nominal situation (and others which are equally
likely hard to predict), this is the safest way to avoid this problem.

Much thanks to Torrey as well for providing a scenario that reproduces this
issue.

(closes issue ASTERISK-21242)
Reported by: Ashley Winters
Tested by: Torrey Searle
patches:
  always-init-t38.patch uploaded by awinters (License 6477)
  A_PARTY.xml uploaded by tsearle (License 5334)
........

Merged revisions 403449 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........

Merged revisions 403450 from http://svn.asterisk.org/svn/asterisk/branches/11
........

Merged revisions 403458 from http://svn.asterisk.org/svn/asterisk/branches/12

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@403466 65c4cc65-6c06-0410-ace0-fbb531ad65f3

res/res_fax_spandsp.c

index bd2bb3c..1bf1f9d 100644 (file)
@@ -545,7 +545,7 @@ static void *spandsp_fax_new(struct ast_fax_session *s, struct ast_fax_tech_toke
 
        p->stats = &spandsp_global_stats.g711;
 
-       if (s->details->caps & AST_FAX_TECH_T38) {
+       if (s->details->caps & (AST_FAX_TECH_T38 | AST_FAX_TECH_AUDIO)) {
                if ((s->details->caps & AST_FAX_TECH_AUDIO) == 0) {
                        /* audio mode was not requested, start in T.38 mode */
                        p->ist38 = 1;
@@ -555,9 +555,7 @@ static void *spandsp_fax_new(struct ast_fax_session *s, struct ast_fax_tech_toke
                /* init t38 stuff */
                t38_terminal_init(&p->t38_state, caller_mode, t38_tx_packet_handler, s);
                set_logging(&p->t38_state.logging, s->details);
-       }
 
-       if (s->details->caps & AST_FAX_TECH_AUDIO) {
                /* init audio stuff */
                fax_init(&p->fax_state, caller_mode);
                set_logging(&p->fax_state.logging, s->details);