Comment 9 for bug 790679

Hi,

> But ok, another option is to re-organize the initialization so that that
> variable gets set before it is possible to abort the initialization.
> Basically move the hip_xfrm_set_nl_ipsec() call to somewhere before the
> first HIP_IFEL.
>
> .. I'll attach a patch now that I actually tried it out.

I would say that moving the initialization only fixes the sympton and not the problem. :) What if the initialization for what you are uninitializing fails? It should be robust in any case! In addition the documentation states that hip_xfrm_set_nl_ipsec() gets an initialized netlink socket. If you move this call further forward it will definitely not get an initialized socket because this happens in rtnl_open_byproto() right before it.

What happens here is the following:
We have got a rtnl_handle struct to store the field descriptor and sequence number and stuff for our netlink socket. It is static and located in hipd.c. Our IPsec implementation is in xfrmapi.c and has no direct access to it. What it has instead is a static pointer that gets pointed to the handle struct in the initialization. This does not happen if anything in the initialization already fails before.

The problem here is not really the uninitialization. The null pointer gets passed through all functions that try to remove the default policies that would usually be set up on startup. We could do sanity checks on the pointers in these functions or in the exit call like Miika proposes but this should not really be necessary. In most cases the sockets are correctly initialized and it does not really matter for the calls.

As it turns out netlink_talk() that sends messages on the netlink sockets is not robust. It assumes to get a valid netlink socket and as a consequence segfaults if it doesn't. This is what happens here. I have committed a fix in trunk revision 5950.

Jookos, please check if it works for you.