memset on mem or memset on circ?
ABCD> "memset(circ, 0xAA, memlen)" and "memset(conn, 0xAA, memlen)" theoretically incorrect. It should be "memset (mem, 0xAA, memlen)" instead.
ABCD: ah. we're not poisoning all of it, because we're starting at the wrong location? ABCD> no, it's right becaouse offset zero. but actually circ and or_circ not the same ah. yeah. i was just checking out TO_ORIGIN_CIRCUIT() to make sure the offset is zero so the suggestion is to change it to mem, in case one day we make the offset not zero? ABCD> yes, at least DOWNCAST(or_connection_t, c) exist for something. explain that second part? ABCD> as result it STRUCT_OFFSET(subtype, basemember) which zero if offset strictly is zero.
we're probably screwed in plenty of other places if we make _base not the first element of these structs. also, why the heck are we poisoning the two types of structs with exactly the same byte? shouldn't one of them by AB or something? 0xCC would probably make us happier
ABCD> I found only those two cases, introduced by r13414.
ah. those are the commits we added when the cold boot attacks were first known to us but hadn't been made public yet. we made sure to scribble on anything that might be sensitive, when we're done using it. i do wonder how many compilers optimize it out, though. in fact. are the compilers more likely to optimize it out when we memset mem and then free mem? whereas they'll leave it in if we memset circ and then free mem? ABCD> compilers so smart this days, who knows.
So, three questions to ponder.
First: should we change circ and conn to mem in these two cases, to make it clearer which struct we're clobbering? I would say yes.
Second: should we leave circ and conn alone, in hopes that the more complex code is more likely to fool the compiler into not optimizing the code out? I would say yes.
Third: should we change one of the AA's into CC, to reduce the chance that one day we see AA in memory and realize we don't know which struct we're looking at? I would say yes.
I think my 'yes' on #1 and legacy/trac#2 (closed) are incompatible, and I think yes for #1 should take priority.
[Automatically added by flyspray2trac: Operating System: All]