search

notify-rs / notify

🔭 Cross-platform filesystem notification library for Rust.
https://docs.rs/notify
2.77k stars 222 forks source link

Fix UB in `windows::start_read` #602

Closed encounter closed 5 months ago

encounter commented 5 months ago

The primary issue is that mem::transmute from isize to Box<_> (without first casting to *mut _) is undefined behavior.

Rust Playground example (check with Tools -> Miri)

On Rust v1.78.0+, this ends up crashing with STATUS_ILLEGAL_INSTRUCTION when ReadDirectoryChangesW fails and this branch is hit in release mode. One easy way to hit this branch is to attempt to create a notifier for \\wsl.localhost\... or similar.

While this could be fixed by simply adding as *mut ReadDirectoryRequest, this cleans up the overall unsafe logic to be more readable and idiomatic Rust.

0xpr03 commented 5 months ago

Ugh looks like freebsd needs a CI fix. Let's see when I can actually fix this all up.

0xpr03 commented 5 months ago

Hm I would have liked #604 to be a separate commit. This way it is harder to diff. Whole thing is already hairy enough.

0xpr03 commented 5 months ago

I will merge the other one first, that will also credit the author and merges the part we know to be working. Please do me the favor and rebase on top of that.