Merge #930

930: Add wrapper for linux kernel module loading r=Susurrus a=bachp

- init_module and finit_module to load kernel modules
- delete_module to unload kernel modules

Co-authored-by: Pascal Bach <pascal.bach@nextrem.ch>

18 files changed, 368 insertions, 70 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 52bd2f24..b63ea5b3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
   ([#923](https://github.com/nix-rust/nix/pull/923))
 - Added a `dir` module for reading directories (wraps `fdopendir`, `readdir`, and `rewinddir`).
   ([#916](https://github.com/nix-rust/nix/pull/916))
+- Added `kmod` module that allows loading and unloading kernel modules on Linux.
+  ([#930](https://github.com/nix-rust/nix/pull/930))
 
 ### Changed
 - Increased required Rust version to 1.22.1/
diff --git a/Cargo.toml b/Cargo.toml
index 063097ab..f2ed0b06 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -23,7 +23,7 @@ cc = "1"
 [dev-dependencies]
 bytes = "0.4.8"
 lazy_static = "1"
-rand = "0.4"
+rand = "0.5"
 tempfile = "3"
 
 [target.'cfg(target_os = "freebsd")'.dev-dependencies]
diff --git a/src/kmod.rs b/src/kmod.rs
new file mode 100644
index 00000000..e853261b
--- /dev/null
+++ b/src/kmod.rs
@@ -0,0 +1,123 @@
+//! Load and unload kernel modules.
+//!
+//! For more details see
+
+use libc;
+use std::ffi::CStr;
+use std::os::unix::io::AsRawFd;
+
+use errno::Errno;
+use Result;
+
+/// Loads a kernel module from a buffer.
+///
+/// It loads an ELF image into kernel space,
+/// performs any necessary symbol relocations,
+/// initializes module parameters to values provided by the caller,
+/// and then runs the module's init function.
+///
+/// This function requires `CAP_SYS_MODULE` privilege.
+///
+/// The `module_image` argument points to a buffer containing the binary image
+/// to be loaded. The buffer should contain a valid ELF image
+/// built for the running kernel.
+///
+/// The `param_values` argument is a string containing space-delimited specifications
+/// of the values for module parameters.
+/// Each of the parameter specifications has the form:
+///
+/// `name[=value[,value...]]`
+///
+/// # Example
+///
+/// ```no_run
+/// use std::fs::File;
+/// use std::io::Read;
+/// use std::ffi::CString;
+/// use nix::kmod::init_module;
+///
+/// let mut f = File::open("mykernel.ko").unwrap();
+/// let mut contents: Vec<u8> = Vec::new();
+/// f.read_to_end(&mut contents).unwrap();
+/// init_module(&mut contents, &CString::new("who=Rust when=Now,12").unwrap()).unwrap();
+/// ```
+///
+/// See [`man init_module(2)`](http://man7.org/linux/man-pages/man2/init_module.2.html) for more information.
+pub fn init_module(module_image: &[u8], param_values: &CStr) -> Result<()> {
+    let res = unsafe {
+        libc::syscall(
+            libc::SYS_init_module,
+            module_image.as_ptr(),
+            module_image.len(),
+            param_values.as_ptr(),
+        )
+    };
+
+    Errno::result(res).map(drop)
+}
+
+libc_bitflags!(
+    /// Flags used by the `finit_module` function.
+    pub struct ModuleInitFlags: libc::c_uint {
+        /// Ignore symbol version hashes.
+        MODULE_INIT_IGNORE_MODVERSIONS;
+        /// Ignore kernel version magic.
+        MODULE_INIT_IGNORE_VERMAGIC;
+    }
+);
+
+/// Loads a kernel module from a given file descriptor.
+///
+/// # Example
+///
+/// ```no_run
+/// use std::fs::File;
+/// use std::ffi::CString;
+/// use nix::kmod::{finit_module, ModuleInitFlags};
+///
+/// let f = File::open("mymod.ko").unwrap();
+/// finit_module(&f, &CString::new("").unwrap(), ModuleInitFlags::empty()).unwrap();
+/// ```
+///
+/// See [`man init_module(2)`](http://man7.org/linux/man-pages/man2/init_module.2.html) for more information.
+pub fn finit_module<T: AsRawFd>(fd: &T, param_values: &CStr, flags: ModuleInitFlags) -> Result<()> {
+    let res = unsafe {
+        libc::syscall(
+            libc::SYS_finit_module,
+            fd.as_raw_fd(),
+            param_values.as_ptr(),
+            flags.bits(),
+        )
+    };
+
+    Errno::result(res).map(drop)
+}
+
+libc_bitflags!(
+    /// Flags used by `delete_module`.
+    ///
+    /// See [`man delete_module(2)`](http://man7.org/linux/man-pages/man2/delete_module.2.html)
+    /// for a detailed description how these flags work.
+    pub struct DeleteModuleFlags: libc::c_int {
+        O_NONBLOCK;
+        O_TRUNC;
+    }
+);
+
+/// Unloads the kernel module with the given name.
+///
+/// # Example
+///
+/// ```no_run
+/// use std::ffi::CString;
+/// use nix::kmod::{delete_module, DeleteModuleFlags};
+///
+/// delete_module(&CString::new("mymod").unwrap(), DeleteModuleFlags::O_NONBLOCK).unwrap();
+/// ```
+///
+/// See [`man delete_module(2)`](http://man7.org/linux/man-pages/man2/delete_module.2.html) for more information.
+pub fn delete_module(name: &CStr, flags: DeleteModuleFlags) -> Result<()> {
+    let res = unsafe { libc::syscall(libc::SYS_delete_module, name.as_ptr(), flags.bits()) };
+
+    Errno::result(res).map(drop)
+}
diff --git a/src/lib.rs b/src/lib.rs
index ed96a8e1..48426594 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -43,7 +43,11 @@ pub mod fcntl;
           target_os = "netbsd",
           target_os = "openbsd"))]
 pub mod ifaddrs;
-#[cfg(any(target_os = "linux", target_os = "android"))]
+#[cfg(any(target_os = "android",
+          target_os = "linux"))]
+pub mod kmod;
+#[cfg(any(target_os = "android",
+          target_os = "linux"))]
 pub mod mount;
 #[cfg(any(target_os = "dragonfly",
           target_os = "freebsd",
