Re: Review Request 52695: Harden libprocess

2018-05-11 Thread Benjamin Mahler 


> On Nov. 2, 2016, 9:32 a.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/Makefile.am
> > Lines 30 (patched)
> > 
> >
> > 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.
> 
> Benjamin Bannier wrote:
> 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.
> 
> Aaron Wood wrote:
> Going to drop this since we've all agreed on Slack to have the frame 
> pointer modification done in a separate patch.

Looks like there wasn't a patch for `-fno-omit-frame-pointer`?

Filed: https://issues.apache.org/jira/browse/MESOS-8908


- Benjamin


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


On Nov. 30, 2016, 8:52 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52695/
> ---
> 
> (Updated Nov. 30, 2016, 8:52 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
> ---
> 
> Add hardened flags for libprocess.
> Take compile flag macro at 391cb680171d3889965b1ead43d3a326c913bc25.
> The macro at 1a869696e4129279f7b99c3f9052717354b79a86 requires autoconf 2.64 
> which breaks on CentOS 6.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 9d496b8 
>   3rdparty/libprocess/configure.ac e65e5ca 
>   3rdparty/libprocess/m4/ax_check_compile_flag.m4 PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/52695/diff/9/
> 
> 
> 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
> 
>

Re: Review Request 52695: Harden libprocess

2016-11-30 Thread Aaron Wood 

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

(Updated Nov. 30, 2016, 8:52 p.m.)


Review request for mesos, James Peach, Michael Park, and Neil Conway.


Changes
---

Don't warn when stack protection isn't used.


Bugs: MESOS-6229
https://issues.apache.org/jira/browse/MESOS-6229


Repository: mesos


Description
---

Add hardened flags for libprocess.
Take compile flag macro at 391cb680171d3889965b1ead43d3a326c913bc25.
The macro at 1a869696e4129279f7b99c3f9052717354b79a86 requires autoconf 2.64 
which breaks on CentOS 6.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am 9d496b8 
  3rdparty/libprocess/configure.ac e65e5ca 
  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

Re: Review Request 52695: Harden libprocess

2016-11-30 Thread Aaron Wood 

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

(Updated Nov. 30, 2016, 5:12 p.m.)


Review request for mesos, James Peach, Michael Park, and Neil Conway.


Changes
---

Fixed newline issue in the macro.


Bugs: MESOS-6229
https://issues.apache.org/jira/browse/MESOS-6229


Repository: mesos


Description
---

Add hardened flags for libprocess.
Take compile flag macro at 391cb680171d3889965b1ead43d3a326c913bc25.
The macro at 1a869696e4129279f7b99c3f9052717354b79a86 requires autoconf 2.64 
which breaks on CentOS 6.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am 9d496b8 
  3rdparty/libprocess/configure.ac e65e5ca 
  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

Re: Review Request 52695: Harden libprocess

2016-11-29 Thread Aaron Wood 

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

(Updated Nov. 29, 2016, 4:26 p.m.)


Review request for mesos, James Peach, Michael Park, and Neil Conway.


Changes
---

Changed version of macro to work with CentOS 6.


Bugs: MESOS-6229
https://issues.apache.org/jira/browse/MESOS-6229


Repository: mesos


Description (updated)
---

Add hardened flags for libprocess.
Take compile flag macro at 391cb680171d3889965b1ead43d3a326c913bc25.
The macro at 1a869696e4129279f7b99c3f9052717354b79a86 requires autoconf 2.64 
which breaks on CentOS 6.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am 7131989 
  3rdparty/libprocess/configure.ac e65e5ca 
  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

Re: Review Request 52695: Harden libprocess

2016-11-17 Thread Michael Park 

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


Ship it!




Ship It!

- Michael Park


On Nov. 9, 2016, 11:07 a.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52695/
> ---
> 
> (Updated Nov. 9, 2016, 11:07 a.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
> ---
> 
> Add hardened flags for libprocess.
> Take compile flag macro at 1a869696e4129279f7b99c3f9052717354b79a86.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 7131989 
>   3rdparty/libprocess/configure.ac e65e5ca 
>   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
> 
>

Re: Review Request 52695: Harden libprocess

2016-11-09 Thread Benjamin Bannier 

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


Ship it!




Lots great, thanks a lot.

- Benjamin Bannier


On Nov. 9, 2016, 8:07 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52695/
> ---
> 
> (Updated Nov. 9, 2016, 8:07 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
> ---
> 
> Add hardened flags for libprocess.
> Take compile flag macro at 1a869696e4129279f7b99c3f9052717354b79a86.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 7131989 
>   3rdparty/libprocess/configure.ac e65e5ca 
>   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
> 
>

Re: Review Request 52695: Harden libprocess

2016-11-09 Thread Aaron Wood 

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

(Updated Nov. 9, 2016, 7:07 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 (updated)
---

Add hardened flags for libprocess.
Take compile flag macro at 1a869696e4129279f7b99c3f9052717354b79a86.


Diffs
-

  3rdparty/libprocess/Makefile.am 7131989 
  3rdparty/libprocess/configure.ac e65e5ca 
  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

Re: Review Request 52695: Harden libprocess

2016-11-08 Thread Benjamin Bannier 


> On Nov. 2, 2016, 10:32 a.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/m4/ax_check_compile_flag.m4, line 1
> > 
> >
> > For future updates it would be great if we'd write down the 
> > autoconf-archive release this file came from (it looks like the latest 
> > release containing it is `v2016.09.16`).
> 
> Aaron Wood wrote:
> I don't see any of the other macros having this information. Would you 
> just prefer a comment at the very top indicating the release?
> I took this from HEAD a few weeks back from this location 
> http://git.savannah.gnu.org/gitweb/?p=autoconf-archive.git;a=blob_plain;f=m4/ax_check_compile_flag.m4
>  How can you tell it's from `v2016.09.16`?
> 
> Benjamin Bannier wrote:
> Yes, currently this doesn't exist in many places, but I think it would be 
> great to at least mention to upstream commit SHA1 you took this from, e.g., 
> in this commit message. This can potentially make applying new upstream 
> version of this file easier should we ever patch it.
> 
> What I did to tell what release this was is in is to checkout the 
> upstream repo, and find which version it matched (`HEAD`). This can become 
> harder if we patch the file substantially, or upstream reorganizes their code.

You marked this as resolved, but I couldn't find the change. Could you please 
update e.g., the commit message to include something like

This commit adds ax_check_compiler_flag.m4 from 
git://git.sv.gnu.org/autoconf-archive.git tag v2016.09.16.


- Benjamin


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


On Nov. 8, 2016, 6:41 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52695/
> ---
> 
> (Updated Nov. 8, 2016, 6:41 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 e65e5ca 
>   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
> 
>

Re: Review Request 52695: Harden libprocess

2016-11-08 Thread Aaron Wood 

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

(Updated Nov. 8, 2016, 5:41 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 e65e5ca 
  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

Re: Review Request 52695: Harden libprocess

2016-11-07 Thread Aaron Wood 

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

(Updated Nov. 7, 2016, 9:51 p.m.)


Review request for mesos, James Peach, Michael Park, and Neil Conway.


Changes
---

Specify the hash in the commit message for the macro we took 
(1a869696e4129279f7b99c3f9052717354b79a86).


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 (updated)
-

  3rdparty/libprocess/Makefile.am 7131989 
  3rdparty/libprocess/configure.ac e65e5ca 
  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

Re: Review Request 52695: Harden libprocess

2016-11-07 Thread Benjamin Bannier 


> On Nov. 2, 2016, 10:32 a.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/m4/ax_check_compile_flag.m4, line 1
> > 
> >
> > For future updates it would be great if we'd write down the 
> > autoconf-archive release this file came from (it looks like the latest 
> > release containing it is `v2016.09.16`).
> 
> Aaron Wood wrote:
> I don't see any of the other macros having this information. Would you 
> just prefer a comment at the very top indicating the release?
> I took this from HEAD a few weeks back from this location 
> http://git.savannah.gnu.org/gitweb/?p=autoconf-archive.git;a=blob_plain;f=m4/ax_check_compile_flag.m4
>  How can you tell it's from `v2016.09.16`?

Yes, currently this doesn't exist in many places, but I think it would be great 
to at least mention to upstream commit SHA1 you took this from, e.g., in this 
commit message. This can potentially make applying new upstream version of this 
file easier should we ever patch it.

What I did to tell what release this was is in is to checkout the upstream 
repo, and find which version it matched (`HEAD`). This can become harder if we 
patch the file substantially, or upstream reorganizes their code.


- Benjamin


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


On Nov. 7, 2016, 10:30 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52695/
> ---
> 
> (Updated Nov. 7, 2016, 10:30 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 e65e5ca 
>   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
> 
>

Re: Review Request 52695: Harden libprocess

2016-11-07 Thread Aaron Wood 

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

(Updated Nov. 7, 2016, 9:30 p.m.)


Review request for mesos, James Peach, Michael Park, and Neil Conway.


Changes
---

Addressed comments, added a new flag to enable/disable hardening, apply 
hardening by default.


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 (updated)
-

  3rdparty/libprocess/Makefile.am 7131989 
  3rdparty/libprocess/configure.ac e65e5ca 
  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

Re: Review Request 52695: Harden libprocess

2016-11-07 Thread Aaron Wood 


> On Nov. 2, 2016, 9:32 a.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/Makefile.am, line 30
> > 
> >
> > 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.
> 
> Benjamin Bannier wrote:
> 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.

Going to drop this since we've all agreed on Slack to have the frame pointer 
modification done in a separate patch.


- Aaron


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


On Nov. 2, 2016, 3: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, 3: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
> 
>

Re: Review Request 52695: Harden libprocess

2016-11-03 Thread Aaron Wood 


> On Nov. 2, 2016, 9:32 a.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/m4/ax_check_compile_flag.m4, line 1
> > 
> >
> > For future updates it would be great if we'd write down the 
> > autoconf-archive release this file came from (it looks like the latest 
> > release containing it is `v2016.09.16`).

I don't see any of the other macros having this information. Would you just 
prefer a comment at the very top indicating the release?
I took this from HEAD a few weeks back from this location 
http://git.savannah.gnu.org/gitweb/?p=autoconf-archive.git;a=blob_plain;f=m4/ax_check_compile_flag.m4
 How can you tell it's from `v2016.09.16`?


- Aaron


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


On Nov. 2, 2016, 3: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, 3: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
> 
>

Re: Review Request 52695: Harden libprocess

2016-11-03 Thread Benjamin Bannier 


> On Nov. 2, 2016, 10:32 a.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/Makefile.am, line 30
> > 
> >
> > 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
> 
>

Re: Review Request 52695: Harden libprocess

2016-11-02 Thread James Peach 


> On Nov. 2, 2016, 9:32 a.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/Makefile.am, line 30
> > 
> >
> > 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?

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.


- James


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


On Nov. 2, 2016, 3: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, 3: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
> 
>

Re: Review Request 52695: Harden libprocess

2016-11-02 Thread Aaron Wood 


> On Nov. 2, 2016, 9:32 a.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/Makefile.am, line 29
> > 
> >
> > Let's not suppress this valid and potentially useful diagnostic for the 
> > whole codebase. It does not trigger a hard failure anyway.

You're right, I had initially set this due to issues with the version of gmock 
that's used and clang. Now that there's no `-Werror` I'll take it out.


- Aaron


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


On Nov. 2, 2016, 3: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, 3: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
> 
>

Re: Review Request 52695: Harden libprocess

2016-11-02 Thread Aaron Wood 

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

(Updated Nov. 2, 2016, 3:14 p.m.)


Review request for mesos, James Peach, Michael Park, and Neil Conway.


Changes
---

Added benchmarks that were done on OS X (Core i7 4770HQ @ 2.20 GHz with 16 GB 
RAM)


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 (updated)


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

Re: Review Request 52695: Harden libprocess

2016-11-02 Thread Aaron Wood 


> On Nov. 2, 2016, 9:32 a.m., Benjamin Bannier wrote:
> > I would really like to see actual timings of e.g., an optimized build 
> > before and after introducing these new flags, e.g., the runtime of 
> > `libprocess-tests` and `benchmarks`.

Just attached a bunch of benchmarking info that I had saved from when I posted 
it in the cxx Slack channel.


- Aaron


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


On Nov. 2, 2016, 3: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, 3: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
> 
>

Re: Review Request 52695: Harden libprocess

2016-11-02 Thread Benjamin Bannier 

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



I would really like to see actual timings of e.g., an optimized build before 
and after introducing these new flags, e.g., the runtime of `libprocess-tests` 
and `benchmarks`.


3rdparty/libprocess/Makefile.am (line 16)


Remove note about `-Werror` which is not used.



3rdparty/libprocess/Makefile.am (lines 16 - 24)


I think this would be easier to follow if you'd incrementially build up 
`AM_CXXFLAGS` while explaining their effect, e.g.,

# Enable common (and some language specific) warnings.
AM_CXXFLAGS += -Wall
# Warn when a comparison is made between signed and unsigned values.
AM_CXXFLAGS += -Wsign-compare
...



3rdparty/libprocess/Makefile.am (line 29)


Let's not suppress this valid and potentially useful diagnostic for the 
whole codebase. It does not trigger a hard failure anyway.



3rdparty/libprocess/Makefile.am (line 30)


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?



3rdparty/libprocess/configure.ac (line 272)


`s/!$/./`



3rdparty/libprocess/m4/ax_check_compile_flag.m4 (line 1)


For future updates it would be great if we'd write down the 
autoconf-archive release this file came from (it looks like the latest release 
containing it is `v2016.09.16`).


- Benjamin Bannier


On Nov. 1, 2016, 11:22 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52695/
> ---
> 
> (Updated Nov. 1, 2016, 11:22 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.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 52695: Harden libprocess

2016-11-01 Thread James Peach 

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


Ship it!




Ship It!

- James Peach


On Nov. 1, 2016, 7:12 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52695/
> ---
> 
> (Updated Nov. 1, 2016, 7:12 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 
> 
> 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.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 52695: Harden libprocess

2016-11-01 Thread Aaron Wood 

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

(Updated Nov. 1, 2016, 7:12 p.m.)


Review request for mesos, James Peach, Michael Park, and Neil Conway.


Changes
---

Only use `-fstack-protector-strong` if it's available to us. Remove `-Werror` 
and tackle this in another discussion/JIRA.


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 (updated)
-

  3rdparty/libprocess/Makefile.am 7131989 
  3rdparty/libprocess/configure.ac 1644035 

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.


Thanks,

Aaron Wood

Re: Review Request 52695: Harden libprocess

2016-10-14 Thread Aaron Wood 

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

(Updated Oct. 14, 2016, 3:20 p.m.)


Review request for mesos, 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 020b0e1 

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.


Thanks,

Aaron Wood

Re: Review Request 52695: Harden libprocess

2016-10-12 Thread Aaron Wood 

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




3rdparty/libprocess/Makefile.am (line 29)


Only use `-fstack-protector-strong` if we have GCC >= 4.9.


- Aaron Wood


On Oct. 11, 2016, 10:47 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52695/
> ---
> 
> (Updated Oct. 11, 2016, 10:47 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> 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 020b0e1 
> 
> 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.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 52695: Harden libprocess

2016-10-11 Thread Aaron Wood 

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

(Updated Oct. 11, 2016, 10:47 p.m.)


Review request for mesos and Michael Park.


Changes
---

Depend on the other RR for fixing new errors.


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 020b0e1 

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.


Thanks,

Aaron Wood

Re: Review Request 52695: Harden libprocess

2016-10-10 Thread Aaron Wood 

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

(Updated Oct. 10, 2016, 7:52 p.m.)


Review request for mesos and Michael Park.


Changes
---

Fix spelling and clarify comment about `-Wall`.


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 (updated)
-

  3rdparty/libprocess/Makefile.am 020b0e1 

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.


Thanks,

Aaron Wood

Re: Review Request 52695: Harden libprocess

2016-10-10 Thread Neil Conway 

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




3rdparty/libprocess/Makefile.am (line 16)


"Enable all warnings" is not a great description for "-Wall" (despite how 
it is named).



3rdparty/libprocess/Makefile.am (line 17)


"comparison"


- Neil Conway


On Oct. 10, 2016, 3:46 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52695/
> ---
> 
> (Updated Oct. 10, 2016, 3:46 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> 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 020b0e1 
> 
> 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.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Review Request 52695: Harden libprocess

2016-10-10 Thread Aaron Wood 

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

Review request for mesos and Michael Park.


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 020b0e1 

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.


Thanks,

Aaron Wood