Re: nss_db: protect against empty mappings

[Carlos O'Donell <carlos at redhat dot com>]

On 6/17/19 11:15 PM, DJ Delorie wrote:
> 
> "Carlos O'Donell" <carlos@redhat.com> writes:
>>> +    {
>>> +      munmap (mapping->header, mapping->len);
>>> +      mapping->header = NULL;
>>
>> No. This merges the fix for 24695 also. We should be careful here to fix just
>> the thing that's broken and avoid merging both fixes which are coincidentally
>> in the same place. I would suggest removing the mapping->header = NULL; change
>> and leave it for your other patch to change.
> 
> I wrote the patches in the other order; if I make this one independent
> the other one will be dependent.  Plus the Makefile is order-dependent
> too.
> 
> Unless I do
> 
>   if (mapping == NULL)
>     return;
> 
> But I don't like using that idiom in case other things need to be done
> in endgrent, independent of mapping.
> 
> Unless you mean to write the patches in isolation and merge them after
> review?  I don't think either of these is critical enough to not just
> wait until they're both reviewed.

Merge the fixes then.

Submit one '[PATCH] nss_db: Protect against empty mappys (BZ #xxx, BZ #yyy)'

And list both bugs in the ChangeLog.

>>> +  unlink ("/var/db/group.db");
>>> +
>>> +  getgrent ();
>>
>> Do you need this getgrent() or can you call endgrent() right away?
> 
> Without it, we don't parse nsswitch, and don't connect to the nss-db
> module, and don't get into the "we don't have a database" case.

OK, then leave it there.

>> A call to endgrent() should be idempotent, it should do nothing,
>> and we should be able to call it many times without errno being
>> set.
> 
> Without getgrent() it really does nothing ;-)

You're right because we don't parse and load db.

-- 
Cheers,
Carlos.