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.

That would be possible, but we'd have to install the software, then guess which binary was the culprit, and then have some way of finding the function boundaries. My crude analysis technique required on having symbols for chrome.dll to indicate where functions started, so I'd have to have switched tools to something else that could find those.

Most disassemblers are fairly decent at guessing function boundaries if the code isn't too bad.

I wouldn't be surprised if that was a license violation

One of the linked pull requests: https://github.com/cisco/openh264/commit/db956674bbdfbaab5ac...

Macros in a macro assembler .. not very nice.

He even mentions that it's Windows-specific, both in the commit message and in the article... and then seemingly fails to make it Windows-only? That's "not very nice" either.

Those PUSH_XMM/POP_XMM macros appear to be Windows-only; I think they expand to nothing on other platforms because they contain their own guard for Windows internally. If that's the case, the call sites don't need to guard for it. I'm guessing that obeying this calling convention is the purpose of those macros.

https://github.com/cisco/openh264/blob/db956674bbdfbaab5acdd...

https://github.com/cisco/openh264/blob/db956674bbdfbaab5acdd...

Exactly. My understanding of the conventions and macros in those source files is that you declare what registers you will be trashing, and then the registers are saved/restored as required by that platform. On Linux it would be a NOP, and on Windows it saves and restores XMM6 and XMM7 (XMM0-XMM5 are volatile).

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.

Traditionally, an assembler neither knows nor cares about such information, it just turns lines of assembly into bytes and does some address fixup for you.

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 touched it and it seems like I didn't restore it...

Suppose if R is divisible by 12 I branch, in the other branch I don't change R, but in that branch I do change R, XORing it with a value which is difficult to explain but has a value between 1 and 3 inclusive, sometimes more than once. At the end of the branch I also clear the bottom two bits of R.

R is actually not clobbered by this function! If the bottom two bits weren't zero before, R isn't divisible by 12, so we didn't change R, and if they were zero, we restore that, the other bits are never changed.

Having the human programmer promise they they wrote a correct clobber list means if their assembler does somehow restore/ preserve register R, the human can just say so, and needn't prove to the compiler somehow that this works. This sort of code is mostly in performance critical components, e.g. video decoding, where we are already trading reliance on fallible humans for better performance, so adding one extra promise feels OK.

> It's literally undecidable in principle whether some assembler correctly restores some register R.

No, it's literally undecidable in principle whether every bit of assembler correctly restores some register R. For any given bit of inline assembler, it's quite likely to be trivial.

In any case, we can have a useful safety feature without requiring the compiler to decide. The compiler can easily work out all the registers which get written to (right?), just not which get restored. So in addition to the clobber list, we could have a list of registers which the programmer asserts that the code restores. A register which is written to has to be on either the clobber list or the restore list (or be an output). This certainly isn't foolproof, but it would catch accidental clobbers.

> No, it's literally undecidable in principle whether every bit of assembler correctly restores some register R. For any given bit of inline assembler, it's quite likely to be trivial.

Exactly. And if it's non-trivial to decide something that basic, you're doing it wrong. Saving and restoring all the registers is always an option. Only saving and restoring some of them is an optimization which must be shown to be sound.

> every bit of assembler correctly restores some register R.

Thanks, I was trying to think about the correct way to express this but clearly I didn't do the best job.

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.

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.

My experience is that most non-trivial assembly language is doing processing of large chunks of data (high-precision math, encrypting blocks of data, FFTs, etc.) and therefore the startup cost of the function is not significant, so a tiny bit of inefficiency in that area is not something people care about.

Or, put another way, writing tiny little assembly language functions is probably not worth it because the mere fact that you are using assembly language instead of (say) C/C++ means that you have missed many opportunities (code reordering, inlining, etc.) so assembly language functions _should_ be doing enough work to justify their calling cost.

But, I'm not working on an assembler or even using one so I don't think I'll even file a feature request.

How does this add to the conversation?

looks like someone spamming links to all their personal projects over HN

We ban accounts that post like this, because the community considers it spamming.

I'm not going to ban you because you've also posted other things to HN and seem like a legit user. But you've been posting these links much too often, so please stop doing that.

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"

