Skip to content

Commit

Permalink
fs-watch: Use a properly sized buffer for inotify events (linkerd#195)
Browse files Browse the repository at this point in the history
The buffer passed to `read(2)` to read inotify events from the inotify
fd must be long enough to fit events with filenames of the maximum
allowable length. The `inotify-rs` library previously always constructed
such a buffer, but as of 1fe4e38, we
are required to provide our own buffer. The buffer we were providing was
insufficiently large for some filenames.

See also the `man` page for `inotify`:
> The behavior when the buffer given to read(2) is too small to return
> information about the next event depends on the kernel version: in
> kernels before 2.6.21, read(2) returns 0; since kernel 2.6.21,
> read(2) fails with the error EINVAL.  Specifying a buffer of size
>
>         sizeof(struct inotify_event) + NAME_MAX + 1
>
> will be sufficient to read at least one event.

(http://man7.org/linux/man-pages/man7/inotify.7.html)

This branch updates the `fs-watch` lib to calculate the necessary buffer
length correctly, and use a properly-sized buffer. I've also added tests
that fail when the buffer is too short.

Fixes linkerd#2331

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
  • Loading branch information
hawkw committed Feb 22, 2019
1 parent 23e02a6 commit c54377f
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 8 deletions.
9 changes: 5 additions & 4 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -423,8 +423,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index"

[[package]]
name = "inotify"
version = "0.6.2-dev"
source = "git+https://github.com/inotify-rs/inotify?rev=f78ed9788d437a78ff253ef3b8fafc61932692d5#f78ed9788d437a78ff253ef3b8fafc61932692d5"
version = "0.7.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"bitflags 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
"futures 0.1.23 (registry+https://github.com/rust-lang/crates.io-index)",
Expand Down Expand Up @@ -519,7 +519,8 @@ dependencies = [
"env_logger 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)",
"futures 0.1.23 (registry+https://github.com/rust-lang/crates.io-index)",
"futures-watch 0.1.0 (git+https://github.com/carllerche/better-future)",
"inotify 0.6.2-dev (git+https://github.com/inotify-rs/inotify?rev=f78ed9788d437a78ff253ef3b8fafc61932692d5)",
"inotify 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)",
"inotify-sys 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
"libc 0.2.48 (registry+https://github.com/rust-lang/crates.io-index)",
"linkerd2-task 0.1.0",
"log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
Expand Down Expand Up @@ -1910,7 +1911,7 @@ dependencies = [
"checksum hyper 0.12.24 (registry+https://github.com/rust-lang/crates.io-index)" = "fdfa9b401ef6c4229745bb6e9b2529192d07b920eed624cdee2a82348cd550af"
"checksum idna 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "014b298351066f1512874135335d62a789ffe78a9974f94b43ed5621951eaf7d"
"checksum indexmap 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)" = "7e81a7c05f79578dbc15793d8b619db9ba32b4577003ef3af1a91c416798c58d"
"checksum inotify 0.6.2-dev (git+https://github.com/inotify-rs/inotify?rev=f78ed9788d437a78ff253ef3b8fafc61932692d5)" = "<none>"
"checksum inotify 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)" = "24e40d6fd5d64e2082e0c796495c8ef5ad667a96d03e5aaa0becfd9d47bcbfb8"
"checksum inotify-sys 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)" = "e74a1aa87c59aeff6ef2cc2fa62d41bc43f54952f55652656b18a02fd5e356c0"
"checksum iovec 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "dbe6e417e7d0975db6512b90796e8ce223145ac4e33c377e4a42882a0e88bb08"
"checksum ipconfig 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)" = "fccb81dd962b29a25de46c4f46e497b75117aa816468b6fff7a63a598a192394"
Expand Down
3 changes: 2 additions & 1 deletion lib/fs-watch/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ tokio-timer = "0.2.4"

[target.'cfg(target_os = "linux")'.dependencies]
libc = "0.2"
inotify = { git = "https://github.com/inotify-rs/inotify", rev = "f78ed9788d437a78ff253ef3b8fafc61932692d5" }
inotify = "0.7"
inotify-sys = "0.1"
procinfo = "0.4.2"

[dev-dependencies]
Expand Down
35 changes: 32 additions & 3 deletions lib/fs-watch/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,14 +177,21 @@ impl PathAndHash {
#[cfg(target_os = "linux")]
pub mod inotify {
extern crate inotify;
extern crate inotify_sys;
extern crate libc;

use self::inotify::{Event, EventMask, EventStream, Inotify, WatchMask};
use self::inotify_sys as ffi;

use futures::{Async, Poll, Stream};
use std::{io, path::PathBuf};
use std::{io, mem, path::PathBuf};

const EVENT_BUF_SZ: usize =
mem::size_of::<ffi::inotify_event>() + (libc::FILENAME_MAX as usize) + 1;

pub struct WatchStream {
inotify: Inotify,
stream: EventStream<[u8; 32]>,
stream: EventStream<Vec<u8>>,
paths: Vec<PathBuf>,
}

Expand All @@ -196,7 +203,7 @@ pub mod inotify {
impl WatchStream {
pub fn new(paths: Vec<PathBuf>) -> Result<Self, io::Error> {
let mut inotify = Inotify::init()?;
let stream = inotify.event_stream([0; 32]);
let stream = inotify.event_stream(Vec::from(&[0; EVENT_BUF_SZ][..]));

let mut watch_stream = WatchStream {
inotify,
Expand Down Expand Up @@ -812,4 +819,26 @@ mod tests {
fn inotify_nonexistent_files_dont_file_delete_events() {
Fixture::new().test_inotify(test_detects_delete_and_recreate)
}

const LONG: &str = "loooooooooooooooooooooooooooooooooooooooooooooooongboi";

#[test]
#[cfg(target_os = "linux")]
fn polling_handles_a_really_long_filename() {
let mut fixture = Fixture::new();
let long = fixture.dir.path().join(LONG);
fixture.paths.push(long);

fixture.test_polling(test_detects_create)
}

#[test]
#[cfg(target_os = "linux")]
fn inotify_handles_a_really_long_filename() {
let mut fixture = Fixture::new();
let long = fixture.dir.path().join(LONG);
fixture.paths.push(long);

fixture.test_inotify(test_detects_create)
}
}

0 comments on commit c54377f

Please sign in to comment.