[SSSD] [PATCH 2/2] Making the authtok structure really opaque.

Jakub Hrozek jhrozek at redhat.com
Tue Apr 2 15:03:54 UTC 2013


On Tue, Apr 02, 2013 at 04:44:07PM +0200, Jakub Hrozek wrote:
> On Tue, Apr 02, 2013 at 08:39:22AM -0400, Simo Sorce wrote:
> > On Tue, 2013-04-02 at 10:21 +0200, Lukas Slebodnik wrote:
> > > On (29/03/13 09:59), Simo Sorce wrote:
> > > >On Fri, 2013-03-29 at 11:29 +0100, Lukas Slebodnik wrote:
> > > >> On (25/03/13 12:44), Jakub Hrozek wrote:
> > > >
> > > >> >Nack, I'm getting segfault on login with these patches, backtrace is
> > > >> >pointing to sss_authtok_copy
> > > >> 
> > > >> Problem was that "struct pam_data" was allocated on stack and zero initialized.
> > > >> Now this case is correctly handled in copy_pam_data.
> > > >> 
> > > >> Changes to previous verion of patches:
> > > >>     --only in patch 0002-Making-the-authtok-structure-really-opaque.patch
> > > >>         in file src/providers/dp_pam_data_util.c
> > > >>             in function copy_pam_data
> > > >> 
> > > >> Patches attached.
> > > >
> > > >
> > > >Hey Lukas,
> > > >given you now made authtok an actual talloc context in all cases, I
> > > >think it would make sense to drop the mem_ctx argument from all the
> > > >sss_authtok_set_*() calls. and pass only a sss_auth_tok pointer, and
> > > >document it must be allocated with sss_authotk_new in authtok.h
> > > TALLOC context removed from copy and setter functions.
> > > 
> > > There isn't other way how to create new authtok like using function
> > > sss_authtok_new and it has already been documented.
> > > >
> > > >Also I would rename sss_authtok_new_empty, to just sss_authotk_new,
> > > >whether by default it is empty or not is an internal detail and the
> > > >suffix '_new' is generally explicit enough.
> > > Renamed.
> > > 
> > > >
> > > >Simo.
> > > >
> > > 
> > > New patches attached.
> > 
> > 
> > Looks good to me, if it passes all tests it's an ACK.
> > 
> > Simo.
> 
> I tested LDAP auth, LDAP binding with a DN and password, Kerberos auth
> and Kerberos chpass. All work fine for me.
> 
> Ack.

Pushed to master.



More information about the sssd-devel mailing list