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. -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
