Segmentation Fault due to method route_list_add_vpn_gateway

[wclpat]

Hi,

I figured out that the actual version of openvpn within GIT has the same problem as the version I'm usually using on ArchLinux? (at the moment: core/openvpn 2.3.0-2).

I started already debugging openvpn with GDB and realized that it crashes when it tries to execute the methode "route_list_add_vpn_gateway" within the route.c file:

Bug:

Program received signal SIGSEGV, Segmentation fault.
0x000000000046644c in route_list_add_vpn_gateway (rl=0x0, es=0x6d3da8, addr=2259231230) at route.c:506
506	  rl->spec.remote_endpoint = addr;
(gdb) display rl
1: rl = (struct route_list *) 0x0
(gdb) 

particular code:

void
route_list_add_vpn_gateway (struct route_list *rl,
          struct env_set *es,
          const in_addr_t addr)
{
  rl->spec.remote_endpoint = addr;
  rl->spec.flags |= RTSA_REMOTE_ENDPOINT;
  setenv_route_addr (es, "vpn_gateway", rl->spec.remote_endpoint, -1);
}

The major problem is that it tries to access c->c1.route_list (within the file forward.c which is at this point 0x0. I didn't specified a --route flag (see client.conf below) so that the route_list struct doesn't get initialized within the method "do_alloc_route_list" (file init.c).

particular forward.c code:

       /* possibly extract a DHCP router message */
        if (flags & PIPV4_EXTRACT_DHCP_ROUTER)
    {
      const in_addr_t dhcp_router = dhcp_extract_router_msg (&ipbuf);
      if (dhcp_router)
        route_list_add_vpn_gateway (c->c1.route_list, c->c2.es, dhcp_router);
    }

My client.conf looks as follows:

client
dev             tap0
proto           tcp
remote          *.*.*.* 443
resolv-retry    infinite
nobind
persist-key
persist-tun
ca              "ca.crt"
cert            "ps.crt"
key             "ps.key"
ns-cert-type     server
comp-lzo
verb            3
script-security 2

The way I execute openvpn is as follows:

$ openvpn client.conf

as soon as above says "Initialization Sequence Completed" I type:

$ dhclient tap0

I issue the dhclient by hand, because I get the following printed:

Thu Apr 18 21:00:57 2013 SENT CONTROL []: 'PUSH_REQUEST' (status=1)
Thu Apr 18 21:00:58 2013 PUSH: Received control message: 'PUSH_REPLY,route-gateway dhcp,ping 10,ping-restart 120'
Thu Apr 18 21:00:58 2013 OPTIONS IMPORT: timers and/or timeouts modified
Thu Apr 18 21:00:58 2013 OPTIONS IMPORT: route-related options modified
Thu Apr 18 21:00:58 2013 TUN/TAP device tap0 opened
Thu Apr 18 21:00:58 2013 TUN/TAP TX queue length set to 100
Thu Apr 18 21:00:58 2013 Initialization Sequence Completed
Thu Apr 18 21:01:02 2013 Extracted DHCP router address: *.*.*.*

I hope that helps you guys fixing this bug.

Best regards

[Gert Döring]

Thanks for the analysis (which makes very much sense). I'll see that I can whip up a patch today, shouldn't be too hard. Just need to find a reasonable place to initialize the route_list (or just "always do it").

[Gert Döring]

Fixing this is actually harder than I thought.

At the place where it bombs (route_list_add_vpn_gateway) we do not have enough context to properly allocate a route_list.

So either it's "if no route list allocated, log error and ignore DHCP packet" or "always allocate route list, even if no route statements have been configured or pushed".

[wclpat]

I would prefer the second suggestion. But I'm not sure how you handle that usually.
By the way, I'm sorry that I haven't looked first into your entire bug tracker, but I saw that somebody created the same ticket already (just with way less details): ticket #258

[Gert Döring]

well, yes, this is likely to be a duplicate of #258, but since you gave us all the details to pinpoint the location, it's good that you brought this to our attention again :-) - #258 was sitting somewhere in my "when I have time I need to build a test setup and see what happens" queue...

[Gert Döring]

this should both fix the segv (due to checking rl via ASSERT) and the underlying problem (route_list is now always initialized)

[Gert Döring]

wclpat, could you please test whether the attached patch fixes the crash for you? It certainly should, but I'm lacking a test setup so only compile-tested it.

Patch should apply fine to 2.3.1 sources from git.

[Gert Döring]

It would have been great to have some feedback, but since the reporter of #258 tested the patch and reported that it fixes the issue for him, I assume it will also fix it here.

I have applied the patch to the git tree today (master and release/2.3 branch), and we'll release 2.3.2 "real soon" with this fix.