Merge pull request #8343 from TheJJ/fix-mknod-mode

mknod: set cli specified file mode even when it's 0o666

Author: TheJJ

src/uu/mknod/src/mknod.rs

@@ -49,10 +49,14 @@ impl FileType {
     }
 }
 
-/// Configuration for directory creation.
+/// Configuration for special inode creation.
 pub struct Config<'a> {
+    /// bitmask of inode mode (permissions and file type)
     pub mode: mode_t,
 
+    /// when false, the exact mode bits will be set
+    pub use_umask: bool,
+
     pub dev: dev_t,
 
     /// Set `SELinux` security context.
@@ -65,18 +69,19 @@ pub struct Config<'a> {
 fn mknod(file_name: &str, config: Config) -> i32 {
     let c_str = CString::new(file_name).expect("Failed to convert to CString");
 
-    // the user supplied a mode
-    let set_umask = config.mode & MODE_RW_UGO != MODE_RW_UGO;
-
     unsafe {
-        // store prev umask
-        let last_umask = if set_umask { libc::umask(0) } else { 0 };
+        // set umask to 0 and store previous umask
+        let have_prev_umask = if config.use_umask {
+            None
+        } else {
+            Some(libc::umask(0))
+        };
 
         let errno = libc::mknod(c_str.as_ptr(), config.mode, config.dev);
 
         // set umask back to original value
-        if set_umask {
-            libc::umask(last_umask);
+        if let Some(prev_umask) = have_prev_umask {
+            libc::umask(prev_umask);
         }
 
         if errno == -1 {
@@ -109,8 +114,16 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     let matches = uu_app().try_get_matches_from(args)?;
 
     let file_type = matches.get_one::<FileType>("type").unwrap();
-    let mode = get_mode(matches.get_one::<String>("mode")).map_err(|e| USimpleError::new(1, e))?
-        | file_type.as_mode();
+
+    let mut use_umask = true;
+    let mode_permissions = match matches.get_one::<String>("mode") {
+        None => MODE_RW_UGO,
+        Some(str_mode) => {
+            use_umask = false;
+            parse_mode(str_mode).map_err(|e| USimpleError::new(1, e))?
+        }
+    };
+    let mode = mode_permissions | file_type.as_mode();
 
     let file_name = matches
         .get_one::<String>("name")
@@ -143,6 +156,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
 
     let config = Config {
         mode,
+        use_umask,
         dev,
         set_selinux_context: set_selinux_context || context.is_some(),
         context,
@@ -210,19 +224,21 @@ pub fn uu_app() -> Command {
         )
 }
 
-fn get_mode(str_mode: Option<&String>) -> Result<mode_t, String> {
-    match str_mode {
-        None => Ok(MODE_RW_UGO),
-        Some(str_mode) => uucore::mode::parse_mode(str_mode)
-            .map_err(|e| translate!("mknod-error-invalid-mode", "error" => e))
-            .and_then(|mode| {
-                if mode > 0o777 {
-                    Err(translate!("mknod-error-mode-permission-bits-only"))
-                } else {
-                    Ok(mode)
-                }
-            }),
-    }
+fn parse_mode(str_mode: &str) -> Result<mode_t, String> {
+    uucore::mode::parse_mode(str_mode)
+        .map_err(|e| {
+            translate!(
+                "mknod-error-invalid-mode",
+                "error" => e
+            )
+        })
+        .and_then(|mode| {
+            if mode > 0o777 {
+                Err(translate!("mknod-error-mode-permission-bits-only"))
+            } else {
+                Ok(mode)
+            }
+        })
 }
 
 fn parse_type(tpe: &str) -> Result<FileType, String> {

tests/by-util/test_mknod.rs

@@ -5,8 +5,11 @@
 
 // spell-checker:ignore nconfined
 
+use std::os::unix::fs::PermissionsExt;
+
 use uutests::new_ucmd;
 use uutests::util::TestScenario;
+use uutests::util::run_ucmd_as_root;
 use uutests::util_name;
 
 #[test]
@@ -120,6 +123,35 @@ fn test_mknod_invalid_mode() {
         .stderr_contains("invalid mode");
 }
 
+#[test]
+fn test_mknod_mode_permissions() {
+    for test_mode in [0o0666, 0o0000, 0o0444, 0o0004, 0o0040, 0o0400, 0o0644] {
+        let ts = TestScenario::new(util_name!());
+        let filename = format!("null_file-{test_mode:04o}");
+
+        if let Ok(result) = run_ucmd_as_root(
+            &ts,
+            &[
+                "--mode",
+                &format!("{test_mode:04o}"),
+                &filename,
+                "c",
+                "1",
+                "3",
+            ],
+        ) {
+            result.success().no_stdout();
+        } else {
+            print!("Test skipped; `mknod c 1 3` for null char dev requires root user");
+            break;
+        }
+
+        assert!(ts.fixtures.is_char_device(&filename));
+        let permissions = ts.fixtures.metadata(&filename).permissions();
+        assert_eq!(test_mode, PermissionsExt::mode(&permissions) & 0o777);
+    }
+}
+
 #[test]
 #[cfg(feature = "feat_selinux")]
 fn test_mknod_selinux() {

tests/uutests/src/lib/util.rs

@@ -1145,6 +1145,19 @@ impl AtPath {
         }
     }
 
+    #[cfg(not(windows))]
+    pub fn is_char_device(&self, char_dev: &str) -> bool {
+        unsafe {
+            let name = CString::new(self.plus_as_string(char_dev)).unwrap();
+            let mut stat: libc::stat = std::mem::zeroed();
+            if libc::stat(name.as_ptr(), &mut stat) >= 0 {
+                libc::S_IFCHR & stat.st_mode as libc::mode_t != 0
+            } else {
+                false
+            }
+        }
+    }
+
     pub fn hard_link(&self, original: &str, link: &str) {
         log_info(
             "hard_link",