I just pushed a patch to master, but I didn't have the geoip libs installed, thus no test.

If everything is ok, you can backport at your convenience, otherwise I will do it when I get the first chance.

Cheers, Daniel

On 27/02/14 08:44, Juha Heinanen wrote:

Daniel-Constantin Mierla writes:

...

Can you send the backtrace? Getting geo db and setting up a testing environment to reproduce takes time.

below, juha

(gdb) where #0 pv_get_geoip (msg=0xb722976c, param=0xb6fe3778, res=0xbf8e7c9c) at geoip_pv.c:329 #1 0x080d7714 in pv_get_spec_value (msg=0xb722976c, sp=0xb6fe376c, value=0xbf8e7c9c) at pvapi.c:1266 #2 0x080a6841 in lval_pvar_assign (h=0xbf8e878c, msg=0xb722976c, lv=0xb6fe3444, rv=0xb6fe3764) at lvalue.c:345 #3 0x080a6ecc in lval_assign (h=0xbf8e878c, msg=0xb722976c, lv=0xb6fe3444, rve=0xb6fe3760) at lvalue.c:410 #4 0x080665e3 in do_action (h=0xbf8e878c, a=0xb6fe3b14, msg=0xb722976c) at action.c:1478 #5 0x08067293 in run_actions (h=0xbf8e878c, a=0xb6fe3b14, msg=0xb722976c) at action.c:1599 #6 0x0805e00d in do_action (h=0xbf8e878c, a=0xb6fe2718, msg=0xb722976c) at action.c:715 #7 0x08067293 in run_actions (h=0xbf8e878c, a=0xb6fe2718, msg=0xb722976c) at action.c:1599 #8 0x0805fc5f in do_action (h=0xbf8e878c, a=0xb6fe30dc, msg=0xb722976c) at action.c:1090 #9 0x08067293 in run_actions (h=0xbf8e878c, a=0xb6fdadfc, msg=0xb722976c) at action.c:1599 #10 0x0806797a in run_top_route (a=0xb6fdadfc, msg=0xb722976c, c=0x0) at action.c:1685 #11 0x080e2973 in receive_msg ( buf=0x8650528 "REGISTER sip:test.tutpro.com SIP/2.0\r\nVia: SIP/2.0/TCP 192.98.102.30:5064;branch=z9hG4bK69de950454ae053b;rport\r\nContact: sip:0x975a7a8@192.98.102.30:5064;transport=tcp;expires=600;+sip.instance="<ur"..., len=616, rcv_info=0xb4b22058) at receive.c:211 #12 0x08162874 in receive_tcp_msg ( tcpbuf=0xb4b22210 "REGISTER sip:test.tutpro.com SIP/2.0\r\nVia: SIP/2.0/TCP 192.98.102.30:5064;branch=z9hG4bK69de950454ae053b;rport\r\nContact: sip:0x975a7a8@192.98.102.30:5064;transport=tcp;expires=600;+sip.instance="<ur"..., len=616, rcv_info=0xb4b22058, con=0xb4b22044) at tcp_read.c:1232 #13 0x08163624 in tcp_read_req (con=0xb4b22044, bytes_read=0xbf8e8b40, read_flags=0xbf8e8b3c) at tcp_read.c:1387 #14 0x081649be in handle_io (fm=0xb7528c70, events=1, idx=-1) at tcp_read.c:1559 #15 0x0815deb9 in io_wait_loop_epoll (h=0x82d60c0, t=2, repeat=0) at io_wait.h:1092 #16 0x081658ba in tcp_receive_loop (unix_sock=42) at tcp_read.c:1728 #17 0x08158ae3 in tcp_init_children () at tcp_main.c:4959 #18 0x080adcee in main_loop () at main.c:1702 ---Type <return> to continue, or q <return> to quit--- #19 0x080b088f in main (argc=17, argv=0xbf8e8f54) at main.c:2533

On 02/03/14 02:59, Juha Heinanen wrote:

Daniel-Constantin Mierla writes:

...

I just pushed a patch to master, but I didn't have the geoip libs installed, thus no test.

If everything is ok, you can backport at your convenience, otherwise I will do it when I get the first chance.

i tried and noticed that i don't have a working master at this time, only 4.1.

Not clear for me -- have you tested with 4.1 and all is ok?

Cheers, Daniel

On 03/03/14 19:29, Juha Heinanen wrote:

Daniel-Constantin Mierla writes:

...

Not clear for me -- have you tested with 4.1 and all is ok?

i thought that you didn't cherry pick the commit to 4.1 yet.

No, but you can do it and if all goes fine, it can be pushed to remote repository. I don't have a geoip deployment at hand these days.

Cheers, Daniel

