Skip to content

Fix seccomp contributed profile for clone syscall #3314

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
estesp merged 1 commit into from
Jun 3, 2019
Merged

Fix seccomp contributed profile for clone syscall #3314

estesp merged 1 commit into from
Jun 3, 2019

Conversation

@KentaTada
Copy link
Contributor

@KentaTada KentaTada commented May 31, 2019 •
edited
Loading

All clone flags for namespace should be denied.
Also x/sys should be used instead of syscall.

Signed-off-by: Kenta Tada [email protected]

@KentaTada KentaTada force-pushed the fix-clone-seccomp-cgroupns branch from 32bfa60 to add4120 Compare May 31, 2019 02:35
@theopenlab-ci
Copy link

theopenlab-ci bot commented May 31, 2019

Build succeeded.

@KentaTada KentaTada force-pushed the fix-clone-seccomp-cgroupns branch from add4120 to 1442839 Compare May 31, 2019 03:01
@theopenlab-ci
Copy link

theopenlab-ci bot commented May 31, 2019

Build succeeded.

@codecov-io
Copy link

codecov-io commented May 31, 2019

Codecov Report

Merging #3314 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #3314   +/-   ##
======================================
  Coverage    44.6%   44.6%           
======================================
  Files         112     112           
  Lines       12180   12180           
======================================
  Hits         5433    5433           
  Misses       5913    5913           
  Partials      834     834
Flag Coverage Δ
#linux 48.49% <ø> (ø) ⬆️
#windows 39.87% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e7a3c9...1442839. Read the comment docs.

@codecov-io
Copy link

codecov-io commented May 31, 2019 •
edited
Loading

Codecov Report

Merging #3314 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #3314   +/-   ##
======================================
  Coverage    44.6%   44.6%           
======================================
  Files         112     112           
  Lines       12180   12180           
======================================
  Hits         5433    5433           
  Misses       5913    5913           
  Partials      834     834
Flag Coverage Δ
#linux 48.49% <ø> (ø) ⬆️
#windows 39.87% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1c5b384...5b9a43d. Read the comment docs.

@estesp
Copy link
Member

estesp commented May 31, 2019

This looks reasonable; however, given this is contributed content (contrib/) outside the core implementation can we change the PR title from "default profile" to "contributed profile" (in the commit message as well) so that anyone searching through issues/PRs in the future doesn't get the wrong idea that containerd uses a seccomp profile by default?

Thanks!

@KentaTada KentaTada force-pushed the fix-clone-seccomp-cgroupns branch from 1442839 to f811b4a Compare June 3, 2019 01:03
@KentaTada KentaTada changed the title Fix seccomp default profile for clone syscall Fix seccomp contributed profile for clone syscall Jun 3, 2019
@theopenlab-ci
Copy link

theopenlab-ci bot commented Jun 3, 2019

Build succeeded.

@KentaTada KentaTada force-pushed the fix-clone-seccomp-cgroupns branch from f811b4a to 30bdb45 Compare June 3, 2019 01:41
@theopenlab-ci
Copy link

theopenlab-ci bot commented Jun 3, 2019

Build succeeded.

@KentaTada KentaTada force-pushed the fix-clone-seccomp-cgroupns branch from 30bdb45 to c3b49b6 Compare June 3, 2019 02:13
@theopenlab-ci
Copy link

theopenlab-ci bot commented Jun 3, 2019

Build succeeded.

@KentaTada KentaTada force-pushed the fix-clone-seccomp-cgroupns branch from c3b49b6 to 767d5f4 Compare June 3, 2019 05:04
@theopenlab-ci
Copy link

theopenlab-ci bot commented Jun 3, 2019

Build succeeded.

Fix seccomp contributed profile for clone syscall
5b9a43d 
All clone flags for namespace should be denied.
Also x/sys should be used instead of syscall.

Signed-off-by: Kenta Tada <[email protected]>
@KentaTada KentaTada force-pushed the fix-clone-seccomp-cgroupns branch from 767d5f4 to 5b9a43d Compare June 3, 2019 05:31
@theopenlab-ci
Copy link

theopenlab-ci bot commented Jun 3, 2019

Build succeeded.

@KentaTada
Copy link
Contributor Author

KentaTada commented Jun 3, 2019

@estesp Thank you for the review. I changed the title and comment. Could you confirm it?

@crosbymichael
Copy link
Member

crosbymichael commented Jun 3, 2019

LGTM

estesp
estesp approved these changes Jun 3, 2019
View reviewed changes
Copy link
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah
Copy link
Member

thaJeztah commented Jun 3, 2019

