[asterisk-bugs] [Asterisk 0019368]: The retrans_pkt function can corrupt the message list in the gateway structure

[Asterisk Bug Tracker]

A NOTE has been added to this issue. 
====================================================================== 
https://issues.asterisk.org/view.php?id=19368 
====================================================================== 
Reported By:                JeffW
Assigned To:                
====================================================================== 
Project:                    Asterisk
Issue ID:                   19368
Category:                   Channels/chan_mgcp
Reproducibility:            random
Severity:                   minor
Priority:                   normal
Status:                     feedback
Asterisk Version:           1.8.3.2 
JIRA:                        
Regression:                 No 
Reviewboard Link:            
SVN Branch (only for SVN checkouts, not tarball releases): N/A 
SVN Revision (number only!):  
Request Review:              
====================================================================== 
Date Submitted:             2011-05-25 15:46 CDT
Last Modified:              2011-06-02 15:17 CDT
====================================================================== 
Summary:                    The retrans_pkt function can corrupt the message
list in the gateway structure
Description: 
I believe there are errors in the retrans_pkt function when a max retries
exceeded error occurs.  In the "for" statement the 'prev" variable is set
to an incorrect value when a message in the list has exceeded its retries. 
It should remain unchanged, but the "for" statement will set "prev" to
point to the message just removed.  This could corrupt the list.

The code attempts to build a list of expired messages using the same
"next" field as used to link the active message list.  This corrupts the
value of "cur->next" which is used to process the rest of the active
message list.  It will be set to null or point to the expired message list.
====================================================================== 

---------------------------------------------------------------------- 
 (0135653) JeffW (reporter) - 2011-06-02 15:17
 https://issues.asterisk.org/view.php?id=19368#c135653 
---------------------------------------------------------------------- 
Here is the diff of the changes I made to correct this problem:


655c655
< 	struct mgcp_message *cur, *exq = NULL, *w, *prev;
---
> 	struct mgcp_message *cur, *exq = NULL, *w, *prev, *next;
661c661,662
< 	for (prev = NULL, cur = gw->msgs; cur; prev = cur, cur = cur->next) {
---
> 	for (prev = NULL, cur = gw->msgs; cur; cur = next) {
> 		next = cur->next;
666a668
> 			prev = cur;
667a670
> 			/* remove current message from the gateway message queue */
669c672
< 				prev->next = cur->next;
---
> 				prev->next = next;      // prev remains unchanged
671,674c674
< 				gw->msgs = cur->next;
< 
< 			ast_log(LOG_WARNING, "Maximum retries exceeded for transaction %u on
[%s]\n",
< 				cur->seqno, gw->name);
---
> 				gw->msgs = next;        // prev remains null
675a676
> 			/* construct a list of expired messages with 'exq' as the list head
*/
697a699,702
> 		ast_log(LOG_WARNING, 
> 		        "Maximum retries exceeded for transaction %u on gateway
%s\nbuffer[%d] = %4.31s\n", 
> 		        cur->seqno, gw->name, cur->len, cur->buf); 

Issue History 
Date Modified    Username       Field                    Change               
====================================================================== 
2011-06-02 15:17 JeffW          Note Added: 0135653                          
======================================================================