I not sure whether that satire. In case its not, most languages that allow inline assembly (like C) have an optional "clobber list" argument that tells the dataflow analysis of the compiler that your assembly snippet overwrites certain registers [1]. Inline assembly doesn't have target specific clobber lists because it's assumed that the code only works on one target and the programmer has to take care of making it work.

1: https://www.ibiblio.org/gferg/ldp/GCC-Inline-Assembly-HOWTO....

The System V x64 ABI is different from the Windows x64 ABI.

THat is essentially how inline assembly in GCC works. You have to declare which registers you are changing and in which registers you expect input and which contain results from your inline assembly block.

This for obvious reasons does not work if you have separate compilation unit written in assembly, then you have to follow the ABI.

Maybe we need some kind of higher level language that gets boiled down into assembly.

And, a compiler that knows custom SIMD optimizations for every algorithm anyone might ever need, and can recognize when you have coded one of them so it can substitute its SIMD version.

C does have official intrinsics for SIMD on Intel platforms, but the people who are good at writing video codecs don't like to use them because Wintel culture has such bad taste at naming functions (thanks to Hungarian notation) that using them is near-unreadable and it's easier to write everything in asm.

You're wrong. Intrinsics for SIMD are not named by Windows people. They were named by Intel engineers and supported now by all other compilers. Naming is actually in the C/C++ style - __simd_do_something_here() and such

I am not wrong and I'm not talking about those.

The Intel platform intrinsics have names like `_mm512_4dpwssd_epi32()`. The standardized SIMD intrinsics with `simd` in the name are much newer than any of the code I'm talking about in ffmpeg/x264/dav1d. These are okay, but not being platform-specific of course means you don't get platform-specific features, which you might want when you're doing this level of optimization.

The other problem is compilers (esp. gcc) were traditionally very bad at code generation for them, although these days they're okay at it.

That kind of feature is very .. un-assembler. You certainly could, but the assembler doesn't keep track of any of the relevant information, so it would be a larger feature than you expect. It doesn't know the calling convention. It doesn't have a map of registers dirtied by which instructions. It doesn't do reachability analysis, so it doesn't even necessarily know what's "in the function".

Doesn't GCC inline assembly actually track that info, with the "clobber" section?

Yes, but it's not a property of the assembly (or assembler), it's a necessity for the compiler to correctly codegen around the inline assembly.

Historically, assemblers have been really dumb, so ABI is not a thing they'd track, especially as... I don't think they know what functions are? So while they can notice call/ret, they have no knowledge of a label being a jump or call target per-se, do they?

So you'd need an assembly-like language to encode this sort of information.

I mean if you have a macro assembler you should be able to write a macro that generates "save registers" instructions before a section of code and "restore registers" instructions after it. The assembler doesn't need to know our care that this section of code is a function.

It's interesting how blurry the line is between a good, full-featured assembler and a crappy compiler!

> I mean if you have a macro assembler you should be able to write a macro that generates "save registers" instructions before a section of code and "restore registers" instructions after it.

Except you want a "macro" which:

- saves only the registers you touched

- which are callee-saved

- according to the ABI you're targeting

And you really only want that for functions, because... that's where ABIs come into play.

> I don't think they know what functions are?

It's a long time since I tangled with x86 assembler; but as I recall, ENTER and LEAVE were specifically for functions, and I'm not aware of any other use for them.

ENTER and LEAVE were specific x86 instructions though, not some kind of assembler special sauce. The assembler just translated your ENTER instruction into the corresponding machine opcode; no ABI knowledge required whatsoever.

I think most assemblers know what functions are. There are usually directives to indicate this. They help with emitting symbols (you need the function name to be emitted or no-one can call it), stack unwind information, and other information (some of which may also be required by the ABI). Here's some relevant documentation for MASM https://learn.microsoft.com/en-us/cpp/assembler/masm/proc?vi...

The short version of it is that assemblers these days are rarely - if ever - just a zero-context stream of machine instructions. There is far more, some of it actually required.

When you use inline assembly, in fact the compiler does preserve the semantics of the surrounding program considering the target's ABI -- if you tell it correctly what impacts the inline asm has. Lots of rules must be followed in order to get this behavior just right. One of the most subtle ones is "early clobbers" [1].

