Skip to content

Sandbox escape by racing seccomp notifications? #27

Open

Description

@Changaco

Sandlock is partly based on seccomp notifications and appears to be implying that they can be safely used for something the seccomp_unotify(2) man page warns against, i.e. to make a security decision to allow a syscall to proceed, which is unsafe due to TOCTOU race conditions. I think this limitation needs to be mentioned at least in the README sections where policy_fn is documented. Additionally, it might be a good idea for every function which returns NotifAction::Continue to explain why it's safe to do so.

Activity

  1. congwang-mk commented on May 1, 2026

    @congwang-mk
    Contributor

    Thanks for raising this. Both of your asks landed:

    PR #28 — audited every NotifAction::Continue site, added module-level safety contracts explaining why each is safe. The audit also surfaced and fixed one bug: sendmsg_on_behalf was falling through to Continue on msghdr-read failure (a racing thread could have skipped the IP allowlist); it now returns EFAULT.

    PR #29 — redesigned the policy_fn event surface around "every held syscall gets the strongest safety its kernel structure allows":

    • Path strings (event.path, path_contains) dropped from events. Path-based access control belongs in static Landlock rules (fs_read / fs_write / fs_deny); ctx.deny_path() stays for runtime additions.
    • event.argv for execve kept and made TOCTOU-safe. Since execve can't be done on-behalf (the kernel must run it in the calling thread), the supervisor now PTRACE_SEIZE + PTRACE_INTERRUPTs sibling threads of the calling tid before sending Continue. The pause is effectively zero-cost — execve's de_thread kills siblings anyway. If the freeze fails (YAMA, etc.), the execve is denied closed.

    The README's Dynamic Policy section now has an explicit TOCTOU note. Closing as resolved.

  2. Changaco commented on May 1, 2026

    @Changaco
    Author

    I'm not sure freezing the sibling threads is sufficient to prevent all races. Couldn't a different process in the same sandbox modify the arguments if they're stored in shared memory?

  3. congwang-mk commented on May 1, 2026

    @congwang-mk
    Contributor

    @Changaco — you were right, the sibling-thread freeze closed only the same-TGID race; a peer process aliasing argv pages via MAP_SHARED (memfd, SysV shm, shared file mmap) or sharing mm_struct via clone(CLONE_VM) could still mutate argv between the supervisor's read and the kernel's post-Continue re-read.

    Fix landed in #33 (merged as 84971c6). The freeze now enumerates every TGID in ProcessIndex (the canonical sandbox-membership set) and PTRACE_SEIZE+PTRACE_INTERRUPTs every TID via /proc/<tgid>/task, not just the caller's siblings. The supervisor's sequential notification dispatch keeps the snapshot stable — no new clone/fork notifications can complete during the freeze, so we don't need any new locking on top.

    Proof sketch for argv TOCTOU-safety on execve, post-fix:

    • Every TGID in the sandbox is in ProcessIndex (registered by the clone/fork notification handler).
    • The freeze walks every such TGID and every thread within it.
    • Threads in user space or interruptible kernel sleep are confirmed in ptrace-stop via blocking waitpid.
    • Threads in TASK_UNINTERRUPTIBLE (vfork parents waiting on the execve caller, etc.) cannot run user code regardless — the kernel is already holding them.
    • ⇒ no entity in the sandbox can mutate argv between the supervisor's inspection and the kernel's re-read.

    One non-obvious wrinkle worth flagging for anyone reading the diff: skipping TASK_UNINTERRUPTIBLE peers isn't an optimization — it's load-bearing for correctness. Naively waitpid'ing on a vfork parent deadlocks the supervisor (parent can't enter ptrace-stop until vfork wait clears, vfork wait can't clear until we send Continue, we can't send Continue until waitpid returns). Skipping the seize for those tasks is what makes the freeze terminate.

    Thanks for filing this — and for not letting the silence after #29 stand. Issue #27 stayed open for the right reason.

  4. Changaco commented on May 2, 2026

    @Changaco
    Author

    I think there still is a race condition. register_child_if_new is only called when a process makes a syscall that triggers a seccomp notification, so ProcessIndex might not always contain all the processes in the sandbox.

  5. congwang-mk commented on May 3, 2026

    @congwang-mk
    Contributor

    I think there still is a race condition. register_child_if_new is only called when a process makes a syscall that triggers a seccomp notification, so ProcessIndex might not always contain all the processes in the sandbox.

    You are right. I created another PR for this: #35
    Please let me know if it completely address your concerns here.

  6. Changaco commented on May 4, 2026

    @Changaco
    Author

    Pausing all the processes in the sandbox should effectively prevent the race condition when there's a single sandbox, but I'm now concerned that processes in different sandboxes might be able to race each other's seccomp notifications through shared memory.

  7. congwang-mk commented on May 4, 2026

    @congwang-mk
    Contributor

    Pausing all the processes in the sandbox should effectively prevent the race condition when there's a single sandbox, but I'm now concerned that processes in different sandboxes might be able to race each other's seccomp notifications through shared memory.

    Do you have a specific example I could investigate with?

    By the way, I proposed a kernel primitive SECCOMP_IOCTL_NOTIF_PIN_ARGS https://lore.kernel.org/all/20260504011207.539408-1-xiyou.wangcong@gmail.com/ which hopefully could get rid of the ptrace workaround here. Please let me know your thoughts.

  8. Changaco commented on May 4, 2026

    @Changaco
    Author

    Do you have a specific example I could investigate with?

    Not really. It simply occurred to me that processes in different sandboxes, which in some cases could be instances of the same program, might be able to exploit shared memory just like processes inside the same sandbox can. I don't know if it would work, but don't the two processes merely need write access to the same file and the ability to mmap it?

    By the way, I proposed a kernel primitive SECCOMP_IOCTL_NOTIF_PIN_ARGS https://lore.kernel.org/all/20260504011207.539408-1-xiyou.wangcong@gmail.com/ which hopefully could get rid of the ptrace workaround here. Please let me know your thoughts.

    I've also been looking into how the kernel could enable truly solving the problem of userspace sandboxing, but I'm not a kernel developer, so I started from zero understanding of how syscalls are implemented and how it might be possible to modify them. For seccomp notifications, I was thinking that the supervisor process could modify its copy of the syscall arguments and ask the kernel to use it instead of the racy original in the sandboxed process' memory. This would enable the supervisor to safely check and modify the syscall arguments.

    My other idea is to add a way to designate a sandboxing program to be executed in place every time another program is about to be executed. The designated security program would decide which restrictions to apply depending on the context, then execute the requested program, somewhat like binfmt_misc enables a userspace “interpreter” to implement support for executables other than Linux ELFs.

  9. 1lann commented on Jun 12, 2026

    @1lann

    I still think this is not safe because seccomp notifications has path resolution independent to that of the kernel's and can occur concurrently with other racing FS operations like a renameat when the sandbox unfreezes, the correct solution I believe is to have a supervisor like the one that already exists for the COW features to perform the FS operations like with InjectFdSend to guarantee consistency with the path resolution.

  10. congwang-mk commented on Jun 23, 2026

    @congwang-mk
    Contributor

    I still think this is not safe because seccomp notifications has path resolution independent to that of the kernel's and can occur concurrently with other racing FS operations like a renameat when the sandbox unfreezes, the correct solution I believe is to have a supervisor like the one that already exists for the COW features to perform the FS operations like with InjectFdSend to guarantee consistency with the path resolution.

    Thanks for catching them, @1lann . I have created two different issues to track them separately: #112 and #111 I will fix them as soon as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions