mkfifo: Fix path-based chmod race #10020

src/uu/mkfifo/src/mkfifo.rs

@@ -4,15 +4,13 @@
 // file that was distributed with this source code.
 
 use clap::{Arg, ArgAction, Command, value_parser};
-use libc::mkfifo;
+use libc::{mkfifo, mode_t, umask};
 use std::ffi::CString;
-use std::fs;
-use std::os::unix::fs::PermissionsExt;
 use uucore::display::Quotable;
 use uucore::error::{UResult, USimpleError};
 use uucore::translate;
 
-use uucore::{format_usage, show};
+use uucore::format_usage;
 
 mod options {
     pub static MODE: &str = "mode";
@@ -38,24 +36,32 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         }
     };
 
+    let has_mode = matches.contains_id(options::MODE);
+    // Set umask to 0 temporarily if -m option is applied
+    // mkfifo applies umask to requested mode
+    let old_umask = if has_mode { unsafe { umask(0) } } else { 0 };
+
     for f in fifos {
-        let err = unsafe {
-            let name = CString::new(f.as_bytes()).unwrap();
-            mkfifo(name.as_ptr(), 0o666)
-        };
+        let name = CString::new(f.as_bytes()).unwrap();
+        let err = unsafe { mkfifo(name.as_ptr(), mode as mode_t) };
+
         if err == -1 {
-            show!(USimpleError::new(
-                1,
-                translate!("mkfifo-error-cannot-create-fifo", "path" => f.quote()),
-            ));
-        }
+            let e = std::io::Error::last_os_error();
 
-        // Explicitly set the permissions to ignore umask
-        if let Err(e) = fs::set_permissions(&f, fs::Permissions::from_mode(mode)) {
-            return Err(USimpleError::new(
-                1,
-                translate!("mkfifo-error-cannot-set-permissions", "path" => f.quote(), "error" => e),
-            ));
+            let err_msg = match e.kind() {
+                std::io::ErrorKind::PermissionDenied => {
+                    translate!(
+                        "mkfifo-error-cannot-set-permissions",
+                        "path" => f.quote(),
+                        "error" => e
+                    )
+                }
+                _ => {
+                    translate!("mkfifo-error-cannot-create-fifo", "path" => f.quote())
+                }
+            };
+
+            return Err(USimpleError::new(1, err_msg));
         }
 
         // Apply SELinux context if requested
@@ -70,13 +76,17 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
                 if let Err(e) =
                     uucore::selinux::set_selinux_security_context(Path::new(&f), context)
                 {
-                    let _ = fs::remove_file(f);
+                    let _ = std::fs::remove_file(f);
                     return Err(USimpleError::new(1, e.to_string()));
                 }
             }
         }
     }
 
+    if has_mode {
+        unsafe { umask(old_umask) };
+    }
+
     Ok(())
 }
 
@@ -124,6 +134,6 @@ fn calculate_mode(mode_option: Option<&String>) -> Result<u32, String> {
     if let Some(m) = mode_option {
         uucore::mode::parse_chmod(mode, m, false, umask)
     } else {
-        Ok(mode & !umask) // Apply umask if no mode is specified
+        Ok(mode) // current mask will be applied automatically
     }
 }

tests/by-util/test_mkfifo.rs

@@ -138,8 +138,7 @@ fn test_create_fifo_permission_denied() {
     at.set_mode(no_exec_dir, 0o644);
 
     let err_msg = format!(
-        "mkfifo: cannot create fifo '{named_pipe}': File exists
-mkfifo: cannot set permissions on '{named_pipe}': Permission denied (os error 13)
+        "mkfifo: cannot set permissions on '{named_pipe}': Permission denied (os error 13)
 "
     );