Skip to content
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.

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

seccomp: add support for seccomp notify #190

Merged
merged 2 commits into from Jun 2, 2021

Conversation

giuseppe
Copy link
Member

@giuseppe giuseppe commented Jul 27, 2020

add support for seccomp notify and add a basic support for emulating
mknod and mknodat. The handler implementation is likely going to
change, for now it is just a PoC to show how it would work.

Requires: containers/crun#438
Requires: libseccomp-2.5

Signed-off-by: Giuseppe Scrivano gscrivan@redhat.com

@lgtm-com
Copy link

lgtm-com bot commented Jul 27, 2020

This pull request introduces 2 alerts when merging 0ed1348 into 3c396d4 - view on LGTM.com

new alerts:

  • 2 for Local variable hides global variable

src/ctrl.c Outdated Show resolved Hide resolved
@giuseppe giuseppe force-pushed the seccomp-notify branch 10 times, most recently from 8b9239c to 14cfa57 Compare Aug 5, 2020
@giuseppe giuseppe marked this pull request as ready for review Aug 25, 2020
@giuseppe giuseppe force-pushed the seccomp-notify branch 2 times, most recently from 389ea32 to 2b8da9d Compare Aug 25, 2020
@giuseppe
Copy link
Member Author

giuseppe commented Aug 27, 2020

@haircommander LGTY?

src/conn_sock.c Outdated Show resolved Hide resolved
src/ctrl.c Outdated Show resolved Hide resolved
src/seccomp_notify.c Outdated Show resolved Hide resolved
@haircommander
Copy link
Collaborator

haircommander commented Aug 27, 2020

sorry, a couple of nits. I prefer it where a callee is defined below the caller. that allows the file to be read top down.

@TomSweeneyRedHat
Copy link
Contributor

TomSweeneyRedHat commented Aug 27, 2020

Other than @haircommander 's comments
LGTM

@saschagrunert
Copy link
Member

saschagrunert commented Aug 27, 2020

@giuseppe please rebase to get the static build fixed.

Makefile Outdated Show resolved Hide resolved
@giuseppe giuseppe force-pushed the seccomp-notify branch 2 times, most recently from 47d6aa7 to 8dc536c Compare Sep 28, 2020
@giuseppe giuseppe force-pushed the seccomp-notify branch 2 times, most recently from e207e16 to e577f9f Compare Apr 26, 2021
@giuseppe
Copy link
Member Author

giuseppe commented Apr 26, 2021

adapted to follow the OCI runtime specs, needs: https://github.com/giuseppe/libpod/tree/seccomp-notify

@giuseppe
Copy link
Member Author

giuseppe commented Apr 26, 2021

also needs: containers/crun#652

@giuseppe giuseppe force-pushed the seccomp-notify branch 4 times, most recently from 9e8f5c2 to d26cdcb Compare Apr 27, 2021
@giuseppe giuseppe marked this pull request as ready for review Apr 29, 2021
@giuseppe
Copy link
Member Author

giuseppe commented Apr 29, 2021

@haircommander @rhatdan adapted to follow the OCI specs

Makefile Outdated Show resolved Hide resolved
src/conmon.c Show resolved Hide resolved
@rhatdan
Copy link
Member

rhatdan commented May 6, 2021

@giuseppe needs a rebase.

@giuseppe
Copy link
Member Author

giuseppe commented May 17, 2021

I am not sure the CI failures are related to this PR

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
add support for seccomp notify and add a basic support for emulating
mknod and mknodat.  The handler implementation is likely going to
change, for now it is just a PoC to show how it would work.

Requires: containers/crun#438
Requires: libseccomp-2.5

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@giuseppe
Copy link
Member Author

giuseppe commented Jun 1, 2021

kubernetes e2e is green as well

@rhatdan
Copy link
Member

rhatdan commented Jun 1, 2021

LGTM
@haircommander PTAL

@haircommander
Copy link
Collaborator

haircommander commented Jun 2, 2021

I would like to get #267 in and then we can merge this

@rhatdan rhatdan merged commit 75e067e into containers:master Jun 2, 2021
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants