Skip to content
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

Add Linux-specific pidfd process extensions (take 2) #81825

Merged
merged 7 commits into from
Aug 1, 2021

Commits on Jul 21, 2021

  1. Add Linux-specific pidfd process extensions

    Background:
    
    Over the last year, pidfd support was added to the Linux kernel. This
    allows interacting with other processes. In particular, this allows
    waiting on a child process with a timeout in a race-free way, bypassing
    all of the awful signal-handler tricks that are usually required.
    
    Pidfds can be obtained for a child process (as well as any other
    process) via the `pidfd_open` syscall. Unfortunately, this requires
    several conditions to hold in order to be race-free (i.e. the pid is not
    reused).
    Per `man pidfd_open`:
    
    ```
    · the disposition of SIGCHLD has not been explicitly set to SIG_IGN
     (see sigaction(2));
    
    · the SA_NOCLDWAIT flag was not specified while establishing a han‐
     dler for SIGCHLD or while setting the disposition of that signal to
     SIG_DFL (see sigaction(2)); and
    
    · the zombie process was not reaped elsewhere in the program (e.g.,
     either by an asynchronously executed signal handler or by wait(2)
     or similar in another thread).
    
    If any of these conditions does not hold, then the child process
    (along with a PID file descriptor that refers to it) should instead
    be created using clone(2) with the CLONE_PIDFD flag.
    ```
    
    Sadly, these conditions are impossible to guarantee once any libraries
    are used. For example, C code runnng in a different thread could call
    `wait()`, which is impossible to detect from Rust code trying to open a
    pidfd.
    
    While pid reuse issues should (hopefully) be rare in practice, we can do
    better. By passing the `CLONE_PIDFD` flag to `clone()` or `clone3()`, we
    can obtain a pidfd for the child process in a guaranteed race-free
    manner.
    
    This PR:
    
    This PR adds Linux-specific process extension methods to allow obtaining
    pidfds for processes spawned via the standard `Command` API. Other than
    being made available to user code, the standard library does not make
    use of these pidfds in any way. In particular, the implementation of
    `Child::wait` is completely unchanged.
    
    Two Linux-specific helper methods are added: `CommandExt::create_pidfd`
    and `ChildExt::pidfd`. These methods are intended to serve as a building
    block for libraries to build higher-level abstractions - in particular,
    waiting on a process with a timeout.
    
    I've included a basic test, which verifies that pidfds are created iff
    the `create_pidfd` method is used. This test is somewhat special - it
    should always succeed on systems with the `clone3` system call
    available, and always fail on systems without `clone3` available. I'm
    not sure how to best ensure this programatically.
    
    This PR relies on the newer `clone3` system call to pass the `CLONE_FD`,
    rather than the older `clone` system call. `clone3` was added to Linux
    in the same release as pidfds, so this shouldn't unnecessarily limit the
    kernel versions that this code supports.
    
    Unresolved questions:
    * What should the name of the feature gate be for these newly added
      methods?
    * Should the `pidfd` method distinguish between an error occurring
      and `create_pidfd` not being called?
    Aaron1011 authored and voidc committed Jul 21, 2021
    Configuration menu
    Copy the full SHA
    694be09 View commit details
    Browse the repository at this point in the history
  2. Typo fix

    Co-authored-by: bjorn3 <bjorn3@users.noreply.github.com>
    2 people authored and voidc committed Jul 21, 2021
    Configuration menu
    Copy the full SHA
    ef03de2 View commit details
    Browse the repository at this point in the history
  3. Add PidFd type and seal traits

    Improve docs
    
    Split do_fork into two
    
    Make do_fork unsafe
    
    Add target attribute to create_pidfd field in Command
    
    Add method to get create_pidfd value
    voidc committed Jul 21, 2021
    Configuration menu
    Copy the full SHA
    619fd96 View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    c3321d3 View commit details
    Browse the repository at this point in the history

Commits on Aug 1, 2021

  1. Do not call getpid wrapper after fork in tests

    The test calls libc::getpid() in the pre_exec hook and asserts that the returned value is different from the PID of the parent.
    However, libc::getpid() returns the wrong value.
    Before version 2.25, glibc caches the PID of the current process with the goal of avoiding additional syscalls.
    The cached value is only updated when the wrapper functions for fork or clone are called.
    In PR rust-lang#81825 we switch to directly using the clone3 syscall.
    Thus, the cache is not updated and getpid returns the PID of the parent.
    source: https://man7.org/linux/man-pages/man2/getpid.2.html#NOTES
    voidc committed Aug 1, 2021
    Configuration menu
    Copy the full SHA
    12fbabd View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    2a4d012 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    4a832d3 View commit details
    Browse the repository at this point in the history