Am Donnerstag 18 März 2010 17:15:39 schrieb Dmitri Pal: > Ralf Haferkamp wrote: > > Am Donnerstag 18 März 2010 15:25:49 schrieb Dmitri Pal: > >> Ralf Haferkamp wrote: > >>> Am Donnerstag 18 März 2010 12:42:23 schrieb Simo Sorce: > >>>> On Wed, 17 Mar 2010 15:33:38 +0100 > >>>> > >>>> Ralf Haferkamp <rha...@suse.de> wrote: > >>>>> Hi, > >>>>> > >>>>> here's another set of enhancements to the LDAP Password Policy > >>>>> support in the PAM module and the LDAP backend. The PAM module > >>>>> now issues warning when either the grace counter or the expire > >>>>> counter of the LDAP Ppolicy Control in > 0. > >>>>> > >>>>> Addtionally I changed the detection for Ppolicy support of the > >>>>> LDAP Server a bit. If the Server returned the Ppolicy Control in > >>>>> the Bind Response ppolicy support is assumed. > >>>>> > >>>>> I left the original check for "pwdAttribute" intact, though I > >>>>> think it doesn't make a lot of sense. The "pwdAttribute" LDAP > >>>>> Attribute is usually not part of the user's entry, it's part of > >>>>> the Entry that contains the Policy. Addtionally it might be > >>>>> protected by ACLs and not be returned for anonymous (without > >>>>> losing any functionality). > >>>> > >>>> Ralf, > >>>> patch looks mostly good, but there are some heavy coding style > >>>> violations. > >>>> > >>>> if ( condition ){ is not good, it should be: is (condition) { > >>>> > >>>> same for some functions foo( ccc ); is not good, use foo(ccc); > >>>> > >>>> Ie, no space in parenthesis, a space only after keywords, and > >>>> always before the opening { > >>>> > >>>> Don't use ++x, but x++ if possible. > >>>> > >>>> Also there is at least one place where the return of talloc is > >>>> not checked. > >>>> > >>>> Finally please try to keep the 80 columns limit where possible. > >>> > >>> Updated patch attached. I think I fixed the coding style issues. > >>> Additionally I just noticed that my orginal patch broke password > >>> resets via LDAP password policies. This should be fixed now as > >>> well. > >>> > >>> regards, > >>> > >>> Ralf > >> > >> + data = talloc_size(pd, 2* sizeof(uint32_t)); > >> + if (*data == NULL) { > >> + DEBUG(1, ("talloc_size failed.\n")); > >> + return ENOMEM; > >> + } > >> > >> > >> The returned value check does not look right. > >> I do not know if there are other places with similar logic. > > > > There weren't (The one above was embarrasing enough. I must have > > overlooked the compiler warning :| ). Updated patch attached. > > Shouldn't it be freed somewhere down within the same function? As Sumit already pointed, that will happen when the associated struct pam_data is talloc_free'd after the response was sent to the client.
Ralf _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel