mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilbert Song <songzihao1...@gmail.com>
Subject Re: Review Request 68018: Added `SeccompFilter` class.
Date Thu, 03 Jan 2019 01:58:25 GMT

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




src/linux/seccomp/seccomp.hpp
Lines 42-43 (patched)
<https://reviews.apache.org/r/68018/#comment296984>

    Add parameters?



src/linux/seccomp/seccomp.hpp
Lines 43 (patched)
<https://reviews.apache.org/r/68018/#comment296986>

    defaults to None()?



src/linux/seccomp/seccomp.cpp
Lines 138 (patched)
<https://reviews.apache.org/r/68018/#comment296997>

    Do I understand correctly that this would not impact on the setuid/setgid after pivot_root
in mesos/launch.cpp?
    
    The side effect is on the task: the task cannot setuid/setgid and cannot change capabilities?



src/linux/seccomp/seccomp.cpp
Lines 141-144 (patched)
<https://reviews.apache.org/r/68018/#comment297000>

    Instead of always set `SCMP_FLTATR_CTL_NNP`. Should we consider to check root privileges
first (e.g., `geteuid() != 0`)?



src/linux/seccomp/seccomp.cpp
Lines 147 (patched)
<https://reviews.apache.org/r/68018/#comment297010>

    Could we use `foreach (const ContainerSeccompProfile::Architecture& arch, profile.architectures())`?
    
    So that it avoids the implicit conversion to `int` and also avoid the `static_cast` below?



src/linux/seccomp/seccomp.cpp
Lines 183 (patched)
<https://reviews.apache.org/r/68018/#comment297011>

    capabilities->get(capabilities::BOUNDING)



src/linux/seccomp/seccomp.cpp
Lines 185 (patched)
<https://reviews.apache.org/r/68018/#comment297013>

    nits:
    
    To be explicit on `syscall.includes().capabilities_size() > 0` ?



src/linux/seccomp/seccomp.cpp
Lines 187 (patched)
<https://reviews.apache.org/r/68018/#comment297014>

    Ditto if we could leverage Enum, instead of `int`.


- Gilbert Song


On Nov. 8, 2018, 7:24 a.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68018/
> -----------------------------------------------------------
> 
> (Updated Nov. 8, 2018, 7:24 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
> 
> 
> Bugs: MESOS-9034
>     https://issues.apache.org/jira/browse/MESOS-9034
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `SeccompFilter` class is a wrapper for `libseccomp` API. Its main
> purpose is to provide a translation of the `ContainerSeccompProfile`
> message into calls of `libseccomp` API.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8 
>   src/Makefile.am 7a4904a3d67479267087fd2313a263d8218843fa 
>   src/linux/seccomp/seccomp.hpp PRE-CREATION 
>   src/linux/seccomp/seccomp.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68018/diff/11/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


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