In many cases, you can get all the benefits of inline assembly from compiler intrinsics while letting the compiler handle all the details of register allocation and scheduling.

Note that in OP's case IIUC this was assembly code and not inline assembly. If you write functions in assembly you are solely responsible for calling conventions and ABI conformance.

[1] https://stackoverflow.com/a/15819941/489590

You could totally design a better convention in your own high-level language which compiled down to assembly, as long as you stayed inside it. But once you need to interface with external code you need to use a shared convention.

Your scheme sounds like it would make your code well-behaved as the callee (with perhaps some performance penalty?). But as a caller, you couldn't trust the external code not to clobber registers.

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

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.

Compilers do inter-procedural register allocation and use custom calling conventions for local calls (where “local” can be quite large with LTO), while preserving ABI externally. This means that clobbering a callee-saved register without saving/restoring it in the same function is not necessarily a bug.

Curiously, I found a register clobber bug in the NaCl cryptography library today. Apparently, they used a custom assembler-preprocessor (qhasm) that avoids certain classes of bugs and aids with porting, but while the tool seems to actually model the register in some way, it does not treat it as callee-saved.

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.

Ugh, I had a similar issue when I forgot to clobber ecx when using inline assembly for rdtscp. Very annoying.

Why does a „3rd party encryption software” appear in the call stack of a user mode process in the first place? Is this another case where a “security” software injects broken DLL files into all processes in your system?

Yes. That's pretty common, unfortunately.

I'd love to see Chrome and Firefox and IE working together to detect this kind of thing and put up a malware warning explaining how to uninstall it, and see how quickly it can be eliminated.

Seriously, there is zero valid reason to ever inject code into another program, other than as a debugging tool on a system being debugged.

There might be a few more. Invasive Anti cheat software seems necessary for competitive multiplayer games on PC.

DLL injection is also used for in-game overlays by Steam/Discord/etc, and I'm not aware of a better method they could use instead given that they're expected to work for games that were never made with those services in mind.

GL/Vulkan extensions for drawing overlays over the current frame. Compositor/Windows extensions for drawing overlays, the way Android has. You don't need to be running inside the current process to draw on the screen.

For the Steam overlay they also need to intercept input (while the overlay is open).

They use wayland compositor (gamescope) under linux (even under X11), in their new SteamOS UI and it's already working a lot better. Plus they can do other awesome stuff with it like completely having control of where and how the game outputs its buffer. So it can fix games with bad multi-monitor behaviour, force game to render in their native aspect ratio / resolution etc... it's just great and makes gaming experience feel more like a console where things (usually) just work.

Gamescope + mangohud + steam overlay working together is very big part of SteamOS and Steam Deck's success, and it does zero runtime modification or hooking into the child process. I can imagine all the trouble you run with the hooking eventually and all the hacks it must have piled up.

They use gamescope under SteamOS, not Linux in general where you already have another compositor. Running all games under a nested compositor (gamescope) would have some perf impact (even if a small one) as well as an unexpected cursor acceleration profile.

True. I personally have setup my distro to launch directly into gamescope optionally for SteamOS like experience. On desktop client they still either hook or use vulkan layer (latter often in proton, thanks to dxvk). However I would not be surprised if they started using nested compositor even on desktop.

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 ignore any unknown DLLs signed by Microsoft.

Would probably make sense to open a scary red warning at startup, telling users that their browser contains untrusted third party code, and stability/security cannot be guaranteed. Probably need to make sure there is no easy way to disable it, or else some IT will just disable it as part of installing DLL-injecting "security" software.

There are ways of doing hidden code injection, without the DLL coming up in the loaded DLLs list – https://reverseengineering.stackexchange.com/questions/2262/... – one option is for the loaded DLL to duplicate itself in memory, jump to the duplicate, then unload the original. Or, you can use VirtualAlloc2/WriteProcessMemory to load code into another process, and CreateRemoteThread to launch it.

I thought these "security" software vendors wouldn't be doing anything so fancy: but from the Chrome bug [0] it looks like some are:

