[Home]

Summary:ASTERISK-08159: powerof(0) can happen with external channel drivers in translate.c
Reporter:HPS (hselasky)Labels:
Date Opened:2006-11-21 03:46:59.000-0600Date Closed:2006-11-30 14:28:38.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:
Description:Hi,

In "translate.c", powerof(d) can be called with d=0, which yields the following error:

ast_log(LOG_WARNING, "Powerof %d: No power??\n", d);

This happens with external channel drivers that have not been updated:

1)

Use "ast_set_read_format()" and "ast_set_write_format()" to initialize the
format used, hence "chan->lastreadformat" was uninitialized.

    fmt = ast_best_codec(fmt);

+    ast_set_read_format(pbx_chan, fmt);
+    ast_set_write_format(pbx_chan, fmt);
+


2)

In any bridge routines, don't forward AST_FRAME_NULL.

But still I think asterisk has a bug, in that it cannot handle  
"AST_FRAME_NULL", hence "f->subclass == 0", and it is forwarded by  
"ast_write()" to "ast_translate_write()", which causes a powerof(0) error
with Asterisk 1.2.12+.

In "ast_translate_write()" and possibly the other functions calling powerof() in "translate.c", invalid memory will be accessed if "powerof(0) == -1". Extra checks need to be added to handle that.

--HPS
Comments:By: Joshua C. Colp (jcolp) 2006-11-21 11:28:30.000-0600

I don't understand what you mean by handling AST_FRAME_NULL. It's purposely ignored in ast_write and doesn't get sent through the translation path. As for having an AST_FRAME_VOICE With a subclass of 0, that should never happen if the code for where the frame came from is written properly. Can you clarify some more?
By: HPS (hselasky) 2006-11-30 10:40:20.000-0600

NOTE: I already replied by e-mail, but it seems like the e-mail got list. At least my notes are not here. So:

Yes, I installed asterisk-1.2.13, and there you will see that:

int ast_write(struct ast_channel *chan, struct ast_frame *fr)
{
       int res = -1;
       struct ast_frame *f = NULL;
       /* Stop if we're a zombie or need a soft hangup */
       ast_mutex_lock(&chan->lock);
       if (ast_test_flag(chan, AST_FLAG_ZOMBIE) || ast_check_hangup(chan))  {
               ast_mutex_unlock(&chan->lock);
               return -1;
       }
       /* Handle any pending masquerades */
       if (chan->masq) {
               if (ast_do_masquerade(chan)) {
                       ast_log(LOG_WARNING, "Failed to perform
masquerade\n");
                       ast_mutex_unlock(&chan->lock);
                       return -1;
               }
       }
       if (chan->masqr) {
               ast_mutex_unlock(&chan->lock);
               return 0;
       }
       if (chan->generatordata) {
               if (ast_test_flag(chan, AST_FLAG_WRITE_INT))
                       ast_deactivate_generator(chan);
               else {
                       ast_mutex_unlock(&chan->lock);
                       return 0;
               }
       }
       /* High bit prints debugging */
       if (chan->fout & 0x80000000)
               ast_frame_dump(chan->name, fr, ">>");
       CHECK_BLOCKING(chan);
       switch(fr->frametype) {
       case AST_FRAME_CONTROL:
               /* XXX Interpret control frames XXX */
               ast_log(LOG_WARNING, "Don't know how to handle control frames
yet\n");
               break;
       case AST_FRAME_DTMF:
               ast_clear_flag(chan, AST_FLAG_BLOCKING);
               ast_mutex_unlock(&chan->lock);
               res = do_senddigit(chan,fr->subclass);
               ast_mutex_lock(&chan->lock);
               CHECK_BLOCKING(chan);
               break;
       case AST_FRAME_TEXT:
               if (chan->tech->send_text)
                       res = chan->tech->send_text(chan, (char *) fr->data);
               else
                       res = 0;
               break;
       case AST_FRAME_HTML:
               if (chan->tech->send_html)
                       res = chan->tech->send_html(chan, fr->subclass, (char
*) fr->data, fr->datalen);
               else
                       res = 0;
               break;
       case AST_FRAME_VIDEO:
               /* XXX Handle translation of video codecs one day XXX */
               if (chan->tech->write_video)
                       res = chan->tech->write_video(chan, fr);
               else
                       res = 0;
               break;
       default:
               if (chan->tech->write) {
                       f = ast_translate_write(chan, fr, 0);


....

From the code snippet above you see that AST_FRAME_NULL will fall into the
"default" handler, and be forwarded to ast_translate_write(), which then
triggers the bug. So maybe there is a missing "case AST_FRAME_NULL" ?

Does this clarify more ?
By: Joshua C. Colp (jcolp) 2006-11-30 14:28:37.000-0600

Fixed in 1.2 as of revision 48161. Thanks!