Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add libc fanotify information record structs and enums into nix. #2551

Open
carlvoller opened this issue Nov 30, 2024 · 3 comments · May be fixed by #2552
Open

Add libc fanotify information record structs and enums into nix. #2551

carlvoller opened this issue Nov 30, 2024 · 3 comments · May be fixed by #2552

Comments

@carlvoller
Copy link

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?

@SteveLauC
Copy link
Member

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
Copy link
Member

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.

@carlvoller
Copy link
Author

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 carlvoller linked a pull request Dec 1, 2024 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants