Project

General

Profile

Bug #1348

Potential memory leak in ike_sa_manager during termination of the daemon

Added by Avinoam Meir over 4 years ago. Updated over 4 years ago.

Status:
Closed
Priority:
Normal
Category:
libcharon
Target version:
Start date:
13.03.2016
Due date:
Estimated time:
Affected version:
5.3.5
Resolution:
Fixed

Description

Hello,

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

Associated revisions

Revision 958c0e8e (diff)
Added by Tobias Brunner over 4 years ago

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.

Fixes #1348.

History

#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_new() (or 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.

#2 Updated by Avinoam Meir over 4 years ago

I tested branch 1348-ikesa-manager-destroy and it looks good.

#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.

Also available in: Atom PDF