[Openswan dev] connections.c

[D. Hugh Redelmeier]

| From: Anthony Tong <atong at TrustedCS.com>

| This is in regards to 
| http://lists.openswan.org/pipermail/cvs/2008-February/006217.html
| 
| alg_ike and alg_esp needs to be cloned in unshare_connection_strings() 
| to avoid the possible double free.

Yes.  This is a discipline that I should have documented better.
Perhaps each such field in struct connection should be commented as
"private copy" or "owned by connection".

When someone came along later and added similar fields, they
apparently did not understand the protocol.

Summary:

There are some strings and chunks of memory that are owned by a
struct connection.  Pointers to them are in fields of struct
connection.

When the struct connection is created, each of these fields must be
populated by a pointer to a copy of the string value or chunk value: it
cannot be shared with any other "owner".  The reason is that when the
struct connection is to be freed, all of these strings and chunks will
be freed too.

One way of creating a new struct connection is through cloning.
Whenever a clone is created, these owned strings must be cloned too
(if they would ahve been shared).  That is done by
unshare_connection_strings.

There are strings not owned by struct connection.  They are some other
logic's job to allocate and free.

In effect, "y owned by x" means "x has the right/duty/obligation to
allocate and free the resources of y".

| However for our build, we chose to avoid this issue by not saving the 
| alg_ike & alg_esp strings in add_connection() in the first place, as 
| they never seem to be used anyway. (well, except for a log message in 2.6)

This is not the clean solution, but you knew that.

I don't know what these strings are intended for.

As you say, alg_esp is not used.  Either it should be used or it
should be eliminated.

They are only set if KERNEL_ALG or IKE_ALG are defined.  Perhaps the
definition and all uses of them should be similarly conditional.

If they remain, they should certainly be cloned in
unshare_connection_strings.  Why did you not use this fix?

| I havent extensively tested 2.6.x yet, but in 2.4.x it solved one of the 
| last pluto memory leaks for us; a heavily loaded pluto stayed at around 
| 15m and nothing unreasonable reported from LEAK_DETECTIVE.

Good to hear.