5 ms·
> A call to memset to clear a memory region after you're done with it because a password was stored there can be optimized out because it doesn't change the beh
by overgard 5y ago
> A call to memset to clear a memory region after you're done with it because a password was stored there can be optimized out because it doesn't change the behavior of the program.
Well, that does change the behavior of the program. After all, the behavior of the program was to wipe the memory. The programmer specified it. The compiler is clearly wrong by any reasonable measure. It's removing an intentional, clearly specified instruction. It's just wrong. It's making an "interpretation" of something the programmer intended as a fact, and breaking things in the process. Why does anyone think this is desirable? Any good programmer knows that correctness comes before speed.
I've been programming C++ since 1998 and I didn't even know this till reading this thread. Blaming the user here is counterproductive. Compilers should not make this optimization, plain and simple. memset should set the memory. If you think otherwise, I invite you to take responsibility for all the users data you have placed in harms way.
- overgard 5y agoBtw to the downvoters, explain to me why a function that is clearly intended to create a side effect that is visible outside the process, IE something you cant reason about, is safe to remove? Memory debuggers are not new or exotic tech. Why do you think that is reasonable to remove if a programmer clearly thought it should be done?
- dagenix 5y ago> explain to me why a function that is clearly intended to create a side effect that is visible outside the process This is simply not accurate. memset_s provides that guarantee. memset does not. It would appear that you have been under the impression that it did. However, it did not.
- rurban 5y agoIt does not. Almost all memset_s implementations are broken, just mine not. memset_s just ensures that the unsane compiler will not optimize it away, whilst it should also ensure that is fenced. Otherwise you can still read the memory from the cache. Calling the ordinary memset_s secure is a bug, it's a mere convenience function.
- dagenix 5y agoI don't follow - how do memory fences make a meaningful difference in security?
- overgard 5y agoDoesn't it seem bad that the default that's been taught is wrong? Admittedly I haven't read a C programming book since 1998 because I've been coding that long, but, it strikes me as incredibly fucked that you need a separate memset_s to mean "I'm not fucking around". When I learned C, memset just meant, uh, set this memory, it was pretty straightforward.
- josephcsible 5y ago> the default that's been taught is wrong There's a big difference between you not liking something and it being wrong. You want to make the common case slower for everyone so that you don't have to learn one extra rule for your uncommon case. > it strikes me as incredibly fucked that you need a separate memset_s to mean "I'm not fucking around". When I learned C, memset just meant, uh, set this memory, it was pretty straightforward. I don't think it's ever worked the way you want it to work.
- astrange 5y agoMemory debuggers aren't mentioned in the C spec, and an implementation that let you see all past contents of memory would be valid (and pretty useful), in which case that memset wouldn't be hiding anything.
- kaba0 5y agoSo you are saying that we should throw away every single optimization, and go back to C being a fancy assembler? As if we consider the memory layout at all times inspectable, pretty much any optimization is impossible. Also, the compiler produces code for the CPU not a debugger — here you can’t implicitly view memory at arbitrary places. I personally prefer to write code that is more readable to humans that will get translated to the optimal instructions over having to learn some tricks known only by some elite C hacker that will actually be inferior, since CPUs are finicky. Even with very good understanding of the CPU one can not really guess which instructions are faster to execute without benchmarks. Nonetheless, there are tiny C compilers that do next to no optimizations, feel free to compare the speed of the resulting binaries.
- gpderetta 5y agomemset it is not clearly intended to create a side effect that is visible outside the process. In fact setting memory is not a visible side effect from the C abstract machine point of view. The compiler cannot read minds, if the programmer clearly thought the code should behave in a certain way it should have told the compiler in a language it understand. Volatile would probably be part of that solution.
- overgard 5y agoThat's the thing: we live in the real world. Yes the C abstract machine blah blah blah. Ok, but, I called memset for a reason, and the compiler writer, being also of the real world, should realize maybe I'm deleting sensitive data? And yes I know there's memset_s but it strikes me as bad prioritization if I have to seek out a secondary function to do what I mean.
- gpderetta 5y agohow can the compiler realize you are deleting sensitive data? The compiler is dumb and just applying a bunch of rules, it has no way to infer intent from your code unless you explicitly tell it so. And no, despite the name, memset does not mean set the memory to 0 anymore than int i = 0 means set the memory to 0. BTW, I think memset_s is also problematic: there is nothing preventing a compiler from storing a copy of your data anywhere else. At the very least is likely to be in registers, possibly partially spilled on the stack.
- overgard 5y ago> how can the compiler realize you are deleting sensitive data? It can't, so don't remove my memset. Why is this hard. Compilers are dumb, yes, so don't delete the thing I wrote on purpose. You are right about the definition of memset. It's unreliable. My point is why is that ok with anyone sane. If I clear 128 bytes on a stack frame that's returning, sure, it's not optimal, but the compiler doesn't know why I did it and I surely did it for a reason. The definition of memset is bad.