I pushed more safety checks for this case. Can you give it another try with the patch from the master (along with the older one)?

Cheers, Daniel

On 03/03/14 21:35, Juha Heinanen wrote:

Daniel-Constantin Mierla writes:

...

No, but you can do it and if all goes fine, it can be pushed to remote repository. I don't have a geoip deployment at hand these days.

i backported the patch to my 4.1 source and still got a crash (i don't remember if this is the same or new one).

-- juha

Program terminated with signal 11, Segmentation fault. #0 pv_get_geoip (msg=0xb72316a4, param=0xb6fe0ce4, res=0xbfb622fc) at geoip_pv.c:332 332 return pv_geoip_get_strzval(msg, param, res, (gdb) where #0 pv_get_geoip (msg=0xb72316a4, param=0xb6fe0ce4, res=0xbfb622fc) at geoip_pv.c:332 #1 0x080d7714 in pv_get_spec_value (msg=0xb72316a4, sp=0xb6fe0cd8, value=0xbfb622fc) at pvapi.c:1266 #2 0x080a6841 in lval_pvar_assign (h=0xbfb6272c, msg=0xb72316a4, lv=0xb6fe097c, rv=0xb6fe0cd0) at lvalue.c:345 #3 0x080a6ecc in lval_assign (h=0xbfb6272c, msg=0xb72316a4, lv=0xb6fe097c, rve=0xb6fe0ccc) at lvalue.c:410 #4 0x080665e3 in do_action (h=0xbfb6272c, a=0xb6fe1080, msg=0xb72316a4) at action.c:1478 #5 0x08067293 in run_actions (h=0xbfb6272c, a=0xb6fe1080, msg=0xb72316a4) at action.c:1599 #6 0x0806797a in run_top_route (a=0xb6fe1080, msg=0xb72316a4, c=0x0) at action.c:1685 #7 0x080e2973 in receive_msg ( buf=0x9df95e8 "REGISTER sip:test.tutpro.com SIP/2.0\r\nVia: SIP/2.0/TCP 192.98.102.30:5064;branch=z9hG4bK7db1d82ab09c855c;rport\r\nContact: sip:0x9c447b8@192.98.102.30:5064;transport=tcp;expires=600;+sip.instance="<ur"..., len=616, rcv_info=0xb4b2a33c) at receive.c:211 #8 0x08162874 in receive_tcp_msg ( tcpbuf=0xb4b2a4f4 "REGISTER sip:test.tutpro.com SIP/2.0\r\nVia: SIP/2.0/TCP 192.98.102.30:5064;branch=z9hG4bK7db1d82ab09c855c;rport\r\nContact: sip:0x9c447b8@192.98.102.30:5064;transport=tcp;expires=600;+sip.instance="<ur"..., len=616, rcv_info=0xb4b2a33c, con=0xb4b2a328) at tcp_read.c:1232 #9 0x08163624 in tcp_read_req (con=0xb4b2a328, bytes_read=0xbfb62ae0, read_flags=0xbfb62adc) at tcp_read.c:1387 #10 0x081649be in handle_io (fm=0xb7530ba8, events=1, idx=-1) at tcp_read.c:1559 #11 0x0815deb9 in io_wait_loop_epoll (h=0x82d60c0, t=2, repeat=0) at io_wait.h:1092 #12 0x081658ba in tcp_receive_loop (unix_sock=42) at tcp_read.c:1728 #13 0x08158ae3 in tcp_init_children () at tcp_main.c:4959 #14 0x080adcee in main_loop () at main.c:1702 #15 0x080b088f in main (argc=17, argv=0xbfb62ef4) at main.c:2533

Can you give here what is in your file geoip_pv.c at line 334? I find there a return that should not be a reason for crash. Be sure you have the two patches done lately to the module.

Cheers, Daniel

On 04/03/14 10:02, Juha Heinanen wrote:

Daniel-Constantin Mierla writes:

...

I pushed more safety checks for this case. Can you give it another try with the patch from the master (along with the older one)?

didn't help, still crashing.

-- juha

(gdb) where #0 pv_get_geoip (msg=0xb72206a4, param=0xb6fcfce4, res=0xbfc0367c) at geoip_pv.c:334 #1 0x080d7714 in pv_get_spec_value (msg=0xb72206a4, sp=0xb6fcfcd8, value=0xbfc0367c) at pvapi.c:1266 #2 0x080a6841 in lval_pvar_assign (h=0xbfc03aac, msg=0xb72206a4, lv=0xb6fcf97c, rv=0xb6fcfcd0) at lvalue.c:345 #3 0x080a6ecc in lval_assign (h=0xbfc03aac, msg=0xb72206a4, lv=0xb6fcf97c, rve=0xb6fcfccc) at lvalue.c:410 #4 0x080665e3 in do_action (h=0xbfc03aac, a=0xb6fd0080, msg=0xb72206a4) at action.c:1478 #5 0x08067293 in run_actions (h=0xbfc03aac, a=0xb6fd0080, msg=0xb72206a4) at action.c:1599 #6 0x0806797a in run_top_route (a=0xb6fd0080, msg=0xb72206a4, c=0x0) at action.c:1685 #7 0x080e2973 in receive_msg ( buf=0x88fc590 "REGISTER sip:test.tutpro.com SIP/2.0\r\nVia: SIP/2.0/TCP 192.98.102.30:5064;branch=z9hG4bK1ec15dee79452bc0;rport\r\nContact: sip:0x9eb17c8@192.98.102.30:5064;transport=tcp;expires=600;+sip.instance="<ur"..., len=616, rcv_info=0xb4b1933c) at receive.c:211 #8 0x08162874 in receive_tcp_msg ( tcpbuf=0xb4b194f4 "REGISTER sip:test.tutpro.com SIP/2.0\r\nVia: SIP/2.0/TCP 192.98.102.30:5064;branch=z9hG4bK1ec15dee79452bc0;rport\r\nContact: sip:0x9eb17c8@192.98.102.30:5064;transport=tcp;expires=600;+sip.instance="<ur"..., len=616, rcv_info=0xb4b1933c, con=0xb4b19328) at tcp_read.c:1232 #9 0x08163624 in tcp_read_req (con=0xb4b19328, bytes_read=0xbfc03e60, read_flags=0xbfc03e5c) at tcp_read.c:1387 #10 0x081649be in handle_io (fm=0xb751fba8, events=1, idx=-1) at tcp_read.c:1559 #11 0x0815deb9 in io_wait_loop_epoll (h=0x82d60c0, t=2, repeat=0) at io_wait.h:1092 #12 0x081658ba in tcp_receive_loop (unix_sock=42) at tcp_read.c:1728 #13 0x08158ae3 in tcp_init_children () at tcp_main.c:4959 #14 0x080adcee in main_loop () at main.c:1702 #15 0x080b088f in main (argc=17, argv=0xbfc04274) at main.c:2533

You don't have the two patches, on master, the code there is:

default: /* cc */ if(gpv->item->r.record==NULL) return pv_get_null(msg, param, res); return pv_geoip_get_strzval(msg, param, res, gpv->item->r.record->country_code);

Be sure you pick commits:

721ffe3576e8e7891c27f8578eb244a857ede759 56ed48bf48c3d78ff5d214833e09a5759f0b5928

Cheers, Daniel

On 04/03/14 22:49, Juha Heinanen wrote:

Daniel-Constantin Mierla writes:

...

Can you give here what is in your file geoip_pv.c at line 334? I find there a return that should not be a reason for crash. Be sure you have the two patches done lately to the module.

i may have lost what is going on, but i do have return statement on that line and that statement is referencing gpv item:

default: /* cc */

/*334/* return pv_geoip_get_strzval(msg, param, res, gpv->item->r.record->country_code);

if you email me a single patch against 4.1, i can give another try. on the other hand, it should be very easy for you to make this test too:

$ apt-get install libgeoip1 geoip-database-contrib

and then in config

loadmodule "geoip" modparam("geoip", "path", "/usr/share/GeoIP/GeoIPCity.dat")

and the test statement:

 $var(tmp) = $gip(src=>cc);

-- juha

On 04/03/14 23:55, Juha Heinanen wrote:

Daniel-Constantin Mierla writes:

...

You don't have the two patches, on master, the code there is:

      default: /* cc */
          if(gpv->item->r.record==NULL)
              return pv_get_null(msg, param, res);
          return pv_geoip_get_strzval(msg, param, res,
                  gpv->item->r.record->country_code);

Be sure you pick commits:

721ffe3576e8e7891c27f8578eb244a857ede759 56ed48bf48c3d78ff5d214833e09a5759f0b5928

i tries picking one of the commit, but it failed and i edited the file manually. if you give me a single patch against 4.1, i'm able to try it out.

you can make the diff between master and 4.1 branches for geoip module - there were no other changes iirc. Or wait for the new release, I am going to backport anyhow.

Daniel

On 06/03/14 04:20, Juha Heinanen wrote:

Daniel-Constantin Mierla writes:

...

you can make the diff between master and 4.1 branches for geoip module - there were no other changes iirc. Or wait for the new release, I am going to backport anyhow.

i saw that you had backported the patches to 4.1. i tried with latest 4.1 and didn't get the crash anymore.

ok, thanks for testing. Cheers, Daniel