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

Fix SSLSocket NULL pointer deference after close #615

Merged
merged 1 commit into from Aug 4, 2020

Conversation

cipherboy
Copy link
Member

When SSLSocket is closed, the underlying pointer will be immediately
freed and NULLed. This means that any future calls into the native
library could result in a NULL pointer dereference in NSS. This broke
IPA integration tests:

Stack trace of thread 32848:
# - 0  0x00007f362d7aa9e5 raise (libc.so.6 + 0x3c9e5)
# - 1  0x00007f362d793895 abort (libc.so.6 + 0x25895)
# - 2  0x00007f362ca8f4d1 _ZN2os5abortEb.cold (libjvm.so + 0x2114d1)
# - 3  0x00007f362d439a32 _ZN7VMError14report_and_dieEv (libjvm.so + 0xbbba32)
# - 4  0x00007f362d22d934 JVM_handle_linux_signal (libjvm.so + 0x9af934)
# - 5  0x00007f362d2209ac _Z13signalHandleriP9siginfo_tPv (libjvm.so + 0x9a29ac)
# - 6  0x00007f362d7aaa70 __restore_rt (libc.so.6 + 0x3ca70)
# - 7  0x00007f361677a369 Java_org_mozilla_jss_ssl_SSLSocket_shutdownNative (libjss4.so + 0x2c369)
# - 8  0x00007f36187443c7 n/a (n/a + 0x0)

Signed-off-by: Alexander Scheel <ascheel@redhat.com>


Related: https://pagure.io/dogtagpki/issue/3198

I'm not yet sure what changed between this and N-1 builds.

@cipherboy cipherboy added Bug Something isn't working critical Changes which are critical bug fixes labels Aug 3, 2020
@cipherboy cipherboy requested a review from jmagne August 3, 2020 14:36
@cipherboy cipherboy self-assigned this Aug 3, 2020
Copy link
Contributor

@jmagne jmagne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks quite straightforward. Out of curiosity, do you have a test case for that that demonstrated that this approach fixes it? Although I"m guessing the stack traces you have probably pointed you in this direction. Will approve after comment.

@cipherboy cipherboy added this to the 4.7.2 milestone Aug 3, 2020
When SSLSocket is closed, the underlying pointer will be immediately
freed and NULLed. This means that any future calls into the native
library could result in a NULL pointer dereference in NSS. This broke
IPA integration tests:

    Stack trace of thread 32848:
    # - 0  0x00007f362d7aa9e5 raise (libc.so.6 + 0x3c9e5)
    # - 1  0x00007f362d793895 abort (libc.so.6 + 0x25895)
    # - 2  0x00007f362ca8f4d1 _ZN2os5abortEb.cold (libjvm.so + 0x2114d1)
    # - 3  0x00007f362d439a32 _ZN7VMError14report_and_dieEv (libjvm.so + 0xbbba32)
    # - 4  0x00007f362d22d934 JVM_handle_linux_signal (libjvm.so + 0x9af934)
    # - 5  0x00007f362d2209ac _Z13signalHandleriP9siginfo_tPv (libjvm.so + 0x9a29ac)
    # - 6  0x00007f362d7aaa70 __restore_rt (libc.so.6 + 0x3ca70)
    # - 7  0x00007f361677a369 Java_org_mozilla_jss_ssl_SSLSocket_shutdownNative (libjss4.so + 0x2c369)
    # - 8  0x00007f36187443c7 n/a (n/a + 0x0)

Signed-off-by: Alexander Scheel <ascheel@redhat.com>
@cipherboy
Copy link
Member Author

@jmagne I have been unable to reproduce the original issue. I'm hoping that if we merge this, we can re-run IPA's nightly tests as usual (but earlier) and confirm that it fixes the reported issue.

@jmagne
Copy link
Contributor

jmagne commented Aug 4, 2020

OK, that sounds reasonable, Will approve.

@jmagne
Copy link
Contributor

jmagne commented Aug 4, 2020

Unfortunately, the buttons to actually finish off this approval seem to be not present. Therefore just go ahead...

@cipherboy
Copy link
Member Author

ACK, thanks Jack!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working critical Changes which are critical bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants