Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Please restore our registers when you’re done with them

randomascii.wordpress.com

Post reply on HN
Print view View original

91–100 of 142 posts

Re: Please restore our registers when you’re done with them

#91

Post by brucedawson »

von_lohengramm wrote: post #48

I'm sure some C++ lawyer can correct me, but isn't branching off of `m_ptr` (e.g. `CHECK()`ing the value) after `std::move(m_ptr)` technically Unspecified Behavior since `std::move(m_ptr)` leaves `m_ptr` as an Unspecified Value? It would be up to the compiler to define the behavior if they so pleased, but the C++ spec would not require such behavior to be defined at all.

I was wondering if anybody was going to point that out. It did occur to me that the CHECK was not technically valid due to that exact concern, but given that we control the compiler and the C++ library implementation and given that it's just debugging code (albeit debugging code that we ship to users) I'm fine with it.

In other words, I guess you shouldn't oughta do that generally, but I was fine with it being used there, and it did its job.

Re: Please restore our registers when you’re done with them

#92

Post by brucedawson »

rocqua wrote: post #22

To what degree is this possible to check statically? It feels like at least simple breaks of the ABI rules like this can be detected somewhat statically. The author already started with a very simple and incomplete version. In general, I wonder, are there any (many?) static analyzers for assembled binaries.

You could write an arbitrarily complex analyzer to try to find violations but I suspect that the halting problem means that you can never be sure you've found all errors. I think that either crude heuristics (found two bugs!) or UBSan style instrumentation (finds all bugs in code executed under test) is the best set of solutions.

Re: Please restore our registers when you’re done with them

#93

Post by brucedawson »

323 wrote: post #15

Sounds like there should be an option when you write assembly code to tell the compiler "please save/restore any register that I'm modifying in this asm code according to the target you are compiling for"

In fact the fix to the WebRTC bug was to adjust the clobber list, thus telling the compiler to save the registers.

Why the compiler didn't notice that registers were being used that weren't on the clobber list is unclear to me. Your suggestion seems totally reasonable.

https://chromium-review.googlesource.com/c/libyuv/libyuv/+/3...

Re: Please restore our registers when you’re done with them

#94

Post by brucedawson »

bombcar wrote: post #6
gumby wrote: post #4

Earlier quoted context omitted.

Many architectures do have a 0 register because the value is useful. Others have a zero instruction (or both). What would an "actual zero" be -- a literal?

Yeah, something like "mov ax, 0h" but I suppose that is way more memory intensive as you have to load a 0 into memory somewhere and then copy it into the register. It strikes me as somehow the compiler is making assumptions that aren't being enforced by the ... OS? Language? not sure what, but it's assuming functions restore registers used but that isn't enforced by anything. From my (long ago) time there was PUSHA a…

The compiler is making assumptions (which it is supposed to make) but nobody is enforcing the assumptions. The only player who could reasonable enforce the assumptions would be the compiler, in a special checking mode. I am not aware of a compiler that does this. Pity.

Re: Please restore our registers when you’re done with them

#95

Post by brucedawson »

gumby wrote: post #11
bombcar wrote: post #6

Earlier quoted context omitted.

Yeah, something like "mov ax, 0h" but I suppose that is way more memory intensive as you have to load a 0 into memory somewhere and then copy it into the register. It strikes me as somehow the compiler is making assumptions that aren't being enforced by the ... OS? Language? not sure what, but it's assuming functions restore registers used but that isn't enforced by anything. From my (long ago) time there was PUSHA a…

> It strikes me as somehow the compiler is making assumptions that aren't being enforced by the ... OS? Language? One case of this problem was in a handwritten assembly file. The other was a compiler bug. This is a case where the ABI requires that if you use a certain register you must save its previous value and restore it afterwords; the two independent bugs were cases of forgetting to look after a certain register…

Both bugs were programming errors in assembly language files. One was inline assembly that was missing entries from a clobber list, the other was an assembly function that lacked invocations of the macros that were supposed to be used to preserve/restore the registers. There was no compiler bug.

Re: Please restore our registers when you’re done with them

#96

Post by brucedawson »

tialaramex wrote: post #36
brucedawson wrote: post #20

