Skip to content

Fix handling of rseq syscall #34

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

Merged
merged 4 commits into from
Apr 9, 2023
Merged

Conversation

otargowski
Copy link
Contributor

Duplicate of the mistakenly closed #32, sorry for the commotion.

The currently used libseccomp (2.3.3 from 2018) was made for Linux 4.15 and doesn't know about the rseq (334) syscall, which seems to always be caught on my system (kernel 6.1, glibc 2.37, gcc 12.2.1) when using default options, like sio2jail -b dir:/ exe. The issue doesn't occur when compiling with older gcc and glibc, like those from sio2's sandboxes, though for me it happens for every C++ program, even main(){}.
If built with the old libseccomp, an exception is thrown, as we try to convert NULL to string when we can't resolve 334 to "rseq". When we use a modern version, it is properly reported that we use a forbidden syscall.

So this pull request:

  • adds a proper, descriptive exception for when libseccomp can't resolve the name of a syscall
  • updates libseccomp in libseccomp.cmake
  • adds rseq to the list of syscalls allowed by the default seccomp policy
  • formats code with make clang-format.

@Wolf480pl
Copy link
Member

For context, here's some info about what rseq is: https://lwn.net/Articles/883104/

@Wolf480pl
Copy link
Member

Do you know of a way to convince glibc that rseq is not supported and should not be used?

I'm afraid this whole "retry critical section if preempted" thing would make instruction count more sensitive to environment, and if we could just disable it then we wouldn't have to worry about checking how it affects people's scores.

@Wolf480pl
Copy link
Member

Wolf480pl commented Apr 2, 2023

Hmm judging by the glibc patch, making rseq return ENOSYS in a similar way to this should work?

Can you test this, and if it works, submit a PR with that instead?

@otargowski
Copy link
Contributor Author

It does indeed work.

@Wolf480pl
Copy link
Member

Sounds good!

Can you squash the "disable rseq" commit into the "update seccomp and allow rseq" one, (or, preferably, have a separate commit for updating seccomp and separate for handling rseq)? That'd make it easier to read the diff.

This eases debugging when libseccomp doesn't know the name of a syscall.
The previously used version couldn't resolve the names of new
syscalls' numbers.
Executables built with a recent glibc will always try this syscall on
startup. ENOSYS will inform them that it is unavailable.
@Wolf480pl Wolf480pl merged commit 4f3d42b into sio2project:master Apr 9, 2023
@otargowski otargowski deleted the rseq-PR branch April 10, 2023 07:41
@Wolf480pl
Copy link
Member

Many thanks for the pull rqeuest!

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.

2 participants