Den 26. aug. 2010 17.26 skrev Paul Mehta <[email protected]>: > > > On Thu, Aug 26, 2010 at 8:06 AM, Vitaly Repeshko <[email protected]>wrote: > >> On Thu, Aug 26, 2010 at 6:55 PM, <[email protected]> wrote: >> > >> > http://codereview.chromium.org/3169050/diff/15001/16001 >> > File src/ia32/codegen-ia32.cc (right): >> > >> > http://codereview.chromium.org/3169050/diff/15001/16001#newcode157 >> > src/ia32/codegen-ia32.cc:157: jit_cookie_ = V8::Random(); >> > No, the platform random probably isn't good enough but implementing a >> > cryptographically secure random number generator for use in things like >> > this (and the manual executable memory randomizer) is on the to-do list. >> > That's probably a separate change. Perhaps a // TODO ? >> >> TODO sounds good. You should file a bug to track this. Especially >> since it already affects the system. >> > > Thanks, I'll be sure to do that and add the TODO. > >> >> > On 2010/08/26 14:40:35, Vitaly wrote: >> >> >> >> Is the platform random function good enough? The attacker has read >> > >> > access to the >> >> >> >> current time and may find a way to predict the cookie value. >> > >> >> Style nit: the cookie field should be set in the initializer list like >> > >> > the other >> >> >> >> fields. >> > >> > http://codereview.chromium.org/3169050/diff/15001/16001#newcode5333 >> > src/ia32/codegen-ia32.cc:5333: __ push(Immediate(bits ^ jit_cookie_)); >> > I'm not quite sure what you mean here. Are you referring to the >> > threshold at which values are encoded (kMaxSmiInlinedBits = 7)? Or are >> > you suggesting something along the lines of #ifndef DEBUG encode #else >> > value splitting ? >> >> x86 instructions have different encoding lengths depending on the >> sizes of the immediate operands. We use this to optimize code size. >> For example, push imm8 is 0x6A imm8, whereas push imm32 is 0x68 imm32. >> This means that depending on the cookie value we will use either a >> two-byte or a five-byte encoding. > > I'll look into the operand sizes, but I believe that this only affects > values >= 1 << kMaxSmiInlinedBits. > > >> So multiple runs of the same v8 >> binary will not only produce different constants in the generated >> code, but also different sizes of code objects, which is not nice for >> debugging and we often have to debug release binaries. >> > > What about a command-line option/api that makes the feature opt-in and if > not it always initializes the cookie to NULL? It would would make the > values very readable. >
That seems utterly pointless. If you have a command line option you have to decide what the default is. Since 99.9% of users will use the default there is no point in having the other option. -- Erik Corry, Software Engineer Google Denmark ApS - Frederiksborggade 20B, 1 sal, 1360 København K - Denmark - CVR nr. 28 86 69 84 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
