mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <benjamin.bann...@mesosphere.io>
Subject Re: Review Request 52695: Harden libprocess
Date Thu, 03 Nov 2016 11:11:59 GMT


> On Nov. 2, 2016, 10:32 a.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/Makefile.am, line 30
> > <https://reviews.apache.org/r/52695/diff/4/?file=1551006#file1551006line30>
> >
> >     I am not a big fan of unconditionally omitting frame pointers as this gives
the optimizer one less register to work with. Unfortunately one cannot easily tell the actual
impact of this from the info here. Is this strictly needed here or just nice to have?
> 
> James Peach wrote:
>     The performance benefit of omitting frame pointers is likely to be marginal on x64_64,
if it is a win at all. The rationale for adding this is that it makes stack walking reliable
in all cases, so debugability is improved and you can get reasonable results when uting `perf`.
Since most users will build with default options I suggested to Aaron that we should make
it the default.

Thanks James, that makes sense.

Since this seems all related to debugability what about enabling it _only for builds with
`--enable-debug`_ (e.g., perf results already now also don't necessarily give full info w/o
debug symbols)? Tangentially related, tcmalloc can fail in debug builds with omitted frame
pointers, so disabling `omit-frame-pointer` in debug builds might safe us from some future
headaches, https://bugs.chromium.org/p/chromium/issues/detail?id=636489.

`stack-protector-strong` can significantly increase the binary size, and we should either
only enable it for e.g., debug builds, or give users a `configure` knob to disable it.

For using `FORTIFY_SOURCE` I think we also need be a little more careful. Support for it is
somewhat broken in clang (https://llvm.org/bugs/show_bug.cgi?id=16821), it only has useful
effects in builds with some level of optimization, and can e.g., mess up reports from sanitizers
injected by users. I can see good uses for a `configure` flag to disable this compiler flag,
but I am not sure what the default should be.


- Benjamin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52695/#review154527
-----------------------------------------------------------


On Nov. 2, 2016, 4:14 p.m., Aaron Wood wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52695/
> -----------------------------------------------------------
> 
> (Updated Nov. 2, 2016, 4:14 p.m.)
> 
> 
> Review request for mesos, James Peach, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-6229
>     https://issues.apache.org/jira/browse/MESOS-6229
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Use a default set of flags to provide additional security and hardening to libprocess.
Additionally, check and catch more warnings/errors.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/Makefile.am 7131989 
>   3rdparty/libprocess/configure.ac 1644035 
>   3rdparty/libprocess/m4/ax_check_compile_flag.m4 PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52695/diff/
> 
> 
> Testing
> -------
> 
> Compared the benchmarks with and without the flags being used. Also did a comparsion
with the flags being used with and without optimizations and without the flags being used
with and without optimizations. Overall the performance hit was very small with a 3-8% overhead
(optimizations brings this down slightly). Most benchmarks were about 5% (or less) slower.
> 
> 
> File Attachments
> ----------------
> 
> --enable-optimized with hardening applied
>   https://reviews.apache.org/media/uploaded/files/2016/11/02/875c9e6e-c73b-4e3c-8265-0f7c6dc00351__hardened-optimized.txt
> Hardening applied but no --enable-optimized
>   https://reviews.apache.org/media/uploaded/files/2016/11/02/932d28a7-2d31-471a-b438-647841a6853c__hardened-unoptimized.txt
> --enable-optimized with no hardening applied
>   https://reviews.apache.org/media/uploaded/files/2016/11/02/896944ea-9b31-4d62-b1b9-97fb4700a882__optimized.txt
> No hardening applied and no --enable-optimized
>   https://reviews.apache.org/media/uploaded/files/2016/11/02/b32667ce-3e3b-4d2b-b4f8-4c2404a0fc1c__unoptimized.txt
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message