On Mon, Mar 25, 2013 at 08:45:38AM +0100, Lukas Slebodnik wrote:
On (22/03/13 14:42), Jakub Hrozek wrote:
>On Fri, Mar 22, 2013 at 09:19:13AM +0100, Lukas Slebodnik wrote:
>> On (21/03/13 20:18), Jakub Hrozek wrote:
>> >On Wed, Mar 20, 2013 at 07:17:28PM +0100, Lukas Slebodnik wrote:
>> >> On (20/03/13 13:58), Jakub Hrozek wrote:
>> >> >On Fri, Mar 15, 2013 at 12:46:34PM +0100, Ondrej Kos wrote:
>> >> >> On 03/15/2013 10:34 AM, Lukas Slebodnik wrote:
>> >> >> >On (15/03/13 09:13), Ondrej Kos wrote:
>> >> >> >>On 03/14/2013 12:58 PM, Lukas Slebodnik wrote:
>> >> >> >>>@@ -151,8 +167,10 @@ void pam_print_data(int l,
struct pam_data *pd)
>> >> >> >>> DEBUG(l, ("tty: %s\n",
PAM_SAFE_ITEM(pd->tty)));
>> >> >> >>> DEBUG(l, ("ruser: %s\n",
PAM_SAFE_ITEM(pd->ruser)));
>> >> >> >>> DEBUG(l, ("rhost: %s\n",
PAM_SAFE_ITEM(pd->rhost)));
>> >> >> >>>- DEBUG(l, ("authtok type: %d\n",
sss_authtok_get_type(&pd->authtok)));
>> >> >> >>>- DEBUG(l, ("newauthtok type: %d\n",
sss_authtok_get_type(&pd->newauthtok)));
>> >> >> >>>+ DEBUG(SSSDBG_CRIT_FAILURE,
>> >> >> >>>+ ("authtok type: %d\n",
sss_authtok_get_type(pd->authtok)));
>> >> >> >>>+ DEBUG(SSSDBG_CRIT_FAILURE,
>> >> >> >>>+ ("newauthtok type: %d\n",
sss_authtok_get_type(pd->newauthtok)));
>> >> >> >>> DEBUG(l, ("priv: %d\n",
pd->priv));
>> >> >> >>> DEBUG(l, ("cli_pid: %d\n",
pd->cli_pid));
>> >> >> >>> }
>> >> >> >>
>> >> >> >>The patches look good to me, just please use the debug
level variable
>> >> >> >>here, not hardcoded level.
>> >> >> >>
>> >> >> >>--
>> >> >> >>Ondrej Kos
>> >> >> >
>> >> >> >Thanks.
>> >> >> >I am attaching updated patches.
>> >> >> >
>> >> >> >LS
>> >> >> >
>> >> >> >
>> >> >> >
>> >> >> Ack from me
>> >> >>
>> >> >> Ondra
>> >> >
>> >> >The second patch no longer applies on the current master, can you
rebase
>> >> >it?
>> >>
>> >> I am attaching rebased patches.
>> >>
>> >> LS
>> >
>> >> +struct sss_auth_token *sss_authtok_new_empty(TALLOC_CTX *mem_ctx)
>> >> +{
>> >> + struct sss_auth_token *token;
>> >> +
>> >> + token = talloc_zero(mem_ctx, struct sss_auth_token);
>> >
>> >^^^ tabs instead of spaces.
>> >
>> >> + if (token == NULL) {
>> >> + DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_zero
failed.\n"));
>> >> + }
>> >> +
>> >> + return token;
>> >> +}
>> >> +
>> >> +
>> >
>> >The rest looks good to me.
>>
>> Thanks.
>> I'm attaching updated patches.
>>
>> LS
>
>> - ret = sss_authtok_set_password(pd, &pd->authtok, password,
keysize);
>> + ret = sss_authtok_set_password(pd, pd->authtok, password, keysize);
>
>We agreed with Lukas off-list that it would be safer to use pd->authtok
>as the memory context here, because the memory blob should be owned by
>the authtok structure, not the pam_data structure. It's unlikely we'll
>ever steal or move the memory, but it's better to be safe.
Talloc contex updated.
I am attaching updated patches
LS
Nack, I'm getting segfault on login with these patches, backtrace is
pointing to sss_authtok_copy