| 01:01.57 | *** join/#asterisk-dev Dovid (~dovid@96.56.22.146) |
| 01:16.35 | *** join/#asterisk-dev Dovid (~dovid@ool-2f10fdb3.dyn.optonline.net) |
| 03:42.37 | *** join/#asterisk-dev electronic_eel (~quassel@213.240.182.146) |
| 06:30.12 | *** join/#asterisk-dev Cyrillax (~Cyrillax@unaffiliated/cyrillax) |
| 06:45.25 | Cyrillax | Hey guys, hoping for a little help here. After working as intended for thousands of hours across multiple machines, I encountered a segfault when using AST_LIST_TRAVERSE. Here is a pastebin with the backtrace, complete modified function, as well as some gdb screencaps: https://pastebin.com/pNyaDaih |
| 06:52.05 | Cyrillax | It seems the iterator (var/cy_var) had a value of 0x20 on one loop, which obviously isn't a valid memory location. And as var was passed to ast_var_name(cy_var), when it tries to compare cy_var->name[0] (via cmp [rdi+0x10], 0x5f) there is a segfault as that's not a valid memory location |
| 07:15.35 | Cyrillax | I'm currently manually stepping through [pseudocode] for(i = chan->varshead->first; i <= chan->varshead->last; i=i->entries->next) looking for entries->next=0x20, and will update |
| 09:03.10 | Cyrillax | i guess that middle expr should be i->next != NULL; but you get the idea. can't believe how many cvars this guy has lol. I'm probably going to add a check for the first letter before the strncmp lol |
| 09:06.10 | Cyrillax | hmm i just reached the end and none of them pointed to 0x20. (i'm looking in the core's memory, so it should be there....) Next step is analyzing the AST_LIST_TRAVERSE macro to see how it works in case it's not like this. |
| 09:12.42 | Cyrillax | This doesn't make sense to me, it's just a simple for loop identical to what i just manually walked through |
| 09:13.13 | Cyrillax | How tf did the var (cy_var) get set to 0x20. |
| 09:14.48 | Cyrillax | I took cy_var = cy_chan->varshead->first, and walked through each cy_var->entries.next and they all look correct until the end (which is after the match) which has next=0 |
| 09:15.31 | *** join/#asterisk-dev mvanbaak (~mvanbaak@asterisk/contributor-and-bug-marshal/mvanbaak) |
| 09:15.31 | *** mode/#asterisk-dev [+o mvanbaak] by ChanServ |
| 09:25.54 | Cyrillax | At this point i'm wondering if it's a compiler bug, because following the c instructions as written yields the correct result |
| 09:27.11 | Cyrillax | That somehow took weeks and thousands of combined hours to crop up... if it was memory corruption, then it should be corrupted in the core file, too. So i'm at a loss, any ideas welcome |
| 09:27.54 | Cyrillax | (Pastebin in first post btw) |
| 09:51.14 | jkroon | Cyrillax, sounds like memory corruption of the list pointers. |
| 09:51.20 | jkroon | buffer overrun somewhere? |
| 09:52.36 | Cyrillax | maybe, I literally walked through the entire varshead->first list in GDB and didn't see any corruption, (using the corefile with the chan that caused the crash) |
| 09:52.42 | jkroon | based on what you're saying though you can't find that value. |
| 09:52.51 | Cyrillax | yup |
| 09:53.33 | jkroon | interesting. can't imagine a compile bug on that though, it's fairly simple code. |
| 09:54.03 | Cyrillax | there's probably a better way, but i went gdb: p *cy_chan->varshead->first->entries.next->entries.next...... until reached $95 = {entries = {next = 0x0} } |
| 09:59.35 | *** join/#asterisk-dev AsteriskRoss (~AsteriskR@37.157.48.2) |
| 10:13.17 | *** join/#asterisk-dev sh_smith (~sh_smith@cpe-172-88-21-24.socal.res.rr.com) |
| 10:36.18 | *** join/#asterisk-dev sh_smith (~sh_smith@cpe-172-88-21-24.socal.res.rr.com) |
| 10:53.23 | *** join/#asterisk-dev sh_smith (~sh_smith@cpe-172-88-21-24.socal.res.rr.com) |
| 14:56.36 | *** join/#asterisk-dev AsteriskRoss (~AsteriskR@37.157.48.2) |
| 15:18.58 | *** join/#asterisk-dev kharwell (uid358942@gateway/web/irccloud.com/x-bvmksumelwqcqlhc) |
| 15:18.58 | *** mode/#asterisk-dev [+o kharwell] by ChanServ |
| 15:33.14 | *** join/#asterisk-dev bford (uid283514@gateway/web/irccloud.com/x-rsoqaqpvkakipzrt) |
| 15:33.14 | *** mode/#asterisk-dev [+o bford] by ChanServ |
| 16:07.05 | *** join/#asterisk-dev tmoore (~tmoore@2600:1702:4110:3d10:7621:ad0f:59fc:8170) |
| 17:33.26 | *** join/#asterisk-dev pchero (~pchero@211.178.226.108) |
| 17:33.26 | *** mode/#asterisk-dev [+o pchero] by ChanServ |
| 18:30.04 | jkroon | gtjoseph, file, in case you haven't noticed, opion 1 is up for review. It's a big but trivial patch. busy working on the other option, just having a verbal argument with the pre-processor. |
| 18:30.22 | jkroon | it'll be a smaller patch but technically much nastier. |
| 18:30.27 | jkroon | (in my opinion) |
| 19:03.39 | kharwell | jkroon: you patch gave me deja vu, and then jogged my memory. I think that's what drove the initial change to the embedded zero length array. I believe I started down a similar path to your current path but then opted for the "short cut" of modifying the zero length array. |
| 19:05.43 | kharwell | Oddly I don't know why it did not give me an error when I reverted the patch and re-compiled. Pretty sure I set the COMPILE_DOUBLE option in menuselect to make sure it wasn't being optimized. Oh well glad it's be caught now at least. |
| 19:07.39 | jkroon | kharwell, very likely :). |
| 19:08.05 | jkroon | already tested the -2 patch with C++ as well. |
| 19:08.37 | jkroon | had to unfortunately add the named structure for the previously anonymous usable case to get the sizeof() to work in both C and C++. |
| 19:10.09 | jkroon | kharwell, i've got two patches there, the one stops using argv where it errors out, and the other fixes the bounds of argv. both will work. which is "best" is really up for debate. I prefer smaller patches, but that one does have the potential to polute the namespace a bit. I don't think that's major. Please pick one, it's not required to merge both, and frankly, I don't care which one. |
| 19:44.44 | *** join/#asterisk-dev tmoore (~tmoore@2600:1702:4110:3d10:7621:ad0f:59fc:8170) |
| 19:47.14 | tmoore | hey asterisk dev team, are sorcery extended fields user accessible? I.E. if my app extended the realtime DB structure would prepending an "@", as noted in res_config_odbc.c, make those fields accessible? |
| 19:50.28 | tmoore | The full use case is we need to dynamically change voicemail prompts based on X-.. SIP headers passed from an upstream proxy. I was thinking extending the "voicemail" table in realtime DB to add "vm-login", etc.. could achieve this or possibly adding a dialplan var that app_voicemail checks when running VoicemailMain(). Any suggestions? |
| 22:06.15 | *** join/#asterisk-dev AsteriskRoss_ (~AsteriskR@37.157.48.2) |