Enable seccomp for s390x and ppc: s390x part #23172
Conversation
|
Looks like build is failing currently; |
thaJeztah
added
the
status/failing-ci
@thaJeztah this is expected because docker/engine-api#248 [4] from issue #23171 is missing in Docker. Once we have all prerequisites we have to bump all the vendor versions. |
|
Should be ok once the requisite parts are merged and tests pass, plus we resolve the extra syscalls needed for s390x discussed in #23171 |
|
@michael-holzheu You need to run |
|
@michael-holzheu do you want to add |
IMHO we can do this later with another pull request (when we are sure about all syscalls). |
|
@michael-holzheu as we are clear now, maybe add here now? |
justincormack
removed
the
status/failing-ci
Ok also ok for me. I will update the PR. |
|
@michael-holzheu can you squash your commits? |
| case "s390", "s390x": | ||
| syscalls = append(syscalls, []*types.Syscall{ | ||
| { | ||
| Name: "s390_runtime_instr", |
There was a problem hiding this comment.
minor nit I was trying to keep the syscalls in alphabetical order everywhere in this file (not 100% sure they all are but I just noticed here...)
There was a problem hiding this comment.
minor nit I was trying to keep the syscalls in alphabetical order everywhere in this file
Ok so I will move s390_runtime_instr to the end of the three syscalls.
I can do this. So in general you want one commit per PR? |
|
@michael-holzheu we don't have a strict rule it depends on the PR, but in this case they are implementing a single feature and are all small so one commit is cleaner and easier for people to follow later when they are looking back at the history. |
To implement seccomp for s390x the following changes are required:
1) seccomp_default: Add s390 compat mode
On s390x (64 bit) we can run s390 (32 bit) programs in 32 bit
compat mode. Therefore add this information to arches().
2) seccomp_default: Use correct flags parameter for sys_clone on s390x
On s390x the second parameter for the clone system call is the flags
parameter. On all other architectures it is the first one.
See kernel code kernel/fork.c:
#elif defined(CONFIG_CLONE_BACKWARDS2)
SYSCALL_DEFINE5(clone, unsigned long, newsp, unsigned long, clone_flags,
int __user *, parent_tidptr,
So fix the docker default seccomp rule and check for the second
parameter on s390/s390x.
3) seccomp_default: Add s390 specific syscalls
For s390 we currently have three additional system calls that should
be added to the seccomp whitelist:
- Other architectures can read/write unprivileged from/to PCI MMIO memory.
On s390 the instructions are privileged and therefore we need system
calls for that purpose:
* s390_pci_mmio_write()
* s390_pci_mmio_read()
- Runtime instrumentation:
* s390_runtime_instr()
4) test_integration: Do not run seccomp default profile test on s390x
The generated profile that we check in is for amd64 and i386
architectures and does not work correctly on s390x.
See also: 75385dc ("Do not run the seccomp tests that use
default.json on non x86 architectures")
5) Dockerfile.s390x: Add "seccomp" to DOCKER_BUILDTAGS
Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
|
@michael-holzheu remind me what we need to fix to get the CI for Z not to hang? It would be nice to fix it. |
We need updated glibc packages in the s390x/gcc:6.1 image. The fix is accepted in Debian, but they have not released a new package up to now. As a workaround I manually built packages and suggested @tianon (who is responsible for the images) to integrate them. But unfortunately I did not get an answer up to now :-( |
|
@michael-holzheu maybe you can create an issue for that so it doesnt get lost... |
|
LGTM I think we now are just waiting for opencontainers/runc#866 |
|
LGTM, thanks @michael-holzheu! |
|
Windows doesn't use this, and failure is not related, so merging |
Yes. And after this is done we have to bump runc in Docker and use the correct RUNC_COMMIT in the Dockerfiles. |
|
Yes the runc bump will happen anyway as we are waiting for 1.0.
|
|
Regarding our s390x CI problem:
I now opened issue #24748 to track this. |
Patches required to enable seccomp support for s390x:
Please do not merge until all prerequisites of issue #23171 are fulfilled.