On Thu, Aug 22, 2013 at 07:27:07PM +0200, Jakub Hrozek wrote: > On Thu, Aug 22, 2013 at 10:18:36AM -0400, Stephen Gallagher wrote: > > -----BEGIN PGP SIGNED MESSAGE----- > > Hash: SHA1 > > > > On 08/22/2013 10:11 AM, Stephen Gallagher wrote: > > > On 08/15/2013 11:50 AM, Stephen Gallagher wrote: > > >> There was duplicated code in cc_file_check_existing() and in > > >> cc_dir_check_existing(). I pulled them into the same function. > > > > > >> There are two changes made to the original code here: 1) Fixes a > > >> use-after-free bug in cc_file_check_existing(). In the original > > >> code, we called krb5_free_context() and then used that context > > >> immediately after that in krb5_cc_close(). This patch corrects > > >> the ordering > > > > > >> 2) The krb5_cc_resolve() call handles KRB5_FCC_NOFILE for all > > >> cache types. Previously, this was only handled for DIR caches. > > > > > >> This second part I need someone with Kerberos knowledge to > > >> verify. Is there a risk of receiving this error for the FILE or > > >> KEYRING types, and if so is this handling still acceptable or > > >> should they be special-cased? > > > > > > > > > > > > Self-nack. New patch attached (I was never actually returning the > > > validity result). > > > > > > Interdiff: > > > > > > diff --git a/src/providers/krb5/krb5_utils.c > > > b/src/providers/krb5/krb5_utils.c index 47c45d0..8166435 100644 --- > > > a/src/providers/krb5/krb5_utils.c +++ > > > b/src/providers/krb5/krb5_utils.c @@ -750,6 +750,7 @@ > > > check_cc_validity(const char *location, } > > > > > > ret = EOK; + *_valid = valid; > > > > > > done: if (ccache) krb5_cc_close(context, ccache); > > > > > > > > > Adding another consistency patch that goes along with this: > > > > > > Only set active and valid on success > > > > The FILE cache only sets the return values of _active and _bool if the > > entire function succeeds. The DIR cache was setting it even on failure. > > This patch makes both consistent. This will benefit static analysis > > tools which would be able to detect if the variable is ever used > > uninitialized anywhere. > > > > > > Both patches are attached for simplicity. > > ACK to both. I tested that the ccache is reused if exists and recreated > if doesn't exist for both DIR and FILE ccaches.
Pushed to master and sssd-1-10 _______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
