On Thu, Jun 07, 2012 at 11:47:35AM +0200, Jan Zelený wrote:
> On Thu, May 31, 2012 at 09:17:18PM +0200, Jan Zeleny wrote:
> > At this moment we will support only asterisk, designating "all
> > services".
> >
> >
https://fedorahosted.org/sssd/ticket/1360
> >
> > Thanks
> > Jan
>
> Nack, you need to initialize services to NULL, otherwise if any
> operation before the strdup failed, you would free random pointer.
Good catch, fixed.
> You can also use sizeof(ALL_SERVICES)-1 and avoid defining
> ALL_SERVICES_LEN (and be safe if ALL_SERVICES changed, not that it's
> likely).
As you have said, the constant is not likely to be changed. The next planned
change is not to use the constant at all and rather dynamically fill this in by
PAM responder. Hence I think leaving this is ok.
> Does the SELinux feature work at all without the patch? If not, we
> should consider moving the ticket to 1.8
The original documentation for the feature was incorrect so it didn't work (or
rather it did work but pam_selinux got all confused). This time I tested the
feature and it is working. There are still some rough edges but I suppose they
are a part of intended behaviour of pam_selinux.
To sum up, yes, this should be also backported to 1.8. Patch attached and
tested.
Sorry, one more thing I didn't notice before, can you move zeroing the
errno right before sss_atomic_write_s() ?