@@ -57,7 +61,8 @@ pub mod net;
 pub mod poll;
 #[deny(missing_docs)]
 pub mod pty;
-#[cfg(any(target_os = "linux", target_os = "android"))]
+#[cfg(any(target_os = "android",
+          target_os = "linux"))]
 pub mod sched;
 pub mod sys;
 // This can be implemented for other platforms as soon as libc
diff --git a/src/pty.rs b/src/pty.rs
index ec250aa7..b7171885 100644
--- a/src/pty.rs
+++ b/src/pty.rs
@@ -108,8 +108,7 @@ pub fn grantpt(fd: &PtyMaster) -> Result<()> {
 /// let slave_name = unsafe { ptsname(&master_fd) }?;
 ///
 /// // Try to open the slave
-/// # #[allow(unused_variables)]
-/// let slave_fd = open(Path::new(&slave_name), OFlag::O_RDWR, Mode::empty())?;
+/// let _slave_fd = open(Path::new(&slave_name), OFlag::O_RDWR, Mode::empty())?;
 /// # Ok(())
 /// # }
 /// ```
diff --git a/test/sys/test_aio.rs b/test/sys/test_aio.rs
index 48399fbd..d4b09b0b 100644
--- a/test/sys/test_aio.rs
+++ b/test/sys/test_aio.rs
@@ -441,8 +441,7 @@ extern fn sigfunc(_: c_int) {
 #[test]
 #[cfg_attr(any(all(target_env = "musl", target_arch = "x86_64"), target_arch = "mips", target_arch = "mips64"), ignore)]
 fn test_write_sigev_signal() {
-    #[allow(unused_variables)]
-    let m = ::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = ::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
     let sa = SigAction::new(SigHandler::Handler(sigfunc),
                             SaFlags::SA_RESETHAND,
                             SigSet::empty());
@@ -580,8 +579,7 @@ fn test_liocb_listio_nowait() {
 #[cfg(not(any(target_os = "ios", target_os = "macos")))]
 #[cfg_attr(any(target_arch = "mips", target_arch = "mips64", target_env = "musl"), ignore)]
 fn test_liocb_listio_signal() {
-    #[allow(unused_variables)]
-    let m = ::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = ::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
     const INITIAL: &[u8] = b"abcdef123456";
     const WBUF: &[u8] = b"CDEF";
     let mut rbuf = vec![0; 4];
diff --git a/test/sys/test_select.rs b/test/sys/test_select.rs
index 19d12fba..cf68700c 100644
--- a/test/sys/test_select.rs
+++ b/test/sys/test_select.rs
@@ -2,7 +2,6 @@ use nix::sys::select::*;
 use nix::unistd::{pipe, write};
 use nix::sys::signal::SigSet;
 use nix::sys::time::{TimeSpec, TimeValLike};
-use std::os::unix::io::RawFd;
 
 #[test]
 pub fn test_pselect() {
diff --git a/test/sys/test_signal.rs b/test/sys/test_signal.rs
index 7d3a9bf2..c8dd9776 100644
--- a/test/sys/test_signal.rs
+++ b/test/sys/test_signal.rs
@@ -28,8 +28,7 @@ fn test_sigprocmask_noop() {
 
 #[test]
 fn test_sigprocmask() {
-    #[allow(unused_variables)]
-    let m = ::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = ::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
 
     // This needs to be a signal that rust doesn't use in the test harness.
     const SIGNAL: Signal = Signal::SIGCHLD;
diff --git a/test/sys/test_signalfd.rs b/test/sys/test_signalfd.rs
index a2f8fd8f..a3b60988 100644
--- a/test/sys/test_signalfd.rs
+++ b/test/sys/test_signalfd.rs
@@ -4,8 +4,7 @@ fn test_signalfd() {
     use nix::sys::signal::{self, raise, Signal, SigSet};
 
     // Grab the mutex for altering signals so we don't interfere with other tests.
-    #[allow(unused_variables)]
-    let m = ::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = ::SIGNAL_MTX.lock().expect("Mutex got poisoned by another test");
 
     // Block the SIGUSR1 signal from automatic processing for this thread
     let mut mask = SigSet::empty();
diff --git a/test/sys/test_termios.rs b/test/sys/test_termios.rs
index 831fc18b..a14b8ce1 100644
--- a/test/sys/test_termios.rs
+++ b/test/sys/test_termios.rs
@@ -19,8 +19,7 @@ fn write_all(f: RawFd, buf: &[u8]) {
 #[test]
 fn test_tcgetattr_pty() {
     // openpty uses ptname(3) internally
-    #[allow(unused_variables)]
-    let m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
 
     let pty = openpty(None, None).expect("openpty failed");
     assert!(termios::tcgetattr(pty.master).is_ok());
@@ -47,8 +46,7 @@ fn test_tcgetattr_ebadf() {
 #[test]
 fn test_output_flags() {
     // openpty uses ptname(3) internally
-    #[allow(unused_variables)]
-    let m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
 
     // Open one pty to get attributes for the second one
     let mut termios = {
@@ -90,8 +88,7 @@ fn test_output_flags() {
 #[test]
 fn test_local_flags() {
     // openpty uses ptname(3) internally
-    #[allow(unused_variables)]
-    let m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
 
     // Open one pty to get attributes for the second one
     let mut termios = {
diff --git a/test/sys/test_uio.rs b/test/sys/test_uio.rs
index 47f3b468..3e4fc28c 100644
--- a/test/sys/test_uio.rs
+++ b/test/sys/test_uio.rs
@@ -1,6 +1,7 @@
 use nix::sys::uio::*;
 use nix::unistd::*;
 use rand::{thread_rng, Rng};
+use rand::distributions::Alphanumeric;
 use std::{cmp, iter};
 use std::fs::{OpenOptions};
 use std::os::unix::io::AsRawFd;
@@ -11,7 +12,7 @@ use tempfile::{tempfile, tempdir};
 fn test_writev() {
     let mut to_write = Vec::with_capacity(16 * 128);
     for _ in 0..16 {
-        let s: String = thread_rng().gen_ascii_chars().take(128).collect();
+        let s: String = thread_rng().sample_iter(&Alphanumeric).take(128).collect();
         let b = s.as_bytes();
         to_write.extend(b.iter().cloned());
     }
@@ -53,7 +54,7 @@ fn test_writev() {
 
 #[test]
 fn test_readv() {
-    let s:String = thread_rng().gen_ascii_chars().take(128).collect();
+    let s:String = thread_rng().sample_iter(&Alphanumeric).take(128).collect();
     let to_write = s.as_bytes().to_vec();
     let mut storage = Vec::new();
     let mut allocated = 0;
@@ -199,8 +200,7 @@ fn test_process_vm_readv() {
     use nix::sys::signal::*;
     use nix::sys::wait::*;
 
-    #[allow(unused_variables)]
-    let m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
+    let _ = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
 
     // Pre-allocate memory in the child, since allocation isn't safe
     // post-fork (~= async-signal-safe)
diff --git a/test/sys/test_wait.rs b/test/sys/test_wait.rs
index 2f68e7c4..d07d82f0 100644
--- a/test/sys/test_wait.rs
+++ b/test/sys/test_wait.rs
@@ -7,8 +7,7 @@ use libc::_exit;
 
 #[test]
 fn test_wait_signal() {
-    #[allow(unused_variables)]
-    let m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
+    let _ = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
 
     // Safe: The child only calls `pause` and/or `_exit`, which are async-signal-safe.
     match fork().expect("Error: Fork Failed") {
@@ -25,8 +24,7 @@ fn test_wait_signal() {
 
 #[test]
 fn test_wait_exit() {
-    #[allow(unused_variables)]
-    let m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
 
     // Safe: Child only calls `_exit`, which is async-signal-safe.
     match fork().expect("Error: Fork Failed") {
@@ -96,8 +94,7 @@ mod ptrace {
 
     #[test]
     fn test_wait_ptrace() {
-        #[allow(unused_variables)]
-        let m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
+        let _m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
 
         match fork().expect("Error: Fork Failed") {
             Child => ptrace_child(),
diff --git a/test/test.rs b/test/test.rs
index a72dc44a..a91b6348 100644
--- a/test/test.rs
+++ b/test/test.rs
@@ -9,9 +9,27 @@ extern crate libc;
 extern crate rand;
 extern crate tempfile;
 
+macro_rules! skip_if_not_root {
+    ($name:expr) => {
+        use nix::unistd::Uid;
+        use std;
+        use std::io::Write;
+
+        if !Uid::current().is_root() {
+            let stderr = std::io::stderr();
+            let mut handle = stderr.lock();
+            writeln!(handle, "{} requires root privileges. Skipping test.", $name).unwrap();
+            return;
+        }
+    };
+}
+
 mod sys;
 mod test_dir;
 mod test_fcntl;
+#[cfg(any(target_os = "android",
+          target_os = "linux"))]
+mod test_kmod;
 #[cfg(any(target_os = "dragonfly",
           target_os = "freebsd",
           target_os = "fushsia",
diff --git a/test/test_kmod/hello_mod/Makefile b/test/test_kmod/hello_mod/Makefile
new file mode 100644
index 00000000..74c99b77
--- /dev/null
+++ b/test/test_kmod/hello_mod/Makefile
@@ -0,0 +1,7 @@
+obj-m += hello.o
+
+all:
+	make -C /lib/modules/$(shell uname -r)/build M=$(shell pwd) modules
+
+clean:
+	make -C /lib/modules/$(shell uname -r)/build M=$(shell pwd) clean
diff --git a/test/test_kmod/hello_mod/hello.c b/test/test_kmod/hello_mod/hello.c
new file mode 100644
index 00000000..1c34987d
--- /dev/null
+++ b/test/test_kmod/hello_mod/hello.c
@@ -0,0 +1,26 @@
+/*
+ *  SPDX-License-Identifier: GPL-2.0+ or MIT
+ */
+#include <linux/module.h>
+#include <linux/kernel.h>
+
+static int number= 1;
+static char *who = "World";
+
+module_param(number, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+MODULE_PARM_DESC(myint, "Just some number");
+module_param(who, charp, 0000);
+MODULE_PARM_DESC(who, "Whot to greet");
+
+int init_module(void)
+{
+	printk(KERN_INFO "Hello %s (%d)!\n", who, number);
+	return 0;
+}
+
+void cleanup_module(void)
+{
+	printk(KERN_INFO "Goodbye %s (%d)!\n", who, number);
+}
+
+MODULE_LICENSE("Dual MIT/GPL");
diff --git a/test/test_kmod/mod.rs b/test/test_kmod/mod.rs
new file mode 100644
index 00000000..e0cb0df5
--- /dev/null
+++ b/test/test_kmod/mod.rs
@@ -0,0 +1,152 @@
+use std::fs::copy;
+use std::path::PathBuf;
+use std::process::Command;
+use tempfile::{tempdir, TempDir};
+
+fn compile_kernel_module() -> (PathBuf, String, TempDir) {
+    let _m = ::FORK_MTX
+        .lock()
+        .expect("Mutex got poisoned by another test");
+
+    let tmp_dir = tempdir().expect("unable to create temporary build directory");
+
+    copy(
+        "test/test_kmod/hello_mod/hello.c",
+        &tmp_dir.path().join("hello.c"),
+    ).expect("unable to copy hello.c to temporary build directory");
+    copy(
+        "test/test_kmod/hello_mod/Makefile",
+        &tmp_dir.path().join("Makefile"),
+    ).expect("unable to copy Makefile to temporary build directory");
+
+    let status = Command::new("make")
+        .current_dir(tmp_dir.path())
+        .status()
+        .expect("failed to run make");
+
+    assert!(status.success());
+
+    // Return the relative path of the build kernel module
+    (tmp_dir.path().join("hello.ko"), "hello".to_owned(), tmp_dir)
+}
+
+use nix::errno::Errno;
+use nix::kmod::{delete_module, DeleteModuleFlags};
+use nix::kmod::{finit_module, init_module, ModuleInitFlags};
+use nix::Error;
+use std::ffi::CString;
+use std::fs::File;
+use std::io::Read;
+
+#[test]
+fn test_finit_and_delete_module() {
+    skip_if_not_root!("test_finit_module");
+
+    let (kmod_path, kmod_name, _kmod_dir) = compile_kernel_module();
+
+    let f = File::open(kmod_path).expect("unable to open kernel module");
+    finit_module(&f, &CString::new("").unwrap(), ModuleInitFlags::empty())
+        .expect("unable to load kernel module");
+
+    delete_module(
+        &CString::new(kmod_name).unwrap(),
+        DeleteModuleFlags::empty(),
+    ).expect("unable to unload kernel module");
+}
+
+#[test]
+fn test_finit_and_delete_modul_with_params() {
+    skip_if_not_root!("test_finit_module_with_params");
+
+    let (kmod_path, kmod_name, _kmod_dir) = compile_kernel_module();
+
+    let f = File::open(kmod_path).expect("unable to open kernel module");
+    finit_module(
+        &f,
+        &CString::new("who=Rust number=2018").unwrap(),
+        ModuleInitFlags::empty(),
+    ).expect("unable to load kernel module");
+
+    delete_module(
+        &CString::new(kmod_name).unwrap(),
+        DeleteModuleFlags::empty(),
+    ).expect("unable to unload kernel module");
+}
+
+#[test]
+fn test_init_and_delete_module() {
+    skip_if_not_root!("test_init_module");
+
+    let (kmod_path, kmod_name, _kmod_dir) = compile_kernel_module();
+
+    let mut f = File::open(kmod_path).expect("unable to open kernel module");
+    let mut contents: Vec<u8> = Vec::new();
+    f.read_to_end(&mut contents)
+        .expect("unable to read kernel module content to buffer");
+    init_module(&mut contents, &CString::new("").unwrap()).expect("unable to load kernel module");
+
+    delete_module(
+        &CString::new(kmod_name).unwrap(),
+        DeleteModuleFlags::empty(),
+    ).expect("unable to unload kernel module");
+}
+
+#[test]
+fn test_init_and_delete_module_with_params() {
+    skip_if_not_root!("test_init_module");
+
+    let (kmod_path, kmod_name, _kmod_dir) = compile_kernel_module();
+
+    let mut f = File::open(kmod_path).expect("unable to open kernel module");
+    let mut contents: Vec<u8> = Vec::new();
+    f.read_to_end(&mut contents)
+        .expect("unable to read kernel module content to buffer");
+    init_module(&mut contents, &CString::new("who=Nix number=2015").unwrap())
+        .expect("unable to load kernel module");
+
+    delete_module(
+        &CString::new(kmod_name).unwrap(),
+        DeleteModuleFlags::empty(),
+    ).expect("unable to unload kernel module");
+}
+
+#[test]
+fn test_finit_module_invalid() {
+    skip_if_not_root!("test_finit_module_invalid");
+
+    let kmod_path = "/dev/zero";
+
+    let f = File::open(kmod_path).expect("unable to open kernel module");
+    let result = finit_module(&f, &CString::new("").unwrap(), ModuleInitFlags::empty());
+
+    assert_eq!(result.unwrap_err(), Error::Sys(Errno::EINVAL));
+}
+
+#[test]
+fn test_finit_module_twice_and_delete_module() {
+    skip_if_not_root!("test_finit_module_twice_and_delete_module");
+
+    let (kmod_path, kmod_name, _kmod_dir) = compile_kernel_module();
+
+    let f = File::open(kmod_path).expect("unable to open kernel module");
+    finit_module(&f, &CString::new("").unwrap(), ModuleInitFlags::empty())
+        .expect("unable to load kernel module");
+
+    let result = finit_module(&f, &CString::new("").unwrap(), ModuleInitFlags::empty());
+
+    assert_eq!(result.unwrap_err(), Error::Sys(Errno::EEXIST));
+
+    delete_module(
+        &CString::new(kmod_name).unwrap(),
+        DeleteModuleFlags::empty(),
+    ).expect("unable to unload kernel module");
+}
+
+#[test]
+fn test_delete_module_not_loaded() {
+    skip_if_not_root!("test_delete_module_not_loaded");
+
+    let result = delete_module(&CString::new("hello").unwrap(), DeleteModuleFlags::empty());
+
+    assert_eq!(result.unwrap_err(), Error::Sys(Errno::ENOENT));
+}
diff --git a/test/test_pty.rs b/test/test_pty.rs
index 0adcc99f..4f428bed 100644
--- a/test/test_pty.rs
+++ b/test/test_pty.rs
@@ -27,8 +27,7 @@ fn test_explicit_close() {
 #[test]
 #[cfg(any(target_os = "android", target_os = "linux"))]
 fn test_ptsname_equivalence() {
-    #[allow(unused_variables)]
-    let m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
 
     // Open a new PTTY master
     let master_fd = posix_openpt(OFlag::O_RDWR).unwrap();
@@ -45,8 +44,7 @@ fn test_ptsname_equivalence() {
 #[test]
 #[cfg(any(target_os = "android", target_os = "linux"))]
 fn test_ptsname_copy() {
-    #[allow(unused_variables)]
-    let m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
 
     // Open a new PTTY master
     let master_fd = posix_openpt(OFlag::O_RDWR).unwrap();
@@ -80,8 +78,7 @@ fn test_ptsname_r_copy() {
 #[test]
 #[cfg(any(target_os = "android", target_os = "linux"))]
 fn test_ptsname_unique() {
-    #[allow(unused_variables)]
-    let m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
 
     // Open a new PTTY master
     let master1_fd = posix_openpt(OFlag::O_RDWR).unwrap();
@@ -103,9 +100,8 @@ fn test_ptsname_unique() {
 /// this test we perform the basic act of getting a file handle for a connect master/slave PTTY
 /// pair.
 #[test]
-fn test_open_ptty_pair() {
-    #[allow(unused_variables)]
-    let m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
+fn test_open_ptty_pair() {    
+    let _m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
 
     // Open a new PTTY master
     let master_fd = posix_openpt(OFlag::O_RDWR).expect("posix_openpt failed");
@@ -126,8 +122,7 @@ fn test_open_ptty_pair() {
 #[test]
 fn test_openpty() {
     // openpty uses ptname(3) internally
-    #[allow(unused_variables)]
-    let m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
 
     let pty = openpty(None, None).unwrap();
     assert!(pty.master > 0);
@@ -162,8 +157,7 @@ fn test_openpty() {
 #[test]
 fn test_openpty_with_termios() {
     // openpty uses ptname(3) internally
-    #[allow(unused_variables)]
-    let m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = ::PTSNAME_MTX.lock().expect("Mutex got poisoned by another test");
 
     // Open one pty to get attributes for the second one
     let mut termios = {
diff --git a/test/test_unistd.rs b/test/test_unistd.rs
index d36a3d39..54cbff8d 100644
--- a/test/test_unistd.rs
+++ b/test/test_unistd.rs
@@ -4,7 +4,7 @@ use nix::unistd::ForkResult::*;
 use nix::sys::signal::{SaFlags, SigAction, SigHandler, SigSet, Signal, sigaction};
 use nix::sys::wait::*;
 use nix::sys::stat::{self, Mode, SFlag};
-use std::{self, env, iter};
+use std::{env, iter};
 use std::ffi::CString;
 use std::fs::File;
 use std::io::Write;
@@ -14,8 +14,7 @@ use libc::{self, _exit, off_t};
 
 #[test]
 fn test_fork_and_waitpid() {
-    #[allow(unused_variables)]
-    let m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
 
     // Safe: Child only calls `_exit`, which is signal-safe
     match fork().expect("Error: Fork Failed") {
@@ -43,8 +42,7 @@ fn test_fork_and_waitpid() {
 #[test]
 fn test_wait() {
     // Grab FORK_MTX so wait doesn't reap a different test's child process
-    #[allow(unused_variables)]
-    let m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
 
     // Safe: Child only calls `_exit`, which is signal-safe
     match fork().expect("Error: Fork Failed") {
@@ -129,15 +127,9 @@ mod linux_android {
 #[cfg(not(any(target_os = "ios", target_os = "macos")))]
 fn test_setgroups() {
     // Skip this test when not run as root as `setgroups()` requires root.
-    if !Uid::current().is_root() {
-        let stderr = std::io::stderr();
-        let mut handle = stderr.lock();
-        writeln!(handle, "test_setgroups requires root privileges. Skipping test.").unwrap();
-        return;
-    }
+    skip_if_not_root!("test_setgroups");
 
-    #[allow(unused_variables)]
-    let m = ::GROUPS_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = ::GROUPS_MTX.lock().expect("Mutex got poisoned by another test");
 
     // Save the existing groups
     let old_groups = getgroups().unwrap();
@@ -159,15 +151,9 @@ fn test_setgroups() {
 fn test_initgroups() {
     // Skip this test when not run as root as `initgroups()` and `setgroups()`
     // require root.
-    if !Uid::current().is_root() {
-        let stderr = std::io::stderr();
-        let mut handle = stderr.lock();
-        writeln!(handle, "test_initgroups requires root privileges. Skipping test.").unwrap();
-        return;
-    }
+    skip_if_not_root!("test_initgroups");
 
-    #[allow(unused_variables)]
-    let m = ::GROUPS_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = ::GROUPS_MTX.lock().expect("Mutex got poisoned by another test");
 
     // Save the existing groups
     let old_groups = getgroups().unwrap();
@@ -195,8 +181,7 @@ macro_rules! execve_test_factory(
     ($test_name:ident, $syscall:ident, $exe: expr $(, $pathname:expr, $flags:expr)*) => (
     #[test]
     fn $test_name() {
-        #[allow(unused_variables)]
-        let m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
+        let _m = ::FORK_MTX.lock().expect("Mutex got poisoned by another test");
         // The `exec`d process will write to `writer`, and we'll read that
         // data from `reader`.
         let (reader, writer) = pipe().unwrap();
@@ -280,8 +265,7 @@ cfg_if!{
 #[test]
 fn test_fchdir() {
     // fchdir changes the process's cwd
-    #[allow(unused_variables)]
-    let m = ::CWD_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = ::CWD_MTX.lock().expect("Mutex got poisoned by another test");
 
     let tmpdir = tempfile::tempdir().unwrap();
     let tmpdir_path = tmpdir.path().canonicalize().unwrap();
@@ -296,8 +280,7 @@ fn test_fchdir() {
 #[test]
 fn test_getcwd() {
     // chdir changes the process's cwd
-    #[allow(unused_variables)]
-    let m = ::CWD_MTX.lock().expect("Mutex got poisoned by another test");
+    let _m = ::CWD_MTX.lock().expect("Mutex got poisoned by another test");
 
     let tmpdir = tempfile::tempdir().unwrap();
     let tmpdir_path = tmpdir.path().canonicalize().unwrap();