@KentaTada could you check if changes are needed to the default seccomp profile in moby/moby? https://github.com/moby/moby/blob/61da822eeb7905a61eeaf18ea386e1a786cf0601/profiles/seccomp/seccomp_default.go#L517-L538

@thaJeztah
Copy link
Member

thaJeztah commented Jun 3, 2019

@justincormack ptal

@estesp estesp merged commit 48a1fca into containerd:master Jun 3, 2019
@justincormack
Copy link
Contributor

justincormack commented Jun 3, 2019

@thaJeztah yes the changes should be the same on moby/moby

@thaJeztah
Copy link
Member

thaJeztah commented Jun 3, 2019

Thx, yes, found the difference after posting here; let me do a PR for moby/moby

@thaJeztah thaJeztah mentioned this pull request Jun 3, 2019
Merged
@thaJeztah
Copy link
Member

thaJeztah commented Jun 3, 2019

opened moby/moby#39308

@KentaTada KentaTada mentioned this pull request Jun 4, 2019
Merged
KentaTada pushed a commit to KentaTada/runc that referenced this pull request Jun 4, 2019
libcontainer: change seccomp test for clone syscall
89d0205 
This commit changes the value of seccomp test for clone syscall.
Also hardcoded values should be changed because it is unclear to
understand what flags are tested.

Related issues:

* containerd/containerd#3314
* moby/moby#39308
* opencontainers/runtime-tools#694

Signed-off-by: Kenta Tada <[email protected]>
@KentaTada KentaTada mentioned this pull request Jun 4, 2019
Merged
KentaTada pushed a commit to KentaTada/runc that referenced this pull request Jun 4, 2019
libcontainer: change seccomp test for clone syscall
b54fd85 
This commit changes the value of seccomp test for clone syscall.
Also hardcoded values should be changed because it is unclear to
understand what flags are tested.

Related issues:

* containerd/containerd#3314
* moby/moby#39308
* opencontainers/runtime-tools#694

Signed-off-by: Kenta Tada <[email protected]>
mauriciovasquezbernal pushed a commit to kinvolk/runc that referenced this pull request Jul 23, 2019
@mauriciovasquezbernal
libcontainer: change seccomp test for clone syscall
07802a7 
This commit changes the value of seccomp test for clone syscall.
Also hardcoded values should be changed because it is unclear to
understand what flags are tested.

Related issues:

* containerd/containerd#3314
* moby/moby#39308
* opencontainers/runtime-tools#694

Signed-off-by: Kenta Tada <[email protected]>
adrianreber pushed a commit to adrianreber/runc that referenced this pull request Aug 6, 2019
@adrianreber
libcontainer: change seccomp test for clone syscall
8b9139a 
This commit changes the value of seccomp test for clone syscall.
Also hardcoded values should be changed because it is unclear to
understand what flags are tested.

Related issues:

* containerd/containerd#3314
* moby/moby#39308
* opencontainers/runtime-tools#694

Signed-off-by: Kenta Tada <[email protected]>
stefanberger pushed a commit to stefanberger/runc that referenced this pull request Dec 31, 2019
@stefanberger
libcontainer: change seccomp test for clone syscall
a33a2a0 
This commit changes the value of seccomp test for clone syscall.
Also hardcoded values should be changed because it is unclear to
understand what flags are tested.

Related issues:

* containerd/containerd#3314
* moby/moby#39308
* opencontainers/runtime-tools#694

Signed-off-by: Kenta Tada <[email protected]>
@thaJeztah thaJeztah mentioned this pull request Aug 24, 2020
Closed
dims pushed a commit to dims/libcontainer that referenced this pull request Oct 19, 2024
libcontainer: change seccomp test for clone syscall
92b04af 
This commit changes the value of seccomp test for clone syscall.
Also hardcoded values should be changed because it is unclear to
understand what flags are tested.

Related issues:

* containerd/containerd#3314
* moby/moby#39308
* opencontainers/runtime-tools#694

Signed-off-by: Kenta Tada <[email protected]>
dims pushed a commit to dims/libcontainer that referenced this pull request Oct 19, 2024
libcontainer: change seccomp test for clone syscall
4921d82 
This commit changes the value of seccomp test for clone syscall.
Also hardcoded values should be changed because it is unclear to
understand what flags are tested.

Related issues:

* containerd/containerd#3314
* moby/moby#39308
* opencontainers/runtime-tools#694

Signed-off-by: Kenta Tada <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@estesp estesp estesp approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@KentaTada @codecov-io @estesp @crosbymichael @thaJeztah @justincormack