[openssl-dev] Syncing OpenSSL and BoringSSL mont ASM code

[Brian Smith]

Andy Polyakov <appro at 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/