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

Temporary fixes for failed segment and forgotten min_stat exception #29

Closed
wants to merge 2 commits into from

Conversation

Zogoo
Copy link

@Zogoo Zogoo commented Sep 6, 2019

Not expecting to be merged, but I hope this will help some guys figure out the issues.

I hope GSS will doing clean up before this one called.
@@ -179,7 +179,7 @@ def self.release_ptr(name_ptr)
class GssCtxIdT < GssPointer
def self.release_ptr(context_ptr)
min_stat = FFI::MemoryPointer.new :OM_uint32
maj_stat = LibGSSAPI.gss_delete_sec_context(min_stat, context_ptr, LibGSSAPI::GSS_C_NO_BUFFER)
Copy link
Author

Choose a reason for hiding this comment

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

Suspecting that C implementation has been already clean up memory. So, In here it's called again and failing with segmentation.

@@ -261,7 +261,7 @@ def unwrap_message(msg, encrypted = true)
# @param [String] keytab the path to the keytab
def set_keytab(keytab)
maj_stat = LibGSSAPI.krb5_gss_register_acceptor_identity(keytab)
raise GssApiError.new(maj_stat, min_stat), "krb5_gss_register_acceptor_identity did not return GSS_S_COMPLETE" if maj_stat != 0
Copy link
Author

Choose a reason for hiding this comment

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

min_stat will be nil in here, but still, it's trying to get some status from here.

Copy link
Author

@Zogoo Zogoo Nov 25, 2020

Choose a reason for hiding this comment

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

@zenchild in this method there is no min_stat defined in here right? Any explanation that why you use undefined min_stat in here?

Copy link
Owner

Choose a reason for hiding this comment

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

Hmmm, that looks like an oversight. There are a few things implemented in this library that I haven't used. I was coding most of it from documentation. 😄 I'll create a new issue to look into it.

Copy link
Owner

Choose a reason for hiding this comment

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

So I looked at the MIT source and this PR should fix the issue (#33 ). I'll push to a new gem soon.

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.

2 participants