On Fri, 2012-12-14 at 14:12 +0100, Michal Židek wrote:
> On 12/13/2012 08:55 PM, Jakub Hrozek wrote:
> > On Thu, Dec 13, 2012 at 02:16:25PM -0500, Simo Sorce wrote:
> >> On Thu, 2012-12-13 at 13:19 -0500, Simo Sorce wrote:
> >>> On Thu, 2012-12-13 at 18:06 +0100, Michal Židek wrote:
> >>>> Patch 1: (sssd_nss part) In functions nss_cmd_getXXnam, if working with
> >>>> a local domain and no results were found in sysdb, the entries are
> >>>> invalidated in memory cache too.
> >>>
> >>> Ok in the general case this patch would be sort of useless because if
> >>> you reach sssd_nss it means the mmap cache is already expired and not
> >>> used. But I see you use this in patch 2, see comments below on that.
> >>>
> >>>> Patch 2: (tools part) Tools sss_userdel and sss_groupdel send
> >>>> SSS_NSS_GETXXNAM requests after the entry was removed from sysdb. With
> >>>> the first patch, this invalidates the entry in memory cache.
> >>>>
> >>>> Patches are in attachment.
> >>>
> >>> Hi Michal I see wht you use getpwnam/getgrnam, but doesn't it kind of
> >>> defeat the purpose of userdel/groupdel ?
> >>>
> >>> If you use those functions then in most cases you are going to actually
> >>> update the user in the cache, not delete it. Is this intentional ? It
> >>> looks backwards given you are calling a 'del' function.
> >>>
> >>> It looks to me that what you really want is an explicit delete function
> >>> in sssd_nss that you can call from the cmdline, is that correct ?
> >>
> >> Ha now I get what I was missing.
> >> Jakub reminded me this is for the LOCAL provider. So in that case there
> >> is obviously nothing to refresh.
> >>
> >> With that in mind, full ACK.
> >>
> >> Simo.
> >
> > Pushed to master and sssd-1-9
> 
> I used the mmap cache functions unconditionally here. In the case when 
> mmap cache initialization fails (which we treat as non-fatal error and 
> continue without the mmap cache), this would cause segfaults.
> 
> The attached patch applies on top of the previous ones.

I think this may happen also with my initgr code.
I think we should patch mmap functions to just return with an error if
the context is null, so we do not have to put ugly checks everywhere.

I prefer to nack this one and will send a patch about that shortly, can
you open a ticket please ?

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to