12 ms·
Does that also zero the padding that may exist between elements of the structure (or at its end)? Because if not, not memset'ing will open you up to all kinds o
by anyfoo 8y ago
Does that also zero the padding that may exist between elements of the structure (or at its end)? Because if not, not memset'ing will open you up to all kinds of info leaks, especially if you plan to send that struct over the network (or to another process, or from kernel to userspace...).
EDIT: Actually I have to apologize, because I’m not certain what the C standard says about whether the padding bytes can change when changing members of the structure (6.2.6.1):
6 When a value is stored in an object of structure or union type, including in a member object, the bytes of the object representation that correspond to any padding bytes take unspecified values. 51)
If that means that the padding can actually also change when just assigning individual members (which I think it does, but footnote 51 only calls out copying the whole struct by assignment, and I’m not 100% sure that “including a member object” means what I think in this context), then that means that copying an unpacked structure across information boundaries is technically always unsafe, because any change of a value in the struct might leak information by changing the padding to undefined values. It’s safe with explicitly packed structs, though (which should be the majority of the cases, as you’d have a representation independent of the architecture), but then so is what the author describes. I’d appreciate if someone could clear this up! memset’ing can still help you getting a structure quickly into a well-defined state, though.
- Someone 8y agoSending structures that include padding or that you don’t know the exact layout of over the network is another thing you shouldn’t do, period. Code doing that may break with a compiler change or a compilation flag change on one end of the connection, even if you can guarantee both ends use the same CPU.
- mehrdadn 8y ago> Sending structures that include padding or that you don’t know the exact layout of over the network is another thing you shouldn’t do, period. What if you do know the exact layout and it happens to have a gap between fields?
- noobermin 8y agoTbh I'm not a network person, but my naive brain says instead of casting to char* and writing to the buffer, couldn't you just write out individual fields to a buffer so you control every byte? That seems smarter and safer.
- mehrdadn 8y agoDo you write a separate serialization function for every single struct and sub-struct you send over a network? And then continue maintaining every single one to keep it consistent whenever a field is added/removed/modified in any struct? (Though I should mention that macros actually do make this doable, but they're hardly pretty, and I have yet to see many people embrace this approach, if they realize it's possible at all.)
- kllrnohj 8y ago> Do you write a separate serialization function for every single struct and sub-struct you send over a network? And then continue maintaining every single one to keep it consistent whenever a field is added/removed/modified in any struct? Yes? Of course you do that? Or pick one of a hundred libraries that do that for you - protobuf, cap'n proto, flatbuffers, etc...
- repsilat 8y agoThat's the protobuf approach, but things like Capnp are different, right? Isn't its wire format its in-memory format? I thought it was closer to the "straight memcpy" approach. Protobuf serialization functions can turn into a lot of generated code too (read: big binaries) and stomp all over your icache, because they're different for each type. More "data-driven" approaches using type-generic functions with some runtime type-specific data (like proto descriptors&reflection) can be a good tradeoff for code that isn't as "hot". Typically more branchy than a memcpy or a type-specific function though.
- 8y ago
- CJefferson 8y agoChanging compiler or flags won't change structs, unless you do something crazy. If it did, you couldn't link C libraries compiled with one compiler with programs compiled on another. You can't be sure you can send such structures to different OSes or CPUs however.
- spc476 8y agoIt depends upon the architecture and compiler. One C compiler for the MC68000 might have int as 16 bits (since externally, the MC68000 had a 16-bit external bus) but another one have ints as 32 bits (since the MC68000 can handle 32-bits internally). This was more of an issue during the 80s and 90s when you had more commercial C compiler available than today.
- Gibbon1 8y agoThat is what stdint is for.
- saagarjha 8y ago> If it did, you couldn't link C libraries compiled with one compiler with programs compiled on another. You cannot do this unless the compilers have the same ABI, which is not part of the standard.
- anyfoo 8y agoThere is an attribute that lets you specify alignment per-struct, which means you will know the exact layout.
- hermitdev 8y agoBut it is still potentially problematic if not all of your clients have the same endianess. Memcpy and exact layout still fails you there.
- v_lisivka 8y agoRDMA (Infiniband) does DMA to/from network. Why we should not use RDMA?
- catern 8y agoHmm, that concern is not just limited to RDMA; any kind of shared memory communication across a trust boundary is vulnerable to info leaks in the padding of the struct. That does seem like a real issue. Not sure what's the appropriate mitigation. Having to memset is a drag, and it's easy to forget to do it in some place or another. I'm tempted to say that you should only use packed structs for shared memory communication across trust boundaries.
- gmueckl 8y agoWell, Infiniband is intended mainly for supercomputers. So security is of no real concern within the Imfiniband network as that is implicitly a trusted environment. Access comtrol to that network is the main security boundary. Edit: Also, lots of simulation codes I am aware of tend to send flat arrays anyway. So I'd say that your concerns are mostly theoretical in nature.
- shaklee3 8y agoRDMA, both roce and infiniband have some significant limitations, specifically at higher rates. But they're the best we have right now for direct dma.
- 0x8BADF00D 8y agoSerialization is a non-trivial problem. Especially when dealing with different OS, network stacks, and architectures. protobuf alleviates this quite a bit.
- johncolanduoni 8y agoThis kind of issue happens all the time with structs that a kernel, driver etc. expose to userspace: https://j00ru.vexillium.org/papers/2018/bochspwn_reloaded.pdf https://j00ru.vexillium.org/papers/2018/bochspwn_reloaded.pd... Not sending the structure over the network is insufficient, and rigorous memset is a clear solution to this problem. Also, C structure layout for a specific ABI is strictly determined; if that weren’t the case you couldn’t compile two C compilation units with different flags/compilers and expect them to work.
- blattimwind 8y agoAlso don't expect memset() to actually be a library call at all.
- rurban 8y agoAnd most importantly dont expect memset to be executed at all. Most -O2 compilers happily optimize the memset away if if doesn't care about the sideeffects. Therefore there exists the "secure" memset variants, which are the insecure variants adding a simple compiler barrier, so it isn't optimized away. And there exists the very few really secure memset_s variants with a real memory barrier which guarantees memset to be executed in load and store order, flushing the cache, so that Spectre attacks are mitigated. security-relevant crypto libraries, like openssl and variants don't care about that, because memory barriers are "too slow". only the Linux kernel does it properly. It's still a mess.
- FrozenVoid 8y agoyou can write your own memset that bypasses all of that. Its just 2 lines of code.
- gmueckl 8y agoAnd that will be slower than calling memset() unless you have a really smart compiler.
- 8y ago
- bjourne 8y agoNo, but the follow up article is called Stop Memcpying Structures. And the follow up to that Stop Memcmping Structures. :)
- mehrdadn 8y agoDid you mean this as a joke? I can't find a follow-up article (this one was written today).
- anyfoo 8y agoYou're probably not serious, but why waste time and complexity copying structure elements into a buffer (which likely needs to be memset/bzero'd!), if you can just directly use the struct, all consistent with the standard? C is not a very high-level language. Apart from where undefined behavior is called out, you can and should assume how data is represented on a low level. It was made for writing operating systems and their kernels after all, where interfacing in a bit-accurate way is common.
- bjourne 8y agoI'm joking because endless flame wars have already been spent debating this issue. But I realize not everyone has heard them yet, so let's strap on the football shoes and bring out the horse carcass one more time! There are platforms on which NULL, integer 0 and floating point 0 is not the same. Hence memsetting won't do the right thing. On modern platforms they are, but still, you don't want to get the Standard C Weenies on your back - they're even more annoying than the Rust Evangelism Strike Force. You don't want demons coming out of your nose, do you? :) They do have a point, though. Suppose you are handling a struct you thought were memsetted but was not (your stupid team mate used those damn initializers!), then when serializing it you will get undefined bytes in the stream, possibly triggering hash values to mismatch causing mysterious rebuilds and other stuff. Wrt to field accesses, yes those can overwrite padding bytes. Suppose a is an 8 bit char field, but padded to 4 bytes. Then "foo->a = 22" could be translated to "MOV [RSP+16], EAX" which overwrites the 3 padding bytes with undefined values. The solution, as you mention, is to use packed structs but in those field accesses are much less efficient. Bottom line: code whichever way you want. :) Me, I like to live in the fast lane from time to time and those memset/cpy/cmp functions are sooo handy. :)
- mehrdadn 8y agoNope it doesn't: //GCC: -O0 //MSVC: /Od #include <memory.h> #include <stdio.h> struct S { unsigned char x; int y; }; int f(int f) { S a; memset(&a, f, sizeof(a)); unsigned char y[2]; memcpy(&y, &a, sizeof(y)); return y[1]; } int g() { S a = { 1, 2 }; unsigned char y[2]; memcpy(&y, &a, sizeof(y)); return y[1]; } int main(int argc, char *argv[]) { f(0xDD); printf("%#x\n", g()); f(0xFF); printf("%#x\n", g()); } It definitely seems like the author didn't realize this fact, or I'd have expected it to be addressed in the article.
- unmole 8y agoI am aware of the fact that unitilized padding can have nasty side effects: https://lwn.net/Articles/417989/ https://lwn.net/Articles/417989/ But yeah, I should have mentioned the caveats too. I will update the post once I'm back at my computer.
- mehrdadn 8y agoI'm confused what I'm supposed to look for in that article (do you mean you were the author?) but anyway -- I feel like calling this merely a "caveat" gives the wrong impression? It seems a bit like telling people to mix bleach with vinegar and then saying "Oops, sorry, I did know that it produces chlorine! I forgot to mention that caveat." The caveat isn't just a side note for the margins, it's a critical reason why people avoid this sort of thing.
- Dylan16807 8y agoTechnically I don't think memset forces the compiler to clear the padding and keep it clear either though. The magic of the C abstract machine.
- mehrdadn 8y agoWhy? memset followed by memcpy must return the same byte everywhere. Otherwise what would memset even mean?
- wbl 8y agoPlease explain how you send a struct over the network without UB and leaking the padding.
- anyfoo 8y agoBy knowing its exact size with sizeof(), and using that and a cast to uint8_t* to both bzero() it out before use and memcpy()ing it when sending it. Unless the C standard says that the compiler may change the padding bytes of the existing storage?
- hedora 8y agoTo be clear, it is something like this, which is not bad at all to write: struct st s; bzero(&s, sizeof(s)); // ... write(fd, &s, sizeof(s)); I’ve dealt with many systems that have tons of unnecessary serialization logic, and found them slower and buggier than just letting the C spec define the data format. Having said that, it is possible to get this sort of thing right with C++ and templates (thanks to inlining). Higher-level languages without compile time code generation (like Java and Python) usually make serialization a train wreck in comparison with “send raw structs” or “make the [C++] compiler generate and typecheck the machine code that does the serialization”
- kabdib 8y agoThere's enough wiggle room in the standard that two compilers would be happy making two compliant but still different physical layouts for the same struct declaration. I'm not even worrying about cross-platform issues or bitfields yet. You can get away with directly writing a struct for simple structs, but eventually it will bite you.
- hedora 8y agoPacked structs are reasonably standard (even gcc and visual studio can be made to pack in the same way using the same syntax these days). The remaining issue is endianness, but you can configure ARM to match x86 at boot so shrug. Also, if I tell any reasonably experienced developer my protocol sends a packed struct with these fields: uint8_t version uint16_t reserved uint8_t op int32_t status uint32_t data_length // void data[] They’ll be able to write a correct deserializer in pretty much any language, and can probably write it in ~ 10 lines of code. I agree that packed structs can be tricky, but they’re much better than the vast majority of serialization libraries I’ve dealt with (including all of the open source ones, and I’ve used all the usual suspects).
- jdblair 8y agoBut if you're slamming in-memory structs over the wire, you're using packed structs. You explicitly don't want padding when you use structs that way. Whether you should do this in 2019 is another question, but sometimes it's still the easiest way to implement a binary protocol.
- cardiffspaceman 8y agoAnd you're not writing code for big-endian machines to interoperate with little-endian machines. It's easy to write binary protocol code that way but it isn't portable.
- jdblair 8y agoIt goes without saying that you have get the endian right when you store the values. That's what the endian macros are for.