Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mark as not initialized in destructor #503

Merged
merged 1 commit into from Feb 26, 2020

Conversation

ansasaki
Copy link
Contributor

This is a follow-up on #418

I was investigating a crash similar to #417 which happened if a wrong PIN was provided when using the openssl + engine_pkcs11 + SoftHSM combination. The root of the problem is that the destructor for SoftHSM can be executed before the atexit() handlers installed by other libraries holding references to SoftHSM.

In one particular case, the engine_pkcs11 would call SoftHSM::C_CloseSession() when exiting, which would end in a crash in HandleManager::getSession() because the handleManager had been destroyed earlier.

The bug was reported as part of OpenSC/libp11#273

@rijswijk
Copy link
Contributor

Thanks for the PR, we'll review it and merge it if it passes all build tests.

@ansasaki
Copy link
Contributor Author

These failures in the CI seem to be unrelated with the change I proposed (I'm seeing failures when running the tests using the current develop branch).

Is there anything I can do?

Set isInitialised field as false in SoftHSM::~SoftHSM() to avoid access
to NULL pointers when running callbacks registered by atexit().

Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
@ansasaki
Copy link
Contributor Author

Hello @rijswijk, @halderen, could you please consider merging this before the new release?

Without this I get crashes in specific scenarios, specially when using OpenSSL + engine_pkcs11 + SoftHSM.

@rijswijk
Copy link
Contributor

Yes, certainly, thanks for reminding us, @halderen is actually collecting input for a release.

@halderen halderen merged commit b64fe22 into opendnssec:develop Feb 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants