Andy Polyakov <ap...@openssl.org> wrote: >> 1. Please >> see >> https://boringssl.googlesource.com/boringssl/+/75b833cc819a9d189adb0fdd56327bee600ff9e9. >> >> I think it would be good for OpenSSL to work with Google to integrate >> this patch. > > Will be looked into...
Awesome! Besides the functions that Google changed, I think it is also necessary to change the other implementations of `bn_mul_mont` (including the C implementation) and also `BN_from_montgomery_word`. >> 2. Is the `__chkstk` code that was added [1] to `bn_mul_mont` really >> necessary? > > The SEGV that is mentioned in the commit message and commentary was > actually observed and reported. Well, it's not called SEGV on Windows, > but equivalent has same devastating effect, i.e. application crash. It > takes super-long RSA key, longer than you'd normally use, so it's not > something that a lot of users risk suffering from. But the problem is > real nevertheless. Thanks for clarifying this. I agree that it seems like a good thing to do. This is a very large `alloca` happening in the asm code, which is probably surprising for people reading the code. Maybe it would be a good idea to move the alloca out into the calling function so that it is clear that a *lot* of stack space is being used, potentially. Cheers, Brian -- https://briansmith.org/ -- openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
