nix icon indicating copy to clipboard operation
nix copied to clipboard

Add libc fanotify information record structs and enums into nix.

Open carlvoller opened this issue 11 months ago • 3 comments
trafficstars

Hi, at the moment nix only supports receiving the fanotify_event_metadata from the Fanotify::read_events method. This skips over all the additional information records that could be returned by fanotify (such as when FAN_REPORT_FID is specified in InitFlags).

The libc crate recently implemented the fanotify_event_info_fid, fanotify_event_info_error and fanotify_event_info_pidfd structs, as well as a number of enums for the additional flags and event types.

Would you be open to a PR implementing these APIs into nix?

carlvoller avatar Nov 30 '24 17:11 carlvoller

at the moment nix only supports receiving the fanotify_event_metadata from the Fanotify::read_events method.

Right

Would you be open to a PR implementing these APIs into nix?

Sure, thanks for your interest in contributing to Nix!

Could you please propose the wrapper APIs before implementing them?

SteveLauC avatar Dec 01 '24 03:12 SteveLauC

Could you please propose the wrapper APIs before implementing them?

If we can do this without breaking the existing APIs, that would be great! But it is also ok to have breaking APIs as the next release will be a major release.

SteveLauC avatar Dec 01 '24 03:12 SteveLauC

I've actually previously forked nix to implement the new APIs myself already. But I'll admit that I'm not 100% satisfied with the approach I took.

I wanted to preserve the existing fanotify nix API as much as possible, so I didn't change anything that would break any existing implementations. Instead, I added the method Fanotify::read_events_with_info_records:

impl Fanotify {
  pub fn read_events_with_info_records(
          &self,
  ) -> Result<Vec<(FanotifyEvent, Vec<FanotifyInfoRecord>)>> {
          ...
          Ok(events)
      }
}

Instead of just returning a Vec<FanotifyEvent>, I return a tuple of Vec<(FanotifyEvent, Vec<FanotifyInfoRecord>)> because a single FanotifyEvent can have multiple information records as part of its response (indicated by the event_len on the FanotifyEvent itself). I determine what kind of record it is by first parsing the hdr part of the struct before casting the entire struct as its respective records.

However, there are a number of different possible types of Information Records that can be returned, so I used an Enum so any consumer can easily match over the records:

#[derive(Debug, Eq, Hash, PartialEq)]
#[allow(missing_copy_implementations)]
pub enum FanotifyInfoRecord {
    /// A [`libc::fanotify_event_info_fid`] event was recieved, usually as
    /// a result of passing [`InitFlags::FAN_REPORT_FID`] or [`InitFlags::FAN_REPORT_DIR_FID`]
    /// into [`Fanotify::init`]. The containing struct includes a `file_handle` for
    /// use with `open_by_handle_at(2)`.
    Fid(FanotifyFidRecord),

    /// A [`libc::fanotify_event_info_error`] event was recieved.
    /// This occurs when a FAN_FS_ERROR occurs, indicating an error with
    /// the watch filesystem object. (such as a bad file or bad link lookup)
    Error(FanotifyErrorRecord),

    /// A [`libc::fanotify_event_info_pidfd`] event was recieved, usually as
    /// a result of passing [`InitFlags::FAN_REPORT_PIDFD`] into [`Fanotify::init`].
    /// The containing struct includes a `pidfd` for reliably determining
    /// whether the process responsible for generating an event has been recycled or terminated
    Pidfd(FanotifyPidfdRecord),
}

Each Enum wraps an abstraction over the internal libc record:

pub struct FanotifyFidRecord(libc::fanotify_event_info_fid);

impl FanotifyFidRecord {

    pub fn filesystem_id(&self) -> libc::__kernel_fsid_t {
        self.0.fsid
    }

    pub fn handle(&self) -> [u8; 0] {
        self.0.handle
    }
}

pub struct FanotifyErrorRecord(libc::fanotify_event_info_error);

impl FanotifyErrorRecord {
    pub fn err(&self) -> Errno {
        Errno::from_raw(self.0.error)
    }

    pub fn err_count(&self) -> u32 {
        self.0.error_count
    }
}

pub struct FanotifyPidfdRecord(libc::fanotify_event_info_pidfd);

impl FanotifyPidfdRecord {
    pub fn pidfd(&self) -> Option<BorrowedFd> {
        if self.0.pidfd == libc::FAN_NOPIDFD || self.0.pidfd == libc::FAN_EPIDFD
        {
            None
        } else {
            Some(unsafe { BorrowedFd::borrow_raw(self.0.pidfd) })
        }
    }
}

impl Drop for FanotifyPidfdRecord {
    fn drop(&mut self) {
        if self.0.pidfd == libc::FAN_NOFD {
            return;
        }
        let e = close(self.0.pidfd);
        if !std::thread::panicking() && e == Err(Errno::EBADF) {
            panic!("Closing an invalid file descriptor!");
        };
    }
}

Personally, I'm especially unsure about my implementation of FanotifyFidRecord. This record contains an fsid and a handle.

fsid feels like it SHOULD have a better abstraction, but I couldn't come up with something better personally. Referencing the statfs APIs in nix also points me towards it being okay to just return the libc::__kernel_fsid_t.

handle points to a variable-length struct file_handle from https://man7.org/linux/man-pages/man2/open_by_handle_at.2.html, which doesn't exist in the libc crate at all. I'm open to creating our own file_handle struct on the Rust side to safely handle this, but I'm not sure if the nix crate is the right place for this (thoughts would be good!)

carlvoller avatar Dec 01 '24 07:12 carlvoller