You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <be...@mesosphere.io> on 2017/07/04 15:06:43 UTC

Re: Review Request 60546: Harden Mesos when building with cmake.

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




cmake/CompilationConfigure.cmake
Lines 207-210 (patched)
<https://reviews.apache.org/r/60546/#comment254394>

    Let's not tie support to arbitrary versions (e.g., this condition rejects clang-4.0 just like gcc-4.3 which doesn't seem like the intention).
    
    Instead let's use `CHECK_CXX_COMPILER_FLAG` to check compiler support for the flags you are passing, and potentially output a warning if we cannot enable any stack protection.



cmake/CompilationConfigure.cmake
Lines 213-214 (patched)
<https://reviews.apache.org/r/60546/#comment254395>

    Not sure this is needed, see below.



cmake/MesosConfigure.cmake
Lines 65 (patched)
<https://reviews.apache.org/r/60546/#comment254396>

    It appears as if just setting
    
        set(CMAKE_POSITION_INDEPENDENT_CODE TRUE)
        
    here as well would take care of both the compiler and linker flags; could you check if it does and potentially streamline this part. It would also be great if we could have a comment explaining why we enforce position independent code.


- Benjamin Bannier


On June 29, 2017, 8:18 p.m., Aaron Wood wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60546/
> -----------------------------------------------------------
> 
> (Updated June 29, 2017, 8:18 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, and Joseph Wu.
> 
> 
> Bugs: MESOS-7737
>     https://issues.apache.org/jira/browse/MESOS-7737
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Apply stack protectors (stronger stack protectors with newer compilers), position independent code suitable for linking into executables, security warnings, and generate position independent executables.
> 
> 
> Diffs
> -----
> 
>   cmake/CompilationConfigure.cmake 3fa2e2f3b 
>   cmake/MesosConfigure.cmake 5ecad2c0f 
> 
> 
> Diff: https://reviews.apache.org/r/60546/diff/1/
> 
> 
> Testing
> -------
> 
> `mkdir build && cd build && cmake .. && make -j$(nproc) && make check -j$(nproc)`
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>


Re: Review Request 60546: Harden Mesos when building with cmake.

Posted by Aaron Wood via Review Board <no...@reviews.apache.org>.

> On July 4, 2017, 3:06 p.m., Benjamin Bannier wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 213-214 (patched)
> > <https://reviews.apache.org/r/60546/diff/1/?file=1767158#file1767158line213>
> >
> >     Not sure this is needed, see below.

Relating to my comment below, I saw that `-pie` was not used to generate a fully position independent executable. Only `-fPIC` was used to generate position independent code for dynamic linking.


> On July 4, 2017, 3:06 p.m., Benjamin Bannier wrote:
> > cmake/MesosConfigure.cmake
> > Lines 65 (patched)
> > <https://reviews.apache.org/r/60546/diff/1/?file=1767159#file1767159line65>
> >
> >     It appears as if just setting
> >     
> >         set(CMAKE_POSITION_INDEPENDENT_CODE TRUE)
> >         
> >     here as well would take care of both the compiler and linker flags; could you check if it does and potentially streamline this part. It would also be great if we could have a comment explaining why we enforce position independent code.

From my testing I found that `-fPIC` was always set when `set(CMAKE_POSITION_INDEPENDENT_CODE TRUE)` is used but `-fPIE` is never set. I figured I'd add it here since it's very relevant when building static libs. I'll definitely add more comments surrounding this stuff.

Also, I'm assuming that the static libs built here are only for linking into executables. Do you think this will hold true going forward?


- Aaron


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


On June 29, 2017, 6:18 p.m., Aaron Wood wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60546/
> -----------------------------------------------------------
> 
> (Updated June 29, 2017, 6:18 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, and Joseph Wu.
> 
> 
> Bugs: MESOS-7737
>     https://issues.apache.org/jira/browse/MESOS-7737
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Apply stack protectors (stronger stack protectors with newer compilers), position independent code suitable for linking into executables, security warnings, and generate position independent executables.
> 
> 
> Diffs
> -----
> 
>   cmake/CompilationConfigure.cmake 3fa2e2f3b 
>   cmake/MesosConfigure.cmake 5ecad2c0f 
> 
> 
> Diff: https://reviews.apache.org/r/60546/diff/1/
> 
> 
> Testing
> -------
> 
> `mkdir build && cd build && cmake .. && make -j$(nproc) && make check -j$(nproc)`
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>