[Home]

Summary:ASTERISK-16943: [patch] When using Realtime gateway definitions, random crashes occur
Reporter:Nahuel Greco (nahuelgreco)Labels:
Date Opened:2010-11-10 23:18:38.000-0600Date Closed:2011-04-25 16:55:02
Priority:CriticalRegression?No
Status:Closed/CompleteComponents:Channels/chan_mgcp
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) dangling-pointers-when-pruning.patch
Description:If you use Realtime MGCP gateways definitions, then Asterisk will randomly crash. The problem is located in the Realtime gateways pruning process that runs every 60 seconds. Here unused Realtime gateways are correctly freed but the gateways linked list is in some cases wrongly relinked, leaving dangling pointers. The fix is to delete the 3832 line where the 'gprev' variable is assigned to a freed gateway.

To illustrate, suppose the following case: you have three gateways in the list, A->B->C->NULL. B and C will be marked for freeing by mgcp_prune_realtime_gateway(), but A will persist because it has pending msgs to send. So:

First loop pass: A will be ignored and gprev will be assigned to A.

Second loop pass: B will be freed, gprev will be (wrongly) assigned to B and A->next will be updated to point to C.

Third loop pass: C will be freed and gprev->next will be updated to the value in C->next, thats means now B->next will point to NULL. This leaves A pointing forever to C, a freed gateway. From now the gateways list is corrupted.

Note, I never saw Asterisk segfaulting when updating the B->next pointer in the freed B gateway at the second pass. This makes the problem more obscure because then Asterisk will segfault at multiple points in chan_mgcp.c.

A patch is attached with the fix and a little cleanup. The problem is present in Asterisk 1.8.0 and trunk rev 294606.
Comments:By: Leif Madsen (lmadsen) 2010-11-18 15:33:37.000-0600

FYI: The chan_mgcp module has very little support really. I'd suggest getting some reviews from the asterisk-dev list, and if someone signs off on it, I'd be happy to commit it.

By: Eduardo Ferro (eferro) 2010-11-27 11:12:00.000-0600

I was reviewing this code and it seems that the patch do the job... The issue was described perfect by nahuelgreco. The problem affects mgcp and ncs protocols.

At this point, I don't have a CMTS hardware at my lab to test the patch, and of course, I can't test this solution in a production system, but I follow/read carefully the initial code and the patch and it looks very well.

I hope this patch can be get committed to the trunk...

By: Digium Subversion (svnbot) 2011-04-25 16:49:02

Repository: asterisk
Revision: 315349

U   branches/1.8/channels/chan_mgcp.c

------------------------------------------------------------------------
r315349 | rmudgett | 2011-04-25 16:49:02 -0500 (Mon, 25 Apr 2011) | 9 lines

When using MGCP realtime gateway definitions, random crashes occur.

Fixed incorrect linked list node removal for realtime gateways.

(closes issue ASTERISK-16943)
Reported by: nahuelgreco
Patches:
     dangling-pointers-when-pruning.patch uploaded by nahuelgreco (license 162)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=315349

By: Digium Subversion (svnbot) 2011-04-25 16:55:01

Repository: asterisk
Revision: 315350

_U  trunk/
U   trunk/channels/chan_mgcp.c

------------------------------------------------------------------------
r315350 | rmudgett | 2011-04-25 16:55:01 -0500 (Mon, 25 Apr 2011) | 16 lines

Merged revisions 315349 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.8

........
 r315349 | rmudgett | 2011-04-25 16:49:00 -0500 (Mon, 25 Apr 2011) | 9 lines
 
 When using MGCP realtime gateway definitions, random crashes occur.
 
 Fixed incorrect linked list node removal for realtime gateways.
 
 (closes issue ASTERISK-16943)
 Reported by: nahuelgreco
 Patches:
       dangling-pointers-when-pruning.patch uploaded by nahuelgreco (license 162)
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=315350