PRoot and seccomp

[alkino]

Hello,
First of all this is not an issue but a contact. I'm a maintainer of PRoot.
I look around what you do here, and you seems to have a nice patch to fix seccomp. Mind you upstream your work?
Regards,
alkino

[michalbednarski]

You mean Linux >=4.8 support? (47138da and follow-up commit as that one wasn't complete fix)

I've also got workarounds for Android Oreo seccomp policy (which blocks syscalls such as open(2) and requires use of openat(2); as well as raises SIGSYS on PR_void)? Also, some other workarounds for kernel bugs/missing features. (Not sure if old devices/kernels are target for upstream version)

Nice to see upstream proot is alive back. If it's going to stay alive I might attach git commit history from upstream.

(Aside: personally I don't really like tri-state tracee->seccomp and already added additional booleans to tracee which probably could remove need for DISABLED/DISABLING separation but that would need additional implementation/testing and I didn't want to needlessly deviate from upstream.)

[oxr463]

Just following up on this; @alkino, did you ever find an appropriate fix for this upstream?

[alkino]

No, it takes too much time. I don't know well seccomp / ptrace. So it's a really hard to figure it out.

[oxr463]

@michalbednarski, would you care to take a look at the latest CI logs for upstream PRoot?

• https://travis-ci.org/proot-me/proot/builds/532081135
• https://gitlab.com/proot/proot/pipelines/61135927

I am working on merging a PR for seccomp, but it doesn't look to be effective.

With travis upgrading its base distro to xenial, PRoot can no longer run properly even in CI.

--

I've forked this repo on GitLab, to try to reproduce using the latest CI/CD.

[michalbednarski]

I've tried fix-seccomp branch form proot-me/proot repo (for which CI builds you've linked, not the linked pull request) and so far I've found one thing that needs to be fixed:

diff --git a/src/tracee/event.c b/src/tracee/event.c
index 809f710..1901bea 100644
--- a/src/tracee/event.c
+++ b/src/tracee/event.c
@@ -501,7 +501,7 @@ int handle_tracee_event_kernel_4_8(Tracee *tracee, int tracee_status)
                                /* SECCOMP TRAP can only be received for
                                 * sysenter events, ignore otherwise */
                                if (!IS_IN_SYSENTER(tracee)) {
-                                       tracee->restart_how = PTRACE_CONT;
+                                       tracee->restart_how = PTRACE_SYSCALL;
                                        return 0;
                                        }
                                 status = ptrace(PTRACE_GETEVENTMSG, tracee->pid, NULL, &flags);

There definitely should be PTRACE_SYSCALL (not PTRACE_CONT) because if we've reached that place with tracee->status == 1 it means proot expects to receive sysexit through SIGTRAP.

Note however, that while this fixes some of tests, it makes other tests hang.

This is what I've found so far, I'll keep looking into it and I think I'll post another update tomorrow.

[oxr463]

Interesting.. Thank you for your insight in this matter.

I've been working on the build system and the test suite specifically for this reason. I hope to add more checks for seccomp and different kernel versions to make this easier to debug in the future.

--

The termux fork has a few skipped tests, and only one failed tests, but it didn't hang on the latest build, (See: https://gitlab.com/proot/termux-proot/-/jobs/214954620).

[michalbednarski]

I've found another issue in proot-me/proot/fix-seccomp: The handle_tracee_event function is called in multiple places, however only one of them has check for kernel version and call handle_tracee_event_kernel_4_8, code in ptrace.c always call handle_tracee_event which expects Linux < 4.8. I think you should rename old handle_tracee_event and make handle_tracee_event wrapper that calls appropriate function based on kernel version.

After those fixes only remaining failing test (On my development machine) is test-kkkkkkkk which appears to be faulty as it fails even when running without proot.

Some tests also fail on my development machine when python is python3, but that doesn't appear to be case on CI.

[michalbednarski]

One more thing I've forgot to write: test which nest proot (proot proot -0 id) fail because proot doesn't pass seccomp events to tracees and you new implementation considers seccomp to be supported if it PTRACE_SETOPTIONS succeeds (version for older kernels consider seccomp supported if they've got seccomp event, although easiest fix would be to check presence of seccomp option in PTRACE_SETOPTIONS and reject that call if seccomp events are requested)

[oxr463]

I made a list of commits to pull in but I wasn't sure how much of it was android-specific.