EVP: don't touch the lock for evp_pkey_downgrade
authorDaniel Bevenius <daniel.bevenius@gmail.com>
Wed, 11 Nov 2020 04:23:11 +0000 (05:23 +0100)
committerTomas Mraz <tmraz@fedoraproject.org>
Thu, 26 Nov 2020 17:06:06 +0000 (18:06 +0100)
commit8dc34b1f579f71f24aa385d33112da4a91db7079
tree3f5fd821b9cb6c7759aed5f7c9b32ecacea2388c
parent2b407d050868c24ee36172e1abcfbfa0f003a98d
EVP: don't touch the lock for evp_pkey_downgrade

This commit tries to address a locking issue in evp_pkey_reset_unlocked
which can occur when it is called from evp_pkey_downgrade.

evp_pkey_downgrade will acquire a lock for pk->lock and if successful
then call evp_pkey_reset_unlocked. evp_pkey_reset_unlocked will call
memset on pk, and then create a new lock and set pk->lock to point to
that new lock. I believe there are two problems with this.

The first is that after the call to memset, another thread would try to
acquire a lock for NULL as that is what the value of pk->lock would be
at that point.

The second issue is that after the new lock has been assigned to
pk->lock, that lock is different from the one currently locked so
another thread trying to acquire the lock will succeed which can lead to
strange behaviour. More details and a reproducer can be found in the
Refs link below.

This changes the evp_pkey_reset_unlocked to not touch the lock
and the creation of a new lock is done in EVP_PKEY_new.

Refs:
https://github.com/danbev/learning-libcrypto/blob/master/notes/issues.md#openssl-investigationtroubleshooting
https://github.com/nodejs/node/issues/29817

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/13374)
crypto/evp/p_lib.c