Fix evp_keyex_create error handling
#487
Conversation
| @@ -656,8 +656,10 @@ static int evp_keyex_create(ptls_key_exchange_algorithm_t *algo, ptls_key_exchan | |||
| } | |||
|
|
|||
| /* setup */ | |||
| if ((ret = evp_keyex_init(algo, ctx, pkey)) != 0) | |||
| if ((ret = evp_keyex_init(algo, ctx, pkey)) != 0) { | |||
| pkey = NULL; /* was free'd in the error path */ | |||
There was a problem hiding this comment.
Thank you for the PR.
IIUC, the rationale for this change is that if evp_keyex_init fails, refcount of pkey is decremented by evp_keyex_init, hence we should set pkey to NULL to avoid another decrement.
Assuming that is the case, I wonder if we should stop decrementing inside evp_keyex_init rather than adjusting here. Rationale is as follows.
In the other call site of evp_keyex_init which is ptls_openssl_create_key_exchange, when we return an error and the input key type is EVP_PKEY_EC or something unknown, refcount of pkey stays as is. It is only when the key type is X25519 that we decrement when returning an error, due to how evp_pkey_init is implemented today.
This sounds like a bug of evp_pkey_init. Hence the idea of not decrementing inside evp_pkey_init.
WDYT?
|
The other issue with this PR is that it is a partial fix; it does not deal with the case where I've created #490, PTAL. |
No description provided.