From c6c851403b6bd16c93227c309abad3e95f5476bf Mon Sep 17 00:00:00 2001
From: Tavian Barnes <tavianator@tavianator.com>
Date: Sat, 20 Feb 2021 16:21:42 -0500
Subject: pty: Make forkpty() unsafe

After the child returns from a fork() of a multi-threaded process, it is
undefined behaviour to call non-async-signal-safe functions according to
POSIX.  Since forkpty() is implemented in terms of fork(), those
restrictions should apply to it too.
---
 CHANGELOG.md     |  4 ++++
 src/pty.rs       | 29 ++++++++++++++++++-----------
 test/test_pty.rs |  4 +++-
 3 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 35c8fd7a..00212924 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,7 +5,11 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased] - ReleaseDate
 ### Added
+
 ### Changed
+- Made `forkpty` unsafe, like `fork`
+  (#[1390](https://github.com/nix-rust/nix/pull/1390))
+
 ### Fixed
 ### Removed
 
diff --git a/src/pty.rs b/src/pty.rs
index d67518f4..52da23db 100644
--- a/src/pty.rs
+++ b/src/pty.rs
@@ -302,7 +302,19 @@ pub fn openpty<'a, 'b, T: Into<Option<&'a Winsize>>, U: Into<Option<&'b Termios>
 /// If `winsize` is not `None`, the window size of the slave will be set to
 /// the values in `winsize`. If `termios` is not `None`, the pseudoterminal's
 /// terminal settings of the slave will be set to the values in `termios`.
-pub fn forkpty<'a, 'b, T: Into<Option<&'a Winsize>>, U: Into<Option<&'b Termios>>>(
+///
+/// # Safety
+///
+/// In a multithreaded program, only [async-signal-safe] functions like `pause`
+/// and `_exit` may be called by the child (the parent isn't restricted). Note
+/// that memory allocation may **not** be async-signal-safe and thus must be
+/// prevented.
+///
+/// Those functions are only a small subset of your operating system's API, so
+/// special care must be taken to only invoke code you can control and audit.
+///
+/// [async-signal-safe]: http://man7.org/linux/man-pages/man7/signal-safety.7.html
+pub unsafe fn forkpty<'a, 'b, T: Into<Option<&'a Winsize>>, U: Into<Option<&'b Termios>>>(
     winsize: T,
     termios: U,
 ) -> Result<ForkptyResult> {
@@ -323,20 +335,15 @@ pub fn forkpty<'a, 'b, T: Into<Option<&'a Winsize>>, U: Into<Option<&'b Termios>
         .map(|ws| ws as *const Winsize as *mut _)
         .unwrap_or(ptr::null_mut());
 
-    let res = unsafe {
-        libc::forkpty(master.as_mut_ptr(), ptr::null_mut(), term, win)
-    };
+    let res = libc::forkpty(master.as_mut_ptr(), ptr::null_mut(), term, win);
 
     let fork_result = Errno::result(res).map(|res| match res {
         0 => ForkResult::Child,
         res => ForkResult::Parent { child: Pid::from_raw(res) },
     })?;
 
-    unsafe {
-        Ok(ForkptyResult {
-            master: master.assume_init(),
-            fork_result,
-        })
-    }
+    Ok(ForkptyResult {
+        master: master.assume_init(),
+        fork_result,
+    })
 }
-
diff --git a/test/test_pty.rs b/test/test_pty.rs
index ab347bb0..3b654443 100644
--- a/test/test_pty.rs
+++ b/test/test_pty.rs
@@ -255,7 +255,9 @@ fn test_forkpty() {
 
     let string = "naninani\n";
     let echoed_string = "naninani\r\n";
-    let pty = forkpty(None, None).unwrap();
+    let pty = unsafe {
+        forkpty(None, None).unwrap()
+    };
     match pty.fork_result {
         Child => {
             write(STDOUT_FILENO, string.as_bytes()).unwrap();
-- 
cgit v1.2.3