ioctl: use bitfield structure

Author: zyuiop

Cargo.lock

@@ -134,6 +134,7 @@ time = { version = "0.3", default-features = false }
 volatile = "0.6"
 zerocopy = { version = "0.8", default-features = false }
 uhyve-interface = "0.1.3"
+bitfield-struct = "0.11.0"
 
 [dependencies.smoltcp]
 version = "0.12"

Cargo.toml

@@ -18,7 +18,7 @@ fn socket_handle_ioctl(
 ) -> io::Result<()> {
 	const FIONBIO: u32 = 0x8008_667eu32;
 
-	if cmd.0 == FIONBIO {
+	if cmd.into_bits() == FIONBIO {
 		let value = unsafe { *(argp as *const i32) };
 		let status_flags = if value != 0 {
 			fd::StatusFlags::O_NONBLOCK

src/fd/socket/mod.rs

@@ -5,49 +5,43 @@ use alloc::sync::Arc;
 use alloc::vec::Vec;
 use core::fmt::Debug;
 
+use bitfield_struct::bitfield;
+
 use crate::fd::{AccessPermission, ObjectInterface};
 use crate::fs::{NodeKind, VfsNode};
 use crate::io;
 
-#[derive(Copy, Clone)]
-pub struct IoCtlCall(pub u32);
-
-impl Debug for IoCtlCall {
-	fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-		f.debug_struct("IoCtlCall")
-			.field("call_nr", &self.call_nr())
-			.field("call_type", &self.call_type())
-			.field("call_size", &self.call_size())
-			.field("call_dir", &self.call_dir())
-			.field(".0", &self.0)
-			.finish()
-	}
+/// Encoding for an IOCTL command, as done in the Linux Kernel.
+///
+/// See [relevant kernel header](https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/asm-generic/ioctl.h?h=v6.15) for reference.
+///
+/// The goal of this interface is to easily support linux applications that communicate via IOCTL,
+/// so linux compatibility is an intended and explicit goal here.
+#[bitfield(u32)]
+pub struct IoCtlCall {
+	call_nr: u8,
+
+	call_type: u8,
+
+	#[bits(2, from = IoCtlDirection::from_bits_truncate, default = IoCtlDirection::empty())]
+	call_dir: IoCtlDirection,
+
+	#[bits(14)]
+	call_size: u16,
 }
 
 bitflags! {
-	#[derive(Debug, Copy, Clone, Default)]
+	#[derive(Debug, Copy, Clone, Default, PartialEq, Eq)]
 	pub struct IoCtlDirection: u8 {
 		const IOC_WRITE = 1;
 		const IOC_READ = 2;
 	}
 }
 
-impl IoCtlCall {
-	pub fn call_nr(&self) -> u8 {
-		(self.0 & 0xff) as u8
-	}
-
-	pub fn call_type(&self) -> u8 {
-		((self.0 >> 8) & 0xff) as u8
-	}
-
-	pub fn call_dir(&self) -> IoCtlDirection {
-		let dir = (self.0 >> 30) & 0x3;
-		IoCtlDirection::from_bits_truncate(dir as u8)
-	}
-
-	pub fn call_size(&self) -> u16 {
-		((self.0 >> 16) & 0x3fff) as u16
+impl IoCtlDirection {
+	// Required for IoCtlCall
+	const fn into_bits(self) -> u8 {
+		self.bits()
 	}
 }
 
@@ -94,3 +88,48 @@ pub(crate) fn register_ioctl(path: &str, ioctl_object: Arc<dyn ObjectInterface>)
 		.traverse_mount(&mut path, Box::new(IoCtlNode(ioctl_object)))
 		.expect("Failed to mount ioctl: filesystem error");
 }
+
+#[cfg(test)]
+mod tests {
+	use crate::fs::ioctl::{IoCtlCall, IoCtlDirection};
+
+	#[test_case]
+	fn ioctl_call_correctly_written() {
+		let call_nr = 0x12u8;
+		let call_type = 0x78u8;
+		let call_dir = IoCtlDirection::IOC_WRITE;
+		let call_size = 0x423u16;
+
+		let ioctl_call_number = (u32::from(call_size) << 18)
+			| (u32::from(call_dir.bits()) << 16)
+			| (u32::from(call_type) << 8)
+			| u32::from(call_nr);
+
+		let call = IoCtlCall::new()
+			.with_call_nr(call_nr)
+			.with_call_type(call_type)
+			.with_call_dir(call_dir)
+			.with_call_size(call_size);
+
+		assert_eq!(ioctl_call_number, call.into_bits());
+	}
+	#[test_case]
+	fn ioctl_call_correctly_parsed() {
+		let call_nr = 0x12u8;
+		let call_type = 0x78u8;
+		let call_dir = IoCtlDirection::IOC_WRITE;
+		let call_size = 0x423u16;
+
+		let ioctl_call_number = (u32::from(call_size) << 18)
+			| (u32::from(call_dir.bits()) << 16)
+			| (u32::from(call_type) << 8)
+			| u32::from(call_nr);
+
+		let parsed = IoCtlCall::from_bits(ioctl_call_number);
+
+		assert_eq!(call_nr, parsed.call_nr());
+		assert_eq!(call_type, parsed.call_type());
+		assert_eq!(call_dir, parsed.call_dir());
+		assert_eq!(call_size, parsed.call_size());
+	}
+}

src/fs/ioctl.rs

@@ -483,7 +483,7 @@ pub unsafe extern "C" fn sys_ioctl(
 	obj.map_or_else(
 		|e| -i32::from(e),
 		|v| {
-			(*v).handle_ioctl(IoCtlCall(cmd as u32), argp)
+			(*v).handle_ioctl(IoCtlCall::from_bits(cmd as u32), argp)
 				.map_or_else(|e| -i32::from(e), |()| 0)
 		},
 	)