-
Notifications
You must be signed in to change notification settings - Fork 880
seccomp: add arch-specific syscalls on ARM #3636
Conversation
Can one of the admins verify this patch? |
I can confirm that this works on ARMv7. Unfortunately I don't have an AArch64 device to test on, nor an ARMv6 device. |
Does it make sense to add these additional syscalls via build tags instead of an |
I think this would be more performant than the current implementation but it would increase maintenance complexity. At least we could use raw runtime.GOARCH since the additional syscalls are needed for all arm flavors. Then the performance impact would be minimal |
ok to test |
semaphore disk is full btw. |
// RktDefaultSeccompBlacklist contains a default blacklist of syscalls, | ||
// used by rkt for seccomp filtering. | ||
RktDefaultSeccompBlacklist = DockerDefaultSeccompBlacklist | ||
// RktDefaultSeccompWhitelist contains a default whitelist of syscalls, | ||
// used by rkt for seccomp filtering. | ||
RktDefaultSeccompWhitelist = DockerDefaultSeccompWhitelist | ||
) | ||
|
||
func init() { | ||
if runtime.GOARCH == "arm" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see we are also doing 64bits whitelisting here, so this should probably also check for the arm64
case.
Yep, we are unfortunately hitting semaphore disk limits. I've opened #3670 in the meanwhile, which should hopefully bring us back to a reasonable amount. I'd like to see one last change in here, otherwise this PR seems fine. |
I'm currently not able to add the line, but I'll do it tomorrow before
lunch CET.
Luca Bruno <notifications@github.com> schrieb am Mo., 8. Mai 2017, 13:13:
… Yep, we are unfortunately hitting semaphore disk limits. I've opened #3670
<#3670> in the meanwhile, which should
hopefully bring us back to a reasonable amount. I'd like to see one last
change in here, otherwise this PR seems fine.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#3636 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AC3LqU74BhY65OlDUzDxQOqZpS5ZZ45Lks5r3vjogaJpZM4M4tjg>
.
|
@trusch I fear something went wrong while rebasing this PR, it now contains quite a number of external commits. |
9b6aeb7
to
8c9b34c
Compare
@lucab sorry, I hope it is fixed now. How should this be handles in general when a PR needs some days? Merging the master into the PR or rebasing on it? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@trusch rebasing your PR on top of master is typically preferred to minimize the number of merge commits and to avoid having conflicts resolved in intermediate merge commits. |
This enables us to run binaries which use these syscalls when systemd has seccomp enforcing enabled. Like reported this should affect at least all gcc compiled binaries. This solves issue rkt#3629.
8c9b34c
to
5f49ab0
Compare
This enables us to run binaries which use these syscalls when systemd has seccomp enforcing enabled.
Like reported this should affect at least all gcc compiled binaries.
This PR needs more testing, I just want to see what CI says. I'll setup at least one testbed with seccomp-enabled systemd. Perhaps @Xenopathic could test on aarch64 devices, I could test for armv6l and armv7l devices.
Fixes #3629.