Skip to content

seccomp: add support for riscv64#15176

Closed
mbiebl wants to merge 1 commit intosystemd:masterfrom
mbiebl:seccomp-riscv64
Closed

seccomp: add support for riscv64#15176
mbiebl wants to merge 1 commit intosystemd:masterfrom
mbiebl:seccomp-riscv64

Conversation

@mbiebl
Copy link
Contributor

@mbiebl mbiebl commented Mar 20, 2020

This patch adds seccomp support to the riscv64 architecture. seccomp
support is available in the riscv64 kernel since version 5.5, and it
has just been added to the libseccomp library.

riscv64 uses generic syscalls like aarch64, so I used that architecture
as a reference to find which code has to be modified.

With this patch, the testsuite passes successfully, including the
test-seccomp test. The system boots and works fine with kernel 5.4 (i.e.
without seccomp support) and kernel 5.5 (i.e. with seccomp support). I
have also verified that the "SystemCallFilter=~socket" option prevents a
service to use the ping utility when running on kernel 5.5.

Patch courtesy of Aurelien Jarno [email protected]

@aurel32 @mbiebl
seccomp: add support for riscv64
2c1c875 
This patch adds seccomp support to the riscv64 architecture. seccomp
support is available in the riscv64 kernel since version 5.5, and it
has just been added to the libseccomp library.

riscv64 uses generic syscalls like aarch64, so I used that architecture
as a reference to find which code has to be modified.

With this patch, the testsuite passes successfully, including the
test-seccomp test. The system boots and works fine with kernel 5.4 (i.e.
without seccomp support) and kernel 5.5 (i.e. with seccomp support). I
have also verified that the "SystemCallFilter=~socket" option prevents a
service to use the ping utility when running on kernel 5.5.
@mbiebl
Copy link
Contributor Author

mbiebl commented Mar 20, 2020

seccomp/libseccomp@5b03588

@keszybz
Copy link
Member

keszybz commented Mar 24, 2020

semaphoreci:

../src/basic/macro.h:494:73: note: in definition of macro ‘IN_SET’
                 static const long double __assert_in_set[] _unused_ = { __VA_ARGS__ }; \
                                                                         ^~~~~~~~~~~
../src/shared/seccomp-util.c: In function ‘seccomp_restrict_address_families’:
../src/shared/seccomp-util.c:1346:22: error: ‘SCMP_ARCH_RISCV64’ undeclared (first use in this function); did you mean ‘SCMP_ARCH_PARISC64’?
                 case SCMP_ARCH_RISCV64:
                      ^~~~~~~~~~~~~~~~~
                      SCMP_ARCH_PARISC64
../src/shared/seccomp-util.c: In function ‘seccomp_memory_deny_write_execute’:
../src/shared/seccomp-util.c:1630:22: error: ‘SCMP_ARCH_RISCV64’ undeclared (first use in this function); did you mean ‘SCMP_ARCH_PARISC64’?
                 case SCMP_ARCH_RISCV64:
                      ^~~~~~~~~~~~~~~~~
                      SCMP_ARCH_PARISC64

azure and centos ci fail similarly.

@keszybz keszybz added the ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR label Mar 24, 2020
@mbiebl
Copy link
Contributor Author

mbiebl commented Mar 24, 2020

This needs the above commit in libseccomp. Afaics there is no official release yet of libseccomp with that commit though (in Debian it was cherry-picked).
Once that happens, I assume the min version in meson.build would have to be bumped as well.

@keszybz
Copy link
Member

keszybz commented Mar 24, 2020

If this is too be merged anytime soon, it'll have to be amended to not break with older libseccomp and kernels.

@mbiebl
Copy link
Contributor Author

mbiebl commented Mar 24, 2020

@aurel32 ^

@mbiebl
Copy link
Contributor Author

mbiebl commented Apr 10, 2020

Let's postpone this until we have a libseccomp upstream release support riscv64. I hope it's ok to leave this PR open.

@poettering
Copy link
Member

poettering commented May 14, 2020

where are we with this?

SNiTEBoBy
SNiTEBoBy suggested changes Jun 10, 2020
View reviewed changes
Copy link

@SNiTEBoBy SNiTEBoBy left a comment

Choose a reason for hiding this comment

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

Pull requested

@poettering
Copy link
Member

poettering commented Aug 18, 2020

so an uptsream libseccomp version has long been released. Either way this patch requires some ifdeffery so that we don't use the definition if it isn't there. We have similar ifdeffery for PARISC, it's not difficult to add to RISCV too.

Dropping the "postponed" flag, since it's not really about that anymore. The patch just needs some ifdeffery now.

@poettering poettering added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed postponed labels Aug 18, 2020
@poettering
Copy link
Member

poettering commented Aug 18, 2020

/cc @aurel32

@aurel32
Copy link
Contributor

aurel32 commented Aug 19, 2020 •
edited
Loading

It appears that support for PARISC is limited to src/nspawn/nspawn-oci.c. I guess Ie should use the same #ifdef strategy in the other parts of the code.

@aurel32
Copy link
Contributor

aurel32 commented Aug 19, 2020

I have just done that there: aurel32@f81ee95

As I am not the creator of this PR, not sure how to continue further. Should I just create a new one?

@keszybz
Copy link
Member

keszybz commented Aug 20, 2020

Yes, please create a new PR.

@aurel32 aurel32 mentioned this pull request Aug 20, 2020
Merged
@aurel32
Copy link
Contributor

aurel32 commented Aug 20, 2020

Yes, please create a new PR.

Done, it's PR #16807. Please close this one.

@keszybz keszybz closed this Aug 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@SNiTEBoBy SNiTEBoBy SNiTEBoBy requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR replaced-by-newer-pr reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks seccomp

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@mbiebl @keszybz @poettering @aurel32 @SNiTEBoBy @anitazha