ruleset: Use OwnedFd instead of RawFd

Improve consistency by delegating lifetime management to OwnedFd.  The
only visible change would be that with latest Rust versions, the
duplicated file descriptor will be greater than 2 (not with Rust 1.63).
See https://doc.rust-lang.org/1.63.0/std/os/unix/io/struct.OwnedFd.html

Remove the explicit RulesetCreated's Drop implementation which is now
handled by its inner OwnedFd.

Replate IntoRawFd with the safer Into<Option<OwnedFd>> for
RulesetCreated.  The former is implemented for OwnedFd.  This is not a
breaking change because the IntoRawFd implementation was never released.

Add tests leveraging RulesetCreated::try_clone().

Signed-off-by: Mickaël Salaün <mic@digikod.net>

Author: l0kod

src/fs.rs

@@ -318,7 +318,7 @@ fn path_beneath_try_compat_children() {
     // Do not actually perform any syscall.
     ruleset.compat.state = CompatState::Dummy;
     assert!(matches!(
-        RulesetCreated::new(ruleset, -1)
+        RulesetCreated::new(ruleset, None)
             .set_compatibility(CompatLevel::HardRequirement)
             .add_rule(PathBeneath::new(PathFd::new("/dev/null").unwrap(), access_file))
             .unwrap_err(),
@@ -332,7 +332,7 @@ fn path_beneath_try_compat_children() {
     // Do not actually perform any syscall.
     ruleset.compat.state = CompatState::Dummy;
     assert!(matches!(
-        RulesetCreated::new(ruleset, -1)
+        RulesetCreated::new(ruleset, None)
             .set_compatibility(CompatLevel::HardRequirement)
             .add_rule(PathBeneath::new(PathFd::new("/dev/null").unwrap(), access_file))
             .unwrap_err(),

src/lib.rs

@@ -452,4 +452,33 @@ mod tests {
             false,
         );
     }
+
+    #[test]
+    fn ruleset_created_try_clone_ownedfd() {
+        use std::os::unix::io::{AsRawFd, OwnedFd};
+
+        let abi = ABI::V1;
+        check_ruleset_support(
+            abi,
+            Some(abi),
+            move |ruleset: Ruleset| -> _ {
+                let ruleset1 = ruleset.handle_access(AccessFs::from_all(abi))?.create()?;
+                let ruleset2 = ruleset1.try_clone().unwrap();
+                let ruleset3 = ruleset2.try_clone().unwrap();
+
+                let some1: Option<OwnedFd> = ruleset1.into();
+                if let Some(fd1) = some1 {
+                    assert!(fd1.as_raw_fd() >= 0);
+
+                    let some2: Option<OwnedFd> = ruleset2.into();
+                    let fd2 = some2.unwrap();
+                    assert!(fd2.as_raw_fd() >= 0);
+
+                    assert_ne!(fd1.as_raw_fd(), fd2.as_raw_fd());
+                }
+                Ok(ruleset3.restrict_self()?)
+            },
+            false,
+        );
+    }
 }

src/ruleset.rs

@@ -6,10 +6,9 @@ use crate::{
     Compatibility, Compatible, CreateRulesetError, HandledAccess, PrivateHandledAccess,
     RestrictSelfError, RulesetError, Scope, ScopeError, TryCompat,
 };
-use libc::close;
 use std::io::Error;
 use std::mem::size_of_val;
-use std::os::unix::io::{IntoRawFd, RawFd};
+use std::os::unix::io::{AsRawFd, FromRawFd, OwnedFd};
 
 #[cfg(test)]
 use crate::*;
@@ -272,7 +271,7 @@ impl Ruleset {
                         CompatLevel::HardRequirement => {
                             Err(CreateRulesetError::MissingHandledAccess)
                         }
-                        _ => Ok(RulesetCreated::new(self, -1)),
+                        _ => Ok(RulesetCreated::new(self, None)),
                     }
                 }
                 CompatState::Full | CompatState::Partial => {
@@ -290,7 +289,10 @@ impl Ruleset {
                         scoped: self.actual_scoped.bits(),
                     };
                     match unsafe { uapi::landlock_create_ruleset(&attr, size_of_val(&attr), 0) } {
-                        fd if fd >= 0 => Ok(RulesetCreated::new(self, fd)),
+                        fd if fd >= 0 => Ok(RulesetCreated::new(
+                            self,
+                            Some(unsafe { OwnedFd::from_raw_fd(fd) }),
+                        )),
                         _ => Err(CreateRulesetError::CreateRulesetCall {
                             source: Error::last_os_error(),
                         }),
@@ -600,15 +602,20 @@ pub trait RulesetCreatedAttr: Sized + AsMut<RulesetCreated> + Compatible {
             };
             match self_ref.compat.state {
                 CompatState::Init | CompatState::No | CompatState::Dummy => Ok(self),
-                CompatState::Full | CompatState::Partial => match unsafe {
-                    uapi::landlock_add_rule(self_ref.fd, T::TYPE_ID, compat_rule.as_ptr(), 0)
-                } {
-                    0 => Ok(self),
-                    _ => Err(AddRuleError::<U>::AddRuleCall {
-                        source: Error::last_os_error(),
+                CompatState::Full | CompatState::Partial => {
+                    #[cfg(test)]
+                    assert!(self_ref.fd.is_some());
+                    let fd = self_ref.fd.as_ref().map(|f| f.as_raw_fd()).unwrap_or(-1);
+                    match unsafe {
+                        uapi::landlock_add_rule(fd, T::TYPE_ID, compat_rule.as_ptr(), 0)
+                    } {
+                        0 => Ok(self),
+                        _ => Err(AddRuleError::<U>::AddRuleCall {
+                            source: Error::last_os_error(),
+                        }
+                        .into()),
                     }
-                    .into()),
-                },
+                }
             }
         };
         Ok(body()?)
@@ -715,15 +722,15 @@ pub trait RulesetCreatedAttr: Sized + AsMut<RulesetCreated> + Compatible {
 /// Ruleset created with [`Ruleset::create()`].
 #[cfg_attr(test, derive(Debug))]
 pub struct RulesetCreated {
-    fd: RawFd,
+    fd: Option<OwnedFd>,
     no_new_privs: bool,
     pub(crate) requested_handled_fs: BitFlags<AccessFs>,
     pub(crate) requested_handled_net: BitFlags<AccessNet>,
     compat: Compatibility,
 }
 
 impl RulesetCreated {
-    pub(crate) fn new(ruleset: Ruleset, fd: RawFd) -> Self {
+    pub(crate) fn new(ruleset: Ruleset, fd: Option<OwnedFd>) -> Self {
         // The compatibility state is initialized by Ruleset::create().
         #[cfg(test)]
         assert!(!matches!(ruleset.compat.state, CompatState::Init));
@@ -793,7 +800,11 @@ impl RulesetCreated {
                     no_new_privs: enforced_nnp,
                 }),
                 CompatState::Full | CompatState::Partial => {
-                    match unsafe { uapi::landlock_restrict_self(self.fd, 0) } {
+                    #[cfg(test)]
+                    assert!(self.fd.is_some());
+                    // Does not consume ruleset FD, which will be automatically closed after this block.
+                    let fd = self.fd.as_ref().map(|f| f.as_raw_fd()).unwrap_or(-1);
+                    match unsafe { uapi::landlock_restrict_self(fd, 0) } {
                         0 => {
                             self.compat.update(CompatState::Full);
                             Ok(RestrictionStatus {
@@ -818,13 +829,7 @@ impl RulesetCreated {
     /// On error, returns [`std::io::Error`].
     pub fn try_clone(&self) -> std::io::Result<Self> {
         Ok(RulesetCreated {
-            fd: match self.fd {
-                -1 => -1,
-                self_fd => match unsafe { libc::fcntl(self_fd, libc::F_DUPFD_CLOEXEC, 0) } {
-                    dup_fd if dup_fd >= 0 => dup_fd,
-                    _ => return Err(Error::last_os_error()),
-                },
-            },
+            fd: self.fd.as_ref().map(|f| f.try_clone()).transpose()?,
             no_new_privs: self.no_new_privs,
             requested_handled_fs: self.requested_handled_fs,
             requested_handled_net: self.requested_handled_net,
@@ -833,20 +838,21 @@ impl RulesetCreated {
     }
 }
 
-impl Drop for RulesetCreated {
-    fn drop(&mut self) {
-        if self.fd >= 0 {
-            unsafe { close(self.fd) };
-        }
+impl From<RulesetCreated> for Option<OwnedFd> {
+    fn from(ruleset: RulesetCreated) -> Self {
+        ruleset.fd
     }
 }
 
-impl IntoRawFd for RulesetCreated {
-    fn into_raw_fd(self) -> RawFd {
-        let fd = self.fd;
-        std::mem::forget(self);
-        fd
-    }
+#[test]
+fn ruleset_created_ownedfd_none() {
+    let ruleset = Ruleset::from(ABI::Unsupported)
+        .handle_access(AccessFs::Execute)
+        .unwrap()
+        .create()
+        .unwrap();
+    let fd: Option<OwnedFd> = ruleset.into();
+    assert!(fd.is_none());
 }
 
 impl AsMut<RulesetCreated> for RulesetCreated {
@@ -1126,7 +1132,7 @@ fn ruleset_unsupported() {
             .unwrap();
         // Fakes a call to create() to test without involving the kernel (i.e. no
         // landlock_ruleset_create() call).
-        let ruleset_created = RulesetCreated::new(ruleset, -1);
+        let ruleset_created = RulesetCreated::new(ruleset, None);
         assert!(matches!(
             ruleset_created
                 .add_rule(PathBeneath::new(