mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrei Budnik <abud...@mesosphere.com>
Subject Re: Review Request 68019: Added a parser for the Docker Seccomp config format.
Date Sat, 19 Jan 2019 11:02:05 GMT


> On Jan. 19, 2019, 1:05 a.m., Gilbert Song wrote:
> > src/linux/seccomp/seccomp_parser.cpp
> > Lines 164-167 (patched)
> > <https://reviews.apache.org/r/68019/diff/14/?file=2119630#file2119630line164>
> >
> >     does the order of the `architecture/subarchitecture` matter? asking cause I
am not sure if we need architecture before its corresponding subarchitecture in the repeated
field

> does the order of the architecture/subarchitecture matter?

No, it doesn't.


> On Jan. 19, 2019, 1:05 a.m., Gilbert Song wrote:
> > src/linux/seccomp/seccomp_parser.cpp
> > Lines 490-494 (patched)
> > <https://reviews.apache.org/r/68019/diff/14/?file=2119630#file2119630line490>
> >
> >     Since we somehow have to create the profile in launch.cpp, could we avoid create
the `ctx` object twice?
> >     
> >     Instead, for the default profile, I think we could add this verification in
isolator::create method after we parse it

I'm totally against moving verification step outside of this function. Such a tiny optimization
does not worth it.

The benefit of verifying Seccomp profile in-place is that:
1) We do not duplicate code for calling verification for callers of this function. We need
to always verify the Seccomp profile after parsing it.
2) If `libseccomp` returns an error (e.g., when it does not recognize a syscall), this function
returns the error, so it will be written in agent logs. That is very helpful for debugging.
Otherwise, an error will be detected in `launch.cpp` - it won't be printed into logs.


- Andrei


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


On Nov. 8, 2018, 3:24 p.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68019/
> -----------------------------------------------------------
> 
> (Updated Nov. 8, 2018, 3:24 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
> 
> 
> Bugs: MESOS-9105
>     https://issues.apache.org/jira/browse/MESOS-9105
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Docker Seccomp config is a JSON file containing Seccomp filtering
> rules. This patch introduces a parser for Docker Seccomp config format.
> This parser accepts a JSON-string, parses and validates it, then
> returns a prepared `ContainerSeccompProfile` message.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt a574d449dc26b820cbef7ff0b5e94b42b6fe86cf 
>   src/Makefile.am cd785255fcdf1302a8f9fa358039e5d1f200e132 
>   src/linux/seccomp/seccomp_parser.hpp PRE-CREATION 
>   src/linux/seccomp/seccomp_parser.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68019/diff/14/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


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