> And I was able to confirm (in some of the dumps, we don't collect the right heap information in all dumps) that Trend Micro code (one region is a DLL that seems to be called ApiHookStub.x64.dll, another is not a direct DLL copy) which has been allocated on our process heap without going through the loader, presumably via something like ::VirtualProtectEx and ::WriteProcessMemory. This is a pattern I see used broadly in Edge crashes we root cause to third-party software.

However, that still can be detected – use VirtualQueryEx to iterate through process address space and find all executable memory regions – any not owned by a loaded DLL (or generated by JavaScript JIT/etc) are evidence of code injection, even if you don't know who the injector is.

[0] https://bugs.chromium.org/p/chromium/issues/detail?id=121838...

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 our process, as it seems to have done. It's a mystery.

I have plans to do the address space iteration you speak of in our crash reporter.

> 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?

Those tactics could work. They would be a bit expensive however, and it would be a shame to have all users paying the performance penalty because of a few bad pieces of software. And, this would not have caught the two errors in the assembly language code within Chrome - that would require testing at every function call, not just Windows API calls. It would be more practical (I think) to do this checking on a special build of Chrome that is shipped to a small percentage of users, so that not everybody pays the price. But, this is an ecosystem problem and I'm not sure Chrome wants to shoulder the entire burden of finding bad software :-)

> They would be a bit expensive however

It would be interesting to see measurements of how big the expense is.

Also, I don't think one necessarily has to do it for every Windows API call – some Windows API calls are more likely to invoke third-party code than others; some API calls are far more performance-critical than others. Maybe one could find a subset of calls to focus on which maximise the likelihood of invoking third-party code but also minimise the performance impact.

> And, this would not have caught the two errors in the assembly language code within Chrome - that would require testing at every function call, not just Windows API calls

For code you control, I think some kind of static analysis would be a better approach – parse inline assembly code and check that every register it touches is marked as clobbered to the compiler. I saw some other comments you were replying to already on that topic. I think this kind of "dynamic" approach should be reserved for third-party code with low trustworthiness.

> It would be more practical (I think) to do this checking on a special build of Chrome that is shipped to a small percentage of users, so that not everybody pays the price.

I was thinking, you could also do it using API hooking. Have some hidden setting to control it, by default off. If it is off, no impact, same as now. If the flag is on, hook (some subset of) Windows APIs with the "unexpected-register-clobber-detector". That way you don't have to produce two completely different builds.

And maybe even, automatically turn that flag on if an install starts to experience crashes–especially if the presence of certain kinds of third-party software is detected.

> But, this is an ecosystem problem and I'm not sure Chrome wants to shoulder the entire burden of finding bad software :-)

Agree. Ideally, Microsoft would take the lead there, since it is their platform. But a world in which the Chrome team does it would be better than a world in which nobody does.

What's more likely to happen is that IT tells people to ignore the scary warning, and scary warnings become less effective.

The modding communities for Bethesda games rely heavily on code injection, especially the script extender plugins that are central to the entire enterprise.

Yeah, but there's ZERO reasons why you should be allowed to modify software to suit you. ZERO. Security über alles, DRM control over all, how dare you modify the software God Developer has given you, it will make you Insecure.

What about AutoHotkey, reWASD, ConEmu, JoyToKey?

Injecting keyboard events does not require running inside the current process. Either emulate a keyboard device or inject events via the APIs for that.

Doesn't Windows do this all the time as part of compatibility shimming?

> Seriously, there is zero valid reason to ever inject code into another program

Sometimes, I use software which does not work the way I would like it to work. When this software is closed source, and the problem sufficiently annoying, I inject code to make it do what I want.

(And no, once I do this I don't open bug reports, unless I can reproduce the problem without code injection.)

Because that's the great thing about owning a computer, and knowing how to really use it. It's a tool for you to command.

Now, will most people ever do this? No. I wish everyone could, but unfortunately, injecting code in a useful way requires a reasonable amount of coding knowledge.

But, this is Hacker News. I would hope most users here can come up with lots of valid reasons to inject code into other programs.

There's a huge difference between "I, the user of this system, am intentionally injecting code into a process to debug/extend it in a way I want, and if it breaks I'll have a pretty good idea that it might be my injected code's fault and know where to start looking" and "software that I may not even have intentionally installed on this system or understand the function of has broken other software on this system".

I agree, but restrictions on one almost invariably restrict the other as well.

The irony of security and anti-cheat software acting like a invasive malware

I used to take point for Mozilla's efforts dealing with third-party interference in our binaries. Browsers are ripe targets for this kind of shit. The stories we could tell...

And from our perspective you should! Would be interesting.

I did write a little bit! Here's an oldie but a goodie:

https://dblohm7.ca/blog/2016/01/11/bugs-from-hell-injected-t...

Please write some stuff about this

I did write a little bit! Here's an oldie but a goodie:

https://dblohm7.ca/blog/2016/01/11/bugs-from-hell-injected-t...

Naïve question about ABIs: shouldn’t the caller be responsible for that? If I want a function to restore certain registers, wouldn’t it be simpler if I was the one that save them on my memory, call the function, and then override the registers with whatever values the function set? Otherwise it seems we’re just… asking for trouble, so to speak.

It would just be slow for the caller to have to push and pop (say 30) registers in general that the specific callee (and transitive callees) may not even use.

Most ABI specify some registers caller-saved, some registers callee-saved (retained unchanged from the perspective of the caller) and some registers scratch (not-saved).

In the end it depends on the architecture and on typical workload which are the fastest--and measurements can be made and it can be found out which combination is the fastest on average.

You usually wouldn't need to push&pop all registers, just ones that you want to preserve across the call. Regardless, yeah, non-volatile aka callee-saved registers are extremely important for good performance of code that calls functions (esp. loops - without callee-saved registers, you'd have to store the loop counter & length on the stack!)

...which is exactly what the callee within the loop would have to do if it used those registers. But I guess your point is that in the callee-save case it only happens when it needs to happen, while in the caller-save case it happens every time whether it needs to or not.

yep, hence why calling conventions usually have both callee-saved and caller-saved registers, so that you only have "unnecessary" stack usage when you need to use more than roughly half of the registers.

Programming defensively in this way is possible but has a huge cost (extra saves and restores). The caller is supposed to be able to trust that certain registers persist across the call. If it can't, it has to save everything to memory before the call and restore it. I suppose a compiler could add a special annotation for "this is an assembler routine and I don't trust that they know the rules", which would generate extra saves and restores, but presumably the routine was coded in assembly for extra speed, so 6 to 8 extra saves and restores would cancel that out.

If someone is trying to use the same assembler routine on Windows as on Linux, it's likely to be wrong for one of them.

> If I want a function to restore certain registers, wouldn’t it be simpler if I was the one that save them on my memory, call the function, and then override the registers with whatever values the function set?

That's a waste if the callee doesn't use them.

Indeed. But isn’t it also a waste when someone doesn’t follow through and then we have situations like the one described on the article?

Such cases are rare bugs in a small amount of code — mostly just compilers and hand-written assembly.

Preemptively saving and restoring all registers in all callers would appreciably slow down every single function call on every device in the world using that ABI.

The cumulative cost would be astronomical.

ABIs are a convention that everybody follows regarding how such preservation should or should not occur, for which registers.

It's common to say "caller cleans up registers x, y, z, callee cleans up a, b, c".

So yes, the caller could do it or not do it, the choices are all possible to do both ways, but that wouldn't be the agreed upon ABI, it'd be something different.

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.

There is a big difference between "undefined" and "unspecified" behavior. In this case, the behavior of `unique_ptr(unique_ptr&&)` is in fact specified. [0]

However, the bigger issue with that code is that it can easily stop working with a simple refactor. Consider:

    void foo(std::unique_ptr<int> ptr) {}
    void bar(std::unique_ptr<int>&& ptr) {}

    int main()
    {
        std::unique_ptr<int> p1{new int{1}};
        std::unique_ptr<int> p2{new int{2}};

        foo(std::move(p1));
        assert(p1 == nullptr);

        bar(std::move(p2));
        assert(p2 != nullptr);
     }

Neither of the above asserts will fire, but from the calling site, they look exactly the same. In my opinion, the more explicit option would be to do something like `bar(std::exchange(p2, nullptr))`

[0]: overload (5) https://en.cppreference.com/w/cpp/memory/unique_ptr/unique_p...

Right, but changes in some other code potentially silently breaking assumptions at the calling site is exactly why you would want an assert like this in the first place.

Moves in C++ don't make the whole object invalid, it just leaves them in a valid but unspecified state, so that at least the destructor can still run.

This means calling operator bool on your unique_ptr ought to be fine, because the unique_ptr still has a valid state (you don't know what that state is, it's unspecified, but it's guaranteed to not be radioactive on mere contact. It has to be a valid unspecified state.)

Moves in C++ are a function call that can do whatever any other function call can. The type itself can have additional guarantees and the standard library types generally do. In the case of std::unique_ptr the guarantee is explicit that the moved-from unique_ptr is nulled.

It’s an unspecified but required to be valid value for the moved type. The author mentions it’s a smart pointer type, which could easily be defined to act like this.

It appears that you need to be really smart in order to not blow your own foot off with this "smart pointer." I'll stick to the regular dumb kind, thanks.

Seriously, before downvoting, read the thread. When you find yourselves arguing endlessly about the distinction between unintended behavior, undefined behavior, and incorrect behavior in something as fundamental as a pointer, something has gone very wrong at a much earlier stage of the conversation.

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.

It appears that the question of what is valid on a moved-from object is tricky. Here is one discussion: https://stackoverflow.com/questions/7027523/what-can-i-do-wi... FWIW, here is the move operator for the type of the object in question: https://source.chromium.org/chromium/chromium/src/+/main:bas...

Note that the linked discussion is about the standard library. For your own types you can make whatever guarantees you want - ultimately as far as the language is concerned, moves are a function call like any other.

If one could reliably detect this DLL injection in the process address space, then the correct "fix" is to crash immediately. Authors of such tools should seek another way of accomplishing their goal, preferably one that does not export their own bugs to innocent bystanders.

In this particular example, why not just

  PXOR XMM7 XMM7

before using XMM7 to zero anything? That way the compiler doesn't have to assume that XMM7 is zero, it can know.

Yes it's an extra instruction but XORing a register with itself is such a common metaphor for zeroing that register that CPU designers try to make it fast.

Edit: Just noticed that Veliladon essentially made the same comment herein and explained the reason why it's not done this way.

It's not just that. If you can't trust the ABI then everything else is wrong too. Everything. Not just zeroing registers. It only just so happens that in this case XMM7 is used for zeroing, but it could be used to save a variable across a function call. Then there is no trick to get it set back to the right value.

The ABI is not optional, or best effort, or best practice, or any other BS that passes in the ordinary world. It is just as required as the correct operation of instructions (e.g., add should actually add things, mul should actually multiply them, and so on).

My reading of it was that the bug was caused by XMM7 not being restored to its previous value.

E.g. on Linux, functions should restore the values of ebx, esi, edi, ... once they're done with them. The article says (on Windows) that XMM7 needs to be restored too.

If you can't trust one register being preserved (per the ABI), then you really can't trust the values of any registers.

Fair point, which is why I used the phrase "this particular example." In fact the compiler's lazy assumption about XMM7 was the key to determining that the ABI was being violated. It would have taken longer to figure this out if the compiler had been doing things my way, because then a failure of the callee to preserve XMM7 wouldn't have mattered.

You could avoid the crash in this specific case. But this crash was triggered by an assertion failure. The same code could be corrupting XMM7 in other situations, which could result in much more subtle bugs.

What's the point of having an ABI if you can't assume it's being used?

If you don't want to trust any callees, you could use an ABI with all registers caller saves, but I don't think any mainstream ABIs are like that.

There's a balance where having some registers be caller saved and others callee saved means a lot less saving required.

I think the real fix is to set all registers to a canary value at the start of int main(), and then when exiting check the canary value is still there.

If anything has messed with any registers without permission, you crash and collect as much data as possible about any injected dll's.

Then you correlate these to find the culprits, and for each you contact the authors of the DLL and figure out a way to block the injection of any unfixed versions that cause crashes.

Code can be injected anywhere. You'd have to add checks to every single function.

Any good function upstream of the bad one that also uses the same register will mask the issue

Each runtime function call would probably need its own unique value, perhaps stashed to memory.

McAfee/Trellix says that they have fixed the bug in the latest (7.4.0) version of their disk encryption product. So apparently my guesses about the root cause were correct.