Compiler bug? Linker bug? Windows Kernel bug

https://randomascii.wordpress.com/2018/02/25/compiler-bug-linker-bug-windows-kernel-bug/

by janvdberg • 8 years ago

246 92 8 years ago

randomascii.wordpress.com

mehrdadn11• 8 years ago

Huh. 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.

MaxBarraclough3• 8 years ago

That'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.

mehrdadn11• 8 years ago

I'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.

zuzun2• 8 years 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.

mehrdadn11• 8 years 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.

benmmurphy• 8 years ago

this reminds me how updating libraries in linux can be kind of dangerous depending on how you perform it. for example if you unlink the library then create a new library with the same name then the system behaves probably how you want it to. existing processes use the original library until they are restarted. however, if you open the existing library with O_TRUNC and overwrite the library then existing processes will use the new code which is super dangerous. by default cp will do the O_TRUNC behaviour, and tar will use the unlink behaviour. its interesting to think that stuff might be accidentally working for some people because of this choice in behaviour for tar.

i haven't actually tested this with a real library. this is what i've assumed happens from playing around with mmap(PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE).

leeter2• 8 years ago

IIRC 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.

mehrdadn11• 8 years ago

This 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.

Athas• 8 years ago

Are PE executable files special in Windows, or could this bug have been hit with normal IO?

mehrdadn11• 8 years ago

I'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• 8 years ago

Indeed but being loaded as code seems important as well. It sounds like the two used together, in certain extreme conditions, causes the issue.

blattimwind• 8 years ago

Since 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.

vardump3• 8 years ago

memory-mapped (which isn't normal I/O)

Memory mapped I/O is very much normal I/O. Normal, as in "common" or "usual".

brucedawson5• 8 years ago

Both parts are special. As the post says, there are three conditions: 1) Write the file with memory-mapped I/O 2) Read the file as a PE file (CreateProcess, LoadLibrary, etc., I'm not positive of the exact requirements but it's something special that I believe only PE files can hit) 3) Very heavy load

dingo_bat2• 8 years ago

So basically if you write and read immediately, windows will give you bad data in some cases. Not a good sign for the underlying design.

huhtenberg2• 8 years ago

No, 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_bat2• 8 years ago

I 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.

quietbritishjim3• 8 years ago

How does your comment contradict the parent one?

Const-me• 8 years ago

And also this only happens on multi-socked (not just multi-core) systems. So the underlying problem is likely related to cache coherency issues across NUMA nodes.

cryptonector• 8 years ago

I'm assuming that the linker exits before the program it linked is executed. I'd hope so! If so then this is clearly an OS bug.

mehrdadn11• 8 years ago

You might as well go all the way and simplify it down to "so basically if you run Windows you might get corrupt data" to make your point clearer.

torstenvl• 8 years ago

HN Guidelines: "Please respond to the strongest plausible interpretation of what someone says, not a weaker one that's easier to criticize. Assume good faith."

DiabloD3• 8 years ago

You might as well go further and simplify it down to "if a computer is involved, it will corrupt your data".

CJefferson• 8 years ago

memmove in glibc produces wrong results in some cases (that was a fun over-Christmas debugging session). There is bugs in all software.

jwilk2• 8 years ago

The "Avoid Windows kernel bug using Python hack" commit (for people with JS disabled):

https://github.com/chromium/chromium/commit/052a09014b2018e2...

bla2• 8 years ago

And this looks like the lld-link change: http://llvm.org/viewvc/llvm-project?view=revision&revision=3...

jwilk2• 8 years ago

The llvm commit on GitHub (which shows whole diff at once):

https://github.com/llvm-mirror/llvm/commit/8b2a3e8b203a332bb...

epberry• 8 years ago

I 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.

mehrdadn11• 8 years ago

Are 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.

brucedawson5• 8 years ago

Bugs are prioritized. Data corruption would be higher than some other bugs, but it also depends on frequency

DoofusOfDeath2• 8 years ago

This 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.

chrisper2• 8 years ago

Are 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).

acdha• 8 years ago

I think in reality it's more nuanced than that: the number of people who use computers who can fix a kernel bug is relatively small, and the process of getting a bug accepted, patched, and shipped is not trivial. I would not want to bet on a bug which Linus disagrees with getting patched faster than, say, a report on some company's enterprise support agreement.

(disclaimer: I've been running Linux and other open-source OSes for over two decades. I like OSS but it's not perfect and staffing for projects is an under-solved problems.)

ashelmire• 8 years ago

Think about your code. Now imagine a codebase 1000 times as large written by thousands of engineers with deadlines. There are bugs or potential vulnerabilities in every codebase, but you’ll never encounter the vast majority of them.

pjc50• 8 years ago

I found my own one: Windows "structured exception handling" relies on code in the kernel that unwinds call stacks. This makes a whole bunch of assumptions about the layout of the code as well. On CE/ARM, we found that Visual Studio was sometimes generating code that used "bl [r3]" constructs which could not be unwound. The workaround was to use a different version of cl.exe that came with the CE dev kit and not the one that came with Visual Studio.

Oh, and if you have more than about 512 bytes of local variables in a stack frame and try to use CE remote debugging, that stops working as well.

slededit• 8 years ago

Its the same on X64, although I think its better documented/understood on that platform.

ksk3• 8 years ago

Yes, but do you know of any large software project that only ships when they hit zero bugs? Actually, this would probably extend to all products. Creating a product with no known defects is not possible AFAICT.

wrs• 8 years ago

If 250kLOC is “large”, then the Space Shuttle’s onboard software is one (the only?) example.

https://www.fastcompany.com/28121/they-write-right-stuff

WalterGR• 8 years ago

that OS shipped with 65,000 known defect

Issue tracking at Microsoft (and presumably all large tech companies) includes things like:

* localization errors

* typos in user-visible text

* suggestions for rewording user-visible text

* issues related to icons and other graphics

* issues related to sounds that ship with the product

* feature requests

* feature enhancements

* tracking tickets for work to be done or work in progress

* tickets for unverified, incorrect, and unreproducable bugs

So a. 65,000 tickets, not 65,000 defects. b. "65,000 tickets" is meaningless without knowing more about them.

zombieprocesses• 8 years ago

This is the case with all major software releases. The bug tracker is filled with bugs for any major release. It's just a matter of team lead, project manager and higher ups deciding what the priorities are. Not every feature makes it and not every bug gets fixed.

The key is to hit the milestones and get the product working as much as possible.

Enterprise Software Motto: "Sometimes good enough is good enough". Amen.

xtrapolate2• 8 years ago

Answer 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/aa3...

masklinn• 8 years ago

Impressive. 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)

xtrapolate2• 8 years ago

Sure, 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.

0xfeba• 8 years ago

I 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• 8 years ago

really love reading bug hunting stories

rurban• 8 years ago

Still 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.

brucedawson5• 8 years ago

The 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.

Compiler bug? Linker bug? Windows Kernel bug

Related Stories