Earlier quoted context omitted.

The webrtc fix was thematically similar in that the programmer declared what registers were trashed and then the compiler knows which registers need to be saved. I'm not sure why the compiler doesn't notice when registers are used without being declared as being trashed - I'm really not an expert at _writing_ assembly language.

It's literally undecidable in principle whether some assembler correctly restores some register R. That's a non-trivial semantic property, Rice's theorem applies. So the compiler's only practical option if it worked this way would be a conservative option - any time it's unclear whether register R is clobbered, treat it as clobbered. As a trivial example of why a register might not be clobbered even though my code to…

It would be easy to have an "auto" clobber list option. The inline assembler would note which registers were touched (mostly trivial, a few instructions have implicit destinations) and would add them all to the clobber list. In about 99% of cases this would be sufficient. I am not aware of any code that conditionally uses a register _and_ conditionally preserves it.

So, 1% of assembly code would use the manual clobber list, but the other 99% would be guaranteed (barring bugs in the compiler) to not have this bug. It seems like the right tradeoff.

Or, instead of an "auto" clobber list the compiler could have a warning if a register is used without being in the clobber list. The programmer could silence that warning in the rare cases where they need to optimize register preservation, and the bugs would be greatly reduced.

Re: Please restore our registers when you’re done with them

#97

Post by skissane »

brucedawson wrote: post #89
skissane wrote: post #85

Earlier quoted context omitted.

On Windows, you can call "EnumProcessModules" to get all the DLLs loaded in the current process – and check those are what you expect. It is potentially a bit fragile though - DLLs can load other DLLs, and it has happened before that in a new Windows version, Microsoft suddenly adds new implementation DLLs which get pulled in by the main system DLLs. One approach might be to look at code-signing on all the DLLs, and…

IIRC we do block DLLs that aren't signed by either Google or Microsoft in some of our processes. In other processes we can't because third-party DLLs are needed for shell extensions (utility processes) or accessibility (browser process). And, as you say, code injection is possible without loading a DLL, and does seem to happen. In this case I don't understand how the state leaked from the file-system filter driver to…

> In this case I don't understand how the state leaked from the file-system filter driver to our process, as it seems to have done.

I assume you made some Windows API call somewhere, which ended up in the filesystem filter driver, which then clobbered the register. And I'm guessing the NT kernel code and Windows DLLs never save/restore the register, because it isn't supposed to be clobbered.

Couldn't one approach be to wrap all Windows API calls with some extra code which saves and restores all the callee-save registers, so even if a buggy kernel driver clobbers them, you don't get hurt by that? I don't know, maybe that's too expensive.

Instead of restoring, one could check for the clobbering, and crash the process immediately. Or maybe Microsoft should add such a wrapping to all calls to third party kernel drivers, and blue screen?

Re: Please restore our registers when you’re done with them

#99

Post by tialaramex »

brucedawson wrote: post #96
tialaramex wrote: post #36

Earlier quoted context omitted.

It's literally undecidable in principle whether some assembler correctly restores some register R. That's a non-trivial semantic property, Rice's theorem applies. So the compiler's only practical option if it worked this way would be a conservative option - any time it's unclear whether register R is clobbered, treat it as clobbered. As a trivial example of why a register might not be clobbered even though my code to…

It would be easy to have an "auto" clobber list option. The inline assembler would note which registers were touched (mostly trivial, a few instructions have implicit destinations) and would add them all to the clobber list. In about 99% of cases this would be sufficient. I am not aware of any code that conditionally uses a register _and_ conditionally preserves it. So, 1% of assembly code would use the manual clobbe…

If your numbers are roughly correct then I agree it's worth trying to do this. My guess was that the assembly we actually use tends to be aggressively hand-optimised to solve very nice problems in the most optimal way and therefore would be more rather than less likely to trip up analysis, but I haven't experimented.

Re: Please restore our registers when you’re done with them

#100

Post by webstrand »

mkj wrote: post #5

It seems that the chrome developers should be able to perform the same binary analysis on the suspect Mcafee software. I guess it's a bit harder without source code to reference side-by-side though.

I wouldn't be surprised if that was a license violation
Post reply on HN