We leak #12
Merged
We leak #12
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
CI points out that with your changes, |
|
Changed to WIP, because I found other places we do not seem to exercise that need to return "static" oids |
frozencemetery
previously requested changes
Aug 27, 2020
simo5
force-pushed
the
we_leak
branch
2 times, most recently
from
7cfaa7d to
7866618
Compare
simo5
dismissed
frozencemetery’s stale review
all comments should have been handled
frozencemetery
previously requested changes
Aug 28, 2020
There was a problem hiding this comment.
-
"Silence a bogus gcc warning" - what warning? (Can you add a note to the commit message?)
-
" Use the correct function to free unused creds" - grep indicates that this problem also occurs in
gssi_import_cred_by_mech().
Before merging, I'll do a cleanup pass on commit messages and include what leaks are fixed unless you beat me to it :).
I will
Fixed this too.
Let me know if what I've done is not sufficient. |
Also improve the DEBUG by reporting that messages were truncated. Signed-off-by: Simo Sorce <simo@redhat.com> Reviewed-by: Robbie Harwood <rharwood@redhat.com>
Signed-off-by: Simo Sorce <simo@redhat.com> [rharwood@redhat.com: edit commit message] Reviewed-by: Robbie Harwood <rharwood@redhat.com>
Add a more robust method to wait until gssproxy has reconfigured. This should work better when tests ar run on slow machines or through tools like valgrind which can considerably slow down the tooling. Also increase timeouts to 30 seconds for said slower machines. Signed-off-by: Simo Sorce <simo@redhat.com> [rharwood@redhat.com: commit message tweaks] Reviewed-by: Robbie Harwood <rharwood@redhat.com>
GSSAPI requires some specific APIs to return "static" OIDs that the user does not have to free. The krb5 mechglue in fact requires mechanisms to also honor this or the mech oid will be irretrievably leaked in some cases. To accomodate this, expand use of global mechs structure we already allocate for the gss_inidicate_mechs case so we can return "static" OIDs from calls like ISC and ASC. Signed-off-by: Simo Sorce <simo@redhat.com> [rharwood@redhat.com: commit message fixups] Reviewed-by: Robbie Harwood <rharwood@redhat.com>
frozencemetery
force-pushed
the
we_leak
branch
from
c4400d9 to
d9924fc
Compare
|
Rebased onto main and made some docs changes. |
|
Fixed the item loop in init function and rebased |
While we had already fixed the leak here in main, the code performed unnecessary extra work, so just replacethe whole lot with a function that does not do any extra allocation or copy. Signed-off-by: Simo Sorce <simo@redhat.com> [rharwood@redhat.com: commit message] Reviewed-by: Robbie Harwood <rharwood@redhat.com>
The xdr_free() call only frees the contents and not the containing structure itself. Signed-off-by: Simo Sorce <simo@redhat.com> Reviewed-by: Robbie Harwood <rharwood@redhat.com>
Signed-off-by: Simo Sorce <simo@redhat.com> Reviewed-by: Robbie Harwood <rharwood@redhat.com>
These are mostly laziness in freeing since the programs are short-lived. Signed-off-by: Simo Sorce <simo@redhat.com> [rharwood@redhat.com: rewrote commit message] Reviewed-by: Robbie Harwood <rharwood@redhat.com>
Signed-off-by: Simo Sorce <simo@redhat.com> [rharwood@redhat.com: rewrote commit message] Reviewed-by: Robbie Harwood <rharwood@redhat.com>
gss_display_name and gss_inquire_name reteurn "static" oids, that are generally not freed by callers, so make sure to match and return actual static OIDs exported by GSSAPI. Also remove gpm_equal_oids() and use the library provided gss_oid_equal function instead. Signed-off-by: Simo Sorce <simo@redhat.com> Reviewed-by: Robbie Harwood <rharwood@redhat.com>
Signed-off-by: Simo Sorce <simo@redhat.com> [rharwood@redhat.com: clarified commit message] Reviewed-by: Robbie Harwood <rharwood@redhat.com>
As per other functions gssapi expect a static OID here. Signed-off-by: Simo Sorce <simo@redhat.com> [rharwood@redhat.com: commit message fixup] Reviewed-by: Robbie Harwood <rharwood@redhat.com>
frozencemetery
force-pushed
the
we_leak
branch
from
1f3bdb7 to
502e448
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Return OIDs as static from SPI functions as expected by GSSAPI
Fix a bunch of leaks as reported by #11
Also improve how we handle reloads or waiting by avoiding arbitrary sleeps, although this did not help with the two tests failing under valgrind as hoped.
Fixes #9
Fixes #11