Potential memory leak in ike_sa_manager during termination of the daemon
I run Address Sanitizer (https://github.com/google/sanitizers/wiki/AddressSanitizer) tool and found memory leak in ike_sa_manager
It happens when new trap acquired during shut down of the daemon and created the followings flow
1) trap_manger->acquire start
2) The daemon start deinitialization
3) ike_sa_manager flushed https://github.com/strongswan/strongswan/blob/master/src/libcharon/daemon.c#L653
4) The trap manager call checkout_by_config https://github.com/strongswan/strongswan/blob/master/src/libcharon/sa/trap_manager.c#L507
5) The ike_sa_manager call checkout_new https://github.com/strongswan/strongswan/blob/master/src/libcharon/sa/ike_sa_manager.c#L1445
7) checkout_new create new ike sa https://github.com/strongswan/strongswan/blob/master/src/libcharon/sa/ike_sa_manager.c#L1191
8) trap_manager checkin the new ike sa and create new entry in ike_sa_manager https://github.com/strongswan/strongswan/blob/master/src/libcharon/sa/trap_manager.c#L528
9) the daemon continue its shut down (and flushing the trap manager) but does not free the entry in ike_sa_manager and doesn't free the ike_sa object.
Attach patch with suggested for local fix to this leak, although, maybe it will be better to flush first the trap_manager and wait for all the acquire calls to finish and then flush the ike_sa_manager in daemon.c https://github.com/strongswan/strongswan/blob/master/src/libcharon/daemon.c#L652
ike-sa-manager: Avoid memory leak if IKE_SAs get checked in after flush() was called
A thread might check out a new IKE_SA via checkout_new() or
checkout_by_config() and start initiating it while the daemon is
terminating and the IKE_SA manager is flushed by the main thread.
That SA is not tracked yet so the main thread is not waiting for it and
the other thread is able to check it in and creating an entry after flush()
already terminated causing a memory leak.
#1 Updated by Tobias Brunner over 4 years ago
- Status changed from New to Feedback
Thanks for the detailed report.
maybe it will be better to flush first the trap_manager and wait for all the acquire calls to finish
I guess we could do that, but I can see other cases where there is a call to
checkout_by_config()) right before the IKE_SA manager gets flushed (simply initiating an SA will do that). We could do what your patch does and reject these IKE_SAs after
flush() has been called, but that would require checking a flag for each call of
checkin(). Another option is to do parts of
flush() again in
destroy() to avoid the leak. I've done that in the 1348-ikesa-manager-destroy branch.
#3 Updated by Tobias Brunner over 4 years ago
- Subject changed from Memory leak in ike_sa_manager to Potential memory leak in ike_sa_manager during termination of the daemon
- Status changed from Feedback to Closed
- Assignee set to Tobias Brunner
- Target version set to 5.5.0
- Resolution set to Fixed
I tested branch 1348-ikesa-manager-destroy and it looks good.
Thanks, I merged the fix to master.