hip_firewall_port_cache_uninit_hldb() crashes if init function has not been called previously

[Henrik Ziegeldorf]

This bug applies to branch lp:~rene-hummen/hipl/ipsec_esp.

The function hip_firewall_cache_delete_hldb() produces a segmentation fault if the corresponding initialization function hip_firewall_cache_init_hldb() has not been called during initialization of the firewall.

This can happen if one of the other init-functions (e.g. hip_fw_init_esp_prot(), hip_fw_init_esp_prot_conntrack(), ...) which are called in firewall_init_extensions() exits with an error.

The bug can be reproduced by adding a simple "return -1" in front of the line 547 in firewall/firewall.c and then starting the firewall.

An example for the log of the firewall, when producing this bug, can be found in the attachments.

[Stefan Götz (stefan.goetz-deactivatedaccount)]

I can't find the file filewall/cache_port.c in trunk. I think this is a file that was removed with the changes from the branch lp:~stefan.goetz/hipl/fw-cache-port-cleanup - are you sure that this is a bug in the current trunk? Or am I overlooking something obvious?

[Henrik Ziegeldorf (henrik-ziegeldorf)]

Sorry, I forgot to mention that this is for the userspace ipsec branch, i.e. ~rene-hummen/hipl/ipsec_esp.
And I got the line number wrong: it's line 547 not 587. Anyway, I attached a diff you can use to reproduce the bug.

[Stefan Götz (stefan.goetz-deactivatedaccount)]

I think that merging lp:~stefan.goetz/hipl/fw-cache-port-cleanup or trunk into lp:~rene-hummen/hipl/ipsec_esp would fix this bug, right? Or is there a reason for not doing so?

[René Hummen (rene-hummen)]

trunk has recently been merged to lp:~rene-hummen/hipl/ipsec_esp and includes all fixes from lp:~stefan.goetz/hipl/fw-cache-port-cleanup. I don't think Henrik is talking about lp:~rene-hummen/hipl/ipsec_esp either.

[Henrik Ziegeldorf (henrik-ziegeldorf)]

I don't know whether merging your branch would fix the bug. Merging trunk does not, or at least merging trunk revision 5290 into lp:~rene-hummen/hipl/ipsec_esp, which has recently been done by Rene, does not.

[Stefan Götz (stefan.goetz-deactivatedaccount)]

You say that this bug report refers to the head revision of branch lp:~rene-hummen/hipl/ipsec_esp, correct? The code corresponding to the head revision of the branch lp:~rene-hummen/hipl/ipsec_esp can be found via the launchpad web interface at http://bazaar.launchpad.net/~rene-hummen/hipl/ipsec_esp/files/head%3A/, correct?

It seems to me that the head of this branch does currently not contain the file firewall/cache_port.c, which you refer to in this bug report. It seems that merging hipl trunk into the branch lp:~rene-hummen/hipl/ipsec_esp had the expected result of the file firewall/cache_port.c being removed and no longer being a part of the branch lp:~rene-hummen/hipl/ipsec_esp.

1) It seems to me that this bug can be considered as fixed because the relevant code no longer exists in both hipl trunk and in the branch lp:~rene-hummen/hipl/ipsec_esp

2) If you disagree with my statement 1), please clarify which revision of which branch this bug report refers to. Please provide a web link that shows the affected code.

[Stefan Götz (stefan.goetz-deactivatedaccount)]

Sorry for my repetitive style in the previous comment; it's not meant as an insult. I was just trying to be as precise as possible to avoid misunderstandings.

[Stefan Götz (stefan.goetz-deactivatedaccount)]

Ah, ok, after personal discussion it turns out that the function in question is not hip_firewall_port_cache_uninit_hldb(). Therefore,
the presence of cache_port.c (which previously implemented this function) is irrelevant.

It would be helpful to update this bug report so that it applies to the current trunk or the head of branch lp:~rene-hummen/hipl/ipsec_esp

[Henrik Ziegeldorf (henrik-ziegeldorf)]

You're right, there was some leftover code in my branch. Sorry for the confusion.
However, the bug still applies to branch lp:~rene-hummen/hipl/ipsec_esp, yet at a slightly different point.

I have included the log of the firewall with the forced initialization error from the diff above.
The firewall was started with options -dA.

The log stops at "debug(firewall/cache.c:315@hip_firewall_cache_delete_hldb): Start hldb delete".
After this, the segmentation fault occurs.

[Miika Komu (miika-iki)]

Is somebody going to fix this problem? You seem know what is the problem (?)

[David Martin (martin-lp)]

The problem was that hip_firewall_cache_delete_hldb() iterates over the database that gets initialised in hip_firewall_cache_init_hldb(). It doesn't check whether this initialisation did actually happen hence the segfault. I've committed a fix in trunk revision 5753.