[strongSwan-dev] malloc check policy

[Tobias Brunner]

Hi Emeric,

> Aren't you afraid by potential side effects using this approach?

On systems with overcommit (like Linux by default) malloc() will most
likely return a non-NULL pointer even if there is currently no physical
memory available.  Processes will just get killed by the OOM killer if
that's the case.  But that may be different on other platforms or with
changed kernel configs, so this might have gotten slightly more
problematic since the charon daemon was originally started.

> You may get a NULL pointer and perform some operations with it, like pointer arithmetic, without crashing.

Sure but I don't see our code doing stuff like that.  In most cases
there will be an immediate segmentation fault (e.g. via INIT macro where
the pointer is dereferenced to initialize the struct or if the allocated
memory is used in memset/memcpy/snprintf).

> Furthermore you could possibly have security issues before eventually crash.

Maybe but as mentioned malloc() returning NULL will almost certainly
result in a segmentation fault in our codebase.

> The question is: since you have done the job to get proper malloc hooks with the leak detective, why not just abort on failure?

I hope you are not enabling leak detective on your production systems.
It's quite a hack and has a huge performance impact due to the single lock.

Most allocations are wrapped in calls to the INIT* macros (~1250).
These could theoretically be modified to handle situations where NULL is
returned.  However, some of these calls are in library code where it
might be more appropriate to just return NULL too or OUT_OF_RES instead
of aborting the process, thus, forcing the caller to handle this, which
would require a lot more code (one of the reasons this hasn't been done
so far).  There are also about 190 direct calls to malloc(), about 40 to
realloc() and about 20 to calloc() in our code.  These would all have to
be handled individually (maybe replaced by a macro).  But as said, a
segmentation fault will be caused there if NULL is returned anyway, so
I'm not sure if it's worth changing any of this.

Regards,
Tobias