Skip to content

Commit f4b60ba

Browse files
committed
refactor(mknod): remove unsafe libc::mknod and libc::umask calls
Replace unsafe libc::mknod() with safe rustix::fs::mknodat(CWD, ...) and unsafe libc::umask() with safe rustix::process::umask().
1 parent 43c6b07 commit f4b60ba

3 files changed

Lines changed: 38 additions & 35 deletions

File tree

Cargo.lock

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/uu/mknod/Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ path = "src/mknod.rs"
2323
clap = { workspace = true }
2424
uucore = { workspace = true, features = ["mode", "fs"] }
2525
fluent = { workspace = true }
26-
nix = { workspace = true }
26+
libc = { workspace = true }
27+
rustix = { workspace = true, features = ["fs", "process"] }
2728

2829
[features]
2930
selinux = ["uucore/selinux"]

src/uu/mknod/src/mknod.rs

Lines changed: 34 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,7 @@
66
// spell-checker:ignore (ToDO) parsemode makedev sysmacros perror IFBLK IFCHR IFIFO sflag
77

88
use clap::{Arg, ArgAction, Command, value_parser};
9-
use nix::libc::{S_IRGRP, S_IROTH, S_IRUSR, S_IWGRP, S_IWOTH, S_IWUSR, mode_t};
10-
use nix::sys::stat::{Mode, SFlag, mknod as nix_mknod, umask as nix_umask};
9+
use libc::{S_IRGRP, S_IROTH, S_IRUSR, S_IWGRP, S_IWOTH, S_IWUSR, mode_t};
1110

1211
use uucore::display::Quotable;
1312
use uucore::error::{UResult, USimpleError, UUsageError, set_exit_code};
@@ -28,28 +27,18 @@ mod options {
2827
}
2928

3029
#[derive(Clone, PartialEq)]
31-
enum FileType {
30+
enum FileTypeArg {
3231
Block,
3332
Character,
3433
Fifo,
3534
}
3635

37-
impl FileType {
38-
fn as_sflag(&self) -> SFlag {
39-
match self {
40-
Self::Block => SFlag::S_IFBLK,
41-
Self::Character => SFlag::S_IFCHR,
42-
Self::Fifo => SFlag::S_IFIFO,
43-
}
44-
}
45-
}
46-
4736
/// Configuration for special inode creation.
4837
struct Config {
4938
/// Permission bits for the inode
5039
mode: Mode,
5140

52-
file_type: FileType,
41+
file_type: FileTypeArg,
5342

5443
/// when false, the exact mode bits will be set
5544
use_umask: bool,
@@ -66,34 +55,46 @@ struct Config {
6655
}
6756

6857
fn mknod(file_name: &str, config: Config) -> i32 {
58+
use rustix::fs::Mode;
59+
use rustix::process::umask;
60+
6961
// set umask to 0 and store previous umask
7062
let have_prev_umask = if config.use_umask {
7163
None
7264
} else {
73-
Some(nix_umask(Mode::empty()))
65+
Some(umask(Mode::empty()))
7466
};
7567

76-
let mknod_err = nix_mknod(
77-
file_name,
78-
config.file_type.as_sflag(),
79-
config.mode,
80-
config.dev as _,
81-
)
82-
.err();
83-
let errno = if mknod_err.is_some() { -1 } else { 0 };
68+
let file_type_bits: mode_t = match config.file_type {
69+
FileTypeArg::Block => libc::S_IFBLK,
70+
FileTypeArg::Character => libc::S_IFCHR,
71+
FileTypeArg::Fifo => libc::S_IFIFO,
72+
};
73+
let c_path = std::ffi::CString::new(file_name).unwrap();
74+
// SAFETY: c_path is a valid null-terminated C string
75+
let ret = unsafe {
76+
libc::mknod(
77+
c_path.as_ptr(),
78+
file_type_bits | config.mode as mode_t,
79+
config.dev as _,
80+
)
81+
};
8482

8583
// set umask back to original value
8684
if let Some(prev_umask) = have_prev_umask {
87-
nix_umask(prev_umask);
85+
umask(prev_umask);
8886
}
8987

90-
if let Some(err) = mknod_err {
88+
let errno = if ret != 0 {
9189
eprintln!(
9290
"{}: {}",
9391
uucore::execution_phrase(),
9492
std::io::Error::from(err)
9593
);
96-
}
94+
-1
95+
} else {
96+
0
97+
};
9798

9899
// Apply SELinux context if requested
99100
#[cfg(feature = "selinux")]
@@ -131,7 +132,7 @@ fn mknod(file_name: &str, config: Config) -> i32 {
131132
pub fn uumain(args: impl uucore::Args) -> UResult<()> {
132133
let matches = uucore::clap_localization::handle_clap_result(uu_app(), args)?;
133134

134-
let file_type = matches.get_one::<FileType>("type").unwrap();
135+
let file_type = matches.get_one::<FileTypeArg>("type").unwrap();
135136

136137
let mut use_umask = true;
137138
let mode_permissions = match matches.get_one::<String>("mode") {
@@ -158,8 +159,8 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
158159
matches.get_one::<u32>(options::MAJOR),
159160
matches.get_one::<u32>(options::MINOR),
160161
) {
161-
(FileType::Fifo, None, None) => 0,
162-
(FileType::Fifo, _, _) => {
162+
(FileTypeArg::Fifo, None, None) => 0,
163+
(FileTypeArg::Fifo, _, _) => {
163164
return Err(UUsageError::new(
164165
1,
165166
translate!("mknod-error-fifo-no-major-minor"),
@@ -266,16 +267,16 @@ fn parse_mode(str_mode: &str) -> Result<u32, String> {
266267
})
267268
}
268269

269-
fn parse_type(tpe: &str) -> Result<FileType, String> {
270+
fn parse_type(tpe: &str) -> Result<FileTypeArg, String> {
270271
// Only check the first character, to allow mnemonic usage like
271272
// 'mknod /dev/rst0 character 18 0'.
272273
tpe.chars()
273274
.next()
274275
.ok_or_else(|| translate!("mknod-error-missing-device-type"))
275276
.and_then(|first_char| match first_char {
276-
'b' => Ok(FileType::Block),
277-
'c' | 'u' => Ok(FileType::Character),
278-
'p' => Ok(FileType::Fifo),
277+
'b' => Ok(FileTypeArg::Block),
278+
'c' | 'u' => Ok(FileTypeArg::Character),
279+
'p' => Ok(FileTypeArg::Fifo),
279280
_ => Err(translate!("mknod-error-invalid-device-type", "type" => tpe.quote())),
280281
})
281282
}

0 commit comments

Comments
 (0)