[PATCH] gracefully handle mmap_cache calls when uninitialized

[Simo Sorce]

Failure to init the mmap_cache is a soft error, so just gracefully
return a EINVAL error from public functions if the mc context is NULL
and do not segfault.

Fixes: https://fedorahosted.org/sssd/ticket/1716

Simo.

[Michal Židek]

On 12/14/2012 02:59 PM, Simo Sorce wrote:
> Failure to init the mmap_cache is a soft error, so just gracefully
> return a EINVAL error from public functions if the mc context is NULL
> and do not segfault.
>
> Fixes: https://fedorahosted.org/sssd/ticket/1716
>
> Simo.

Looks good to me. Tested with uninitialized mmap cache and worked fine.

But I have failed to find were we explicitly set the context of memory 
caches to NULL when not initialized properly. Maybe we should place it 
to nss_process_init to the "if (ret)" branch when memory cache 
initialization fails to make it more obvious.

If it is set to NULL explicitly somewhere else, than it is ACK, 
otherwise, please, add it to the nss_process_init and it will be ACK.

Thanks
Michal

[Michal Židek]

On 12/14/2012 03:17 PM, Michal Židek wrote:
> On 12/14/2012 02:59 PM, Simo Sorce wrote:
> > Failure to init the mmap_cache is a soft error, so just gracefully
> > return a EINVAL error from public functions if the mc context is NULL
> > and do not segfault.
> >
> > Fixes: https://fedorahosted.org/sssd/ticket/1716
> >
> > Simo.
>
> Looks good to me. Tested with uninitialized mmap cache and worked fine.
>
> But I have failed to find were we explicitly set the context of memory
> caches to NULL when not initialized properly. Maybe we should place it
> to nss_process_init to the "if (ret)" branch when memory cache
> initialization fails to make it more obvious.
>
> If it is set to NULL explicitly somewhere else, than it is ACK,
> otherwise, please, add it to the nss_process_init and it will be ACK.
>
> Thanks
> Michal

Sorry, I see it now. :-)
ACK.

Michal

[Jakub Hrozek]

On Fri, Dec 14, 2012 at 03:30:14PM +0100, Michal Židek wrote:
> On 12/14/2012 03:17 PM, Michal Židek wrote:
> > On 12/14/2012 02:59 PM, Simo Sorce wrote:
> > > Failure to init the mmap_cache is a soft error, so just gracefully
> > > return a EINVAL error from public functions if the mc context is NULL
> > > and do not segfault.
> > >
> > > Fixes: https://fedorahosted.org/sssd/ticket/1716
> > >
> > > Simo.
> >
> > Looks good to me. Tested with uninitialized mmap cache and worked fine.
> >
> > But I have failed to find were we explicitly set the context of memory
> > caches to NULL when not initialized properly. Maybe we should place it
> > to nss_process_init to the "if (ret)" branch when memory cache
> > initialization fails to make it more obvious.
> >
> > If it is set to NULL explicitly somewhere else, than it is ACK,
> > otherwise, please, add it to the nss_process_init and it will be ACK.
> >
> > Thanks
> > Michal
>
> Sorry, I see it now. :-)
> ACK.
>
> Michal

Pushed to master and sssd-1-9

[Simo Sorce]

On Fri, 2012-12-14 at 15:17 +0100, Michal Židek wrote:
> On 12/14/2012 02:59 PM, Simo Sorce wrote:
> > Failure to init the mmap_cache is a soft error, so just gracefully
> > return a EINVAL error from public functions if the mc context is NULL
> > and do not segfault.
> >
> > Fixes: https://fedorahosted.org/sssd/ticket/1716
> >
> > Simo.
>
> Looks good to me. Tested with uninitialized mmap cache and worked fine.
>
> But I have failed to find were we explicitly set the context of memory 
> caches to NULL when not initialized properly. Maybe we should place it 
> to nss_process_init to the "if (ret)" branch when memory cache 
> initialization fails to make it more obvious.

Here:
http://git.fedorahosted.org/cgit/sssd.git/tree/src/responder/nss/nsssrv.c#n4...
> If it is set to NULL explicitly somewhere else, than it is ACK,

All nctx is set to zero on init, so unless you successfully init a mmap
you have a guaranteed NULL there.
> otherwise, please, add it to the nss_process_init and it will be ACK.

Ok, it's an ACK then :)

Simo.