`std::fs::remove_dir_all` occasionally panics on Windows with `assertion failed: info.is_aligned()`

[badboy]

remove_dir_all panics on some Windows version when calling std::fs::remove_dir_all due to a newly introduced assert!.

This code:

std::fs::remove_dir_all(&pings_dir)?;

I expected to see this happen: It removes everything or reports an error.

Instead, this happened: It panics with assertion failed: info.is_aligned().

Meta

$ rustc --version
rustc 1.65.0 (897e37553 2022-11-02)

(Crashes on Windows, but we're just using upstream 1.65.0 there).

Full Firefox crash report: https://crash-stats.mozilla.org/report/index/02651884-5b3a-488e-9a43-994b80221106
Firefox bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1799442

Last commit that changed that: c41f21b
PR: #101171
File: https://github.com/rust-lang/rust/blob/master/library/std/src/sys/windows/fs.rs#L737-L739

We saw that on 3 Windows 10 versions: 10.0.18362, 10.0.18363 and 10.0.19044.
Though I haven't checked the latest crashes, it might be more now.

cc @thomcc

[thomcc]

Oof, this sucks! I don't know why this wouldn't be unaligned. As the comment there states, the function's docs state that it should be aligned.

While we could use an unaligned read for the various fields, this would only reduce the alignment requirement to 2bytes. There's not much we can do for the filename array, since we need to provide a &[u16] to the caller. Hopefully that's enough... I'll do this after work.

CC @ChrisDenton who may have some ideas.

[ChrisDenton]

This is concerning to me, for more than just Rust. These structs should really be aligned as documented.

@badboy Do you happen to have any more context for the crashes? My best guess is this may ultimately be caused by a bad filesystem driver (e.g. certain VM shared drives and RAM drives are particularly notorious). If it's the OS itself then that's a bigger issue. Also I'm assuming the crashes are on actual Windows and not WINE or other emulation?

In any case, yeah we'll need to weaken our assumption here which is unfortunate.

[thomcc]

I went through the other places I asserted alignment in #101171, and they should be fine -- those are entirely under our control (unlike this where I relied on the system behaving as documented), so I don't think there's any need to remove them too.

[badboy]

This is concerning to me, for more than just Rust. These structs should really be aligned as documented.

@badboy Do you happen to have any more context for the crashes? My best guess is this may ultimately be caused by a bad filesystem driver (e.g. certain VM shared drives and RAM drives are particularly notorious). If it's the OS itself then that's a bigger issue. Also I'm assuming the crashes are on actual Windows and not WINE or other emulation?

In any case, yeah we'll need to weaken our assumption here which is unfortunate.

From all I see in the Firefox crash reports these are actual machines.
It's definitely buggy behavior compared to the docs, but I don't know where that bug is (Windows bug, driver bug, broken memory/disk, ...)

[badboy]

According to our bots this is a topcrasher on Firefox Nightly.
We might have a workaround there (avoiding that specific codepath overall), it should land with the next nightly, so I will know more after the weekend.

[mati865]

Looking at the report in OP there are at least 2 DLLs that could have caused the issue: SuRun (SuRunExt.dll) that restricts programs permissions and Sandboxie (SbieDll.dll) which is a some kind of a sandbox. I have never used either of them but given their functionality it sounds they could be the reason of this misalignment (I think it's even UB to misalign pointer that is documented as aligned...).

[thomcc]

It being a top crasher is worrying (even if Firefox can work around it, Firefox is not the only Rust application on Windows). That said, if this only impacts sandboxed processes it'd be less of a concern.

[ChrisDenton]

Would anyone be willing to report this to sandboxie?