9 ms·
Compiler bug? Linker bug? Windows Kernel bug
- mehrdadn 9y agoHuh. There's a memory-mapped I/O bug I've known about for years, I wonder if this is related to it? It's something along the following lines, but I forget the exact details, so some bits (no pun intended) might be wrong: if you create a memory mapping and then close the underlying file handle, you can even delete the file at that point, but the memory mapping will still hang around and pretend to work, and you won't know it's not working. I'm not sure exactly what happens under the hood but I recall it doesn't behave sanely.
- MaxBarraclough 9y agoThat's undefined behaviour then, right? Even if it's not the sane behaviour. I imagine that might be reason enough for them to treat it as a low-priority.
- mehrdadn 9y agoI'm not sure... UB in general seems to be a language-level construct. OSes (and below) don't really have much UB to speak of, and I assume this is because they are expected to provide security and stability regardless of misbehaving user programs. And also, UB is usually said in response to a program expecting a certain behavior, but in this case I'm not worried about the program itself, but about the system and where exactly the black hole that the physical memory is mapped to is (given that it is no longer a file). In this case, I wouldn't be surprised if there are serious stability or even security issues lying behind this... I just haven't bothered to investigate.
- barrkel 9y agoIt would rather be classified as undocumented behaviour. Undocumented behaviour is frequently maintained in OSes because programs become reliant on it, to the point that sometimes bugs need to be emulated to avoid breakage.
- DoofusOfDeath 9y agoIt's been a long time since I've looked at Windows documentation, but I wonder if it's reasonable to treat their documentation as the actual contract offered by the OS? I had the impression that poor documentation forced a lot of Windows programmers to understand Microsoft's API's via experimentation.
- MaxBarraclough 9y ago> UB in general seems to be a language-level construct Not always. Various standard library functions can cause UB, and not just things like C's div function. In C++, calling front() on an empty std::vector<T> causes UB, for instance. http://www.cplusplus.com/reference/vector/vector/front/ http://www.cplusplus.com/reference/vector/vector/front/ I agree that from a system perspective, it doesn't look like good behaviour.
- mehrdadn 9y ago> Not always. Various standard library functions can cause UB, and not just things like C's div function. In C++, calling front() on an empty std::vector<T> causes UB, for instance. std::vector et al. are parts of the C++ language. "Language" != pure syntax.
- MaxBarraclough 9y agoI think I see your point - that despite the way C++ libraries can and do expose UB if not used as intended, it doesn't mean that Windows does the same. For what it's worth, I read that POSIX does have UB in the spec (as in, distinct from how the compiler is allowed to break things in terms of language-level UB), at least in some ghastly signal handling situations. I wasn't able to find at a glance whether Windows does too, or if it categorically doesn't. > According to POSIX, the behavior of a process is undefined after it ignores a SIGFPE, SIGILL, or SIGSEGV signal that was not generated by kill(2) or raise(3).
- zuzun 9y agoA call to unlink removes the name from the filesystem, but keeps the file around as long as it's still opened by a process; and mmap keeps the file opened, even if you close the original file descriptor.
- mehrdadn 9y ago> A call to unlink removes the name from the filesystem, but keeps the file around as long as it's still opened by a process; and mmap keeps the file opened, even if you close the original file descriptor. Are you familiar with Windows or just assuming all OSes works like *nix systems? That's definitely not at all close to how Windows file systems behave.
- pjmlp 9y agoNot even all UNIXes. It is implementation defined on the UNIX OS variant and mounted file system. The beauty of POSIX is how it appears to be portable, while leaving quite a few things being implementation defined.
- mehrdadn 9y agoToday I learned!
- gpderetta 9y agoNot sure what UNIX variant are you thinking, but from POSIX: > The mmap() function adds an extra reference to the file associated with the file descriptor fildes which is not removed by a subsequent close() on that file descriptor. This reference is removed when there are no more mappings to the file. I'm sure there have been unix variants that got it wrong and might not even have been in posix from the beginning (it is there since at least SUSv2); still I think this is traditional unix behavior.
- zuzun 9y agoYour comment contains no details whatsoever, then you described perfectly sane mmap behavior, so what do I know?
- leeter 9y agoIIRC this isn't a bug but a compatibility hack. The pages really are unmapped in your process but if you try to access them the kernel will remap them for you and lie when you try to remap that file (as it already had the handle open and assigned to your process). This was done because a few well known pieces of software did this back before protected mode. So MS pretends the hack to keep them working. I'm pretty sure this goes away if you list the latest OS in your manifest and have DEP and NX turned on for your app.
- mehrdadn 9y agoThis doesn't seem to be the case. On the latest Windows 10 I tried out this test program HANDLE file = CreateFile(TEXT("\\\\.\\C:\\mmap.bin"), GENERIC_ALL | SYNCHRONIZE, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL, OPEN_ALWAYS, 0 /*can also try: FILE_FLAG_DELETE_ON_CLOSE*/, NULL); if (file != INVALID_HANDLE_VALUE) { HANDLE mapping = CreateFileMapping(file, NULL, PAGE_EXECUTE_READWRITE, 0, 1, NULL); void *ptr = MapViewOfFile(mapping, FILE_MAP_ALL_ACCESS, 0, 0, 0); CloseHandle(file); *static_cast<unsigned char *>(ptr) += 1; } with /NXCOMPAT and even /DYNAMICBASE and with the embedded manifest <?xml version="1.0" encoding="UTF-8" standalone="yes"?> <assembly manifestVersion="1.0" xmlns="urn:schemas-microsoft-com:asm.v1" xmlns:asmv3="urn:schemas-microsoft-com:asm.v3"> <assemblyIdentity type="win32" name="Microsoft.Windows.Foo" version="1.0.0.0"></assemblyIdentity> <trustInfo xmlns="urn:schemas-microsoft-com:asm.v3"> <security> <requestedPrivileges> <requestedExecutionLevel level="asInvoker" uiAccess="false"></requestedExecutionLevel> </requestedPrivileges> </security> </trustInfo> <compatibility xmlns="urn:schemas-microsoft-com:compatibility.v1"> <application> <supportedOS Id="{8e0f7a12-bfb3-4fe8-b9a5-48fd50a15a9a}"></supportedOS> </application> </compatibility> </assembly> but it seems to still modify the file after closure even after the CloseHandle call.
- leeter 9y agoWhen I get time I might actually debug this to see what's going on. I'm quite curious what happens when you access the pointer again. If the Kernel really is remapping the pages again as the anecdotes from Raymond would suggest it might be. Or if it just hasn't reclaimed them yet. VMMap and ProcessExplorer should give the story.
- Athas 9y agoAre PE executable files special in Windows, or could this bug have been hit with normal IO?
- mehrdadn 9y agoI'm not sure in what manner they would need to be special for this, but I would expect the significant part was the fact that it was memory-mapped (which isn't normal I/O), not that they were PE files. Edit (in response to comments): Actually, I don't think it has anything to do with PE files being optimized in some special fashion, because they say loading via LoadLibrary(Ex) can cause this issue too, and those are very much user-mode constructs, and hence can't mess with the caching/paging behavior (which are lower, in the kernel). AFAIK the only kernel-mode component to them is to just create a file mapping (NtCreateSection) and after that all they do is a bunch of user-mode juggling (fixing up relocations, loading other libraries that are depended on, calling entrypoints, etc.).
- ChrisSD 9y agoIndeed but being loaded as code seems important as well. It sounds like the two used together, in certain extreme conditions, causes the issue.
- blattimwind 9y agoSince it's only observed by execution, its entirely possible that the PE loader has been optimized in a low-level fashion leading to bugs that are not exposed in general I/O facilities.
- deleted 9y ago[deleted]
- gmueckl 9y agoMy reading is that PE images are memory mapped into RAM instead of being read from disk. This is why they require write locks on the files during execution: the program image is backed from the corresponding file instead of the normal page file. So my speculation is that this bug is a race condition between the cleanup of a dirty memory mapped section of a file and the reading of the same file contents in a separate mapping.
- dingo_bat 9y agoSo basically if you write and read immediately, windows will give you bad data in some cases. Not a good sign for the underlying design.
- huhtenberg 9y agoNo, basically it's not like that. From the TFA: if a program writes a PE file (EXE or DLL) using memory mapped file I/O and if that program is then immediately executed (or loaded with LoadLibrary or LoadLibraryEx), and if the system is under very heavy disk I/O load then a necessary file-buffer flush may fail
- dingo_bat 9y agoI don't know if you're trolling. > if a program writes a PE file (EXE or DLL) using memory mapped file I/O and Write > if that program is then immediately executed (or loaded Read immediately > then a necessary file-buffer flush may fail You get bad data in memory.
- huhtenberg 9y agoYou've abstracted very specific conditions to the point of absurdity. I don't see any reason for that except for pulling a strawman and taking a stab at Windows.
- quietbritishjim 9y agoHow does your comment contradict the parent one?
- barrkel 9y agoMemory-mapped I/O is a lot more precise a description of "writing" and more unusual than simply writing out, and execution similarly is not a normal read path for a just-written file. The parent made it sounds like you could just write a loop with for(;;) { write_file("x"); read_file("x"); } and expect discrepancies. That's not the case and it's wilfully misleading to suggest it is.
- jwilk 9y agoThe "Avoid Windows kernel bug using Python hack" commit (for people with JS disabled): https://github.com/chromium/chromium/commit/052a09014b2018e2a1e3b7f046ea9ad3355b831e https://github.com/chromium/chromium/commit/052a09014b2018e2...
- bla2 9y agoAnd this looks like the lld-link change: http://llvm.org/viewvc/llvm-project?view=revision&revision=325274 http://llvm.org/viewvc/llvm-project?view=revision&revision=3...
- jwilk 9y agoThe llvm commit on GitHub (which shows whole diff at once): https://github.com/llvm-mirror/llvm/commit/8b2a3e8b203a332bbd71cb9a5708f870dec3ae74 https://github.com/llvm-mirror/llvm/commit/8b2a3e8b203a332bb...
- brucedawson 9y agoThanks for the link. I meant to track that down and I appreciate your doing that for me.
- epberry 9y agoI used to be surprised when people came across these bugs. I would think: a kernel bug! They must have truly hit an edge case for something like that to skip into production. But then I read Showstopper: The Race to Build Windows NT, specifically the part where that OS shipped with 65,000 known defect (and it was considered one of the most stable software releases of all time), and it became clear to me that these bugs are probably known by the team when the OS ships but just not considered high priority enough to fix. Same with the recent articles about iOS and engineers not getting time to fix P2 bugs. Long story short my entire perspective on kernel bugs (and partly software development) changed and I began to think of them more as known flaws that weren’t fixed because of schedules rather than unknown bugs that were uncovered in the wild.
- mehrdadn 9y agoAre data corruption defects treated the same way as other things? i.e., would kernels get shipped with known defects that cause data corruption under load? I could see kernels being shipped with known cases of hanging, crashes, out of memory, etc., but the same for data corruption under load would be a little hard to swallow.
- brucedawson 9y agoBugs are prioritized. Data corruption would be higher than some other bugs, but it also depends on frequency
- DoofusOfDeath 9y agoThis is one aspect of OS development in which I think open source wins: users are not at the mercy of the OS developers for bug tracking and/or fixes.
- chrisper 9y agoAre we not? If I find a bug, I sure hope they fix it soon. I do not have the time and energy to work myself into the kernel to fix the bug (unless it is a small bug that does not require a lot of knowledge about the kernel itself).
- xtrapolate 9y agoAnswer is: "none of the above". Coherency is covered by the WinAPI documentation (check the remarks section). Always RTFM. https://msdn.microsoft.com/en-us/library/windows/desktop/aa366537(v=vs.85).aspx https://msdn.microsoft.com/en-us/library/windows/desktop/aa3...
- masklinn 9y agoImpressive. It would be difficult for you to be more wrong, and reading the article and noting that > [ex-coworkers at microsoft] confirmed that my fix should mitigate the bug (I’d already noted that it had allowed ~600 clean builds in a row), and promised to create a proper fix in Windows. would have saved you the bother of nerd-sniping. The coherency notes in the article you link are irrelevant because they're about concurrent access. In TFA, the file is created (through mapping), closed, then executed. Sequentially. Also no ReadFile or WriteFile involved (in fact though I don't know how Windows implements it one would expect PE loading to use memory mappings, and thus be covered under coherence guarantees)
- xtrapolate 9y agoSure, you can call your "ex-coworkers at Microsoft" (most of which are irrelevant to debugging/verifying this issue) - or, you can just spend some time reading the documentation, as previously suggested. The coherency issues are your "tread lightly" warning. The docs regarding FlushViewOfFile expand on this specific issue: Flushing a range of a mapped view initiates writing of dirty pages within that range to the disk. Dirty pages are those whose contents have changed since the file view was mapped. The FlushViewOfFile function does not flush the file metadata, and it does not wait to return until the changes are flushed from the underlying hardware disk cache and physically written to disk. To flush all the dirty pages plus the metadata for the file and ensure that they are physically written to disk, call FlushViewOfFile and then call the FlushFileBuffers function. So, after carefully reading this documentation, we can clearly conclude OP's pull-request is actually missing a call to FlushViewOfFile prior to calling FlushFileBuffers.
- zturner 9y ago
- deleted 9y ago[deleted]
- 0xfeba 9y agoI just wish they'd fix this bug with incorrect restored window states. Eg, when monitor layouts changed maximized windows are windowed, but they still think they are maximized. It requires clicking the maximize button twice to fix it. Been there since Windows 98, at least.
- S3raph 9y agoreally love reading bug hunting stories
- rurban 9y agoStill a linker bug to me. Where did it became a kernel bug? Looks like wrong usage of memory-mapped I/O, not like a bug in memory-mapped I/O itself.
- brucedawson 9y agoThe linker did nothing wrong. It wrote to a file using memory mapped I/O. Shortly afterwards the build system tried to run the just-linked file. This is supposed to work. It usually does. Due to this bug it occasionally doesn't. The linkers are getting a compatibility hack (FlushFileBuffers call) but this is not supposed to be necessary, by the file-system contract.