[PATCH 2/2] nss: handle stat failure in check_reload_and_get (BZ #28752)

[Sam James]

> On 4 Jun 2022, at 22:29, Sam James via Libc-alpha <libc-alpha@sourceware.org> wrote:
> 
> 
> 
>> On 4 Jun 2022, at 21:51, DJ Delorie <dj@redhat.com> wrote:
>> 
>> Sam James <sam@gentoo.org> writes:
>>> Huh, it still fails, but makes a bit more sense. It's upset because
>>> now we apparently we return a result despite not being started
>>> up:
>> 
>> So back to basics...
>> 
>> This routine is called when we have a real getXbyY() call and need
>> a config.
>> 
>> Our code happens after we've checked for nsswitch.conf changing.
>> 
>> action: FALSE = error returned to user
>> action: TRUE = old configuration re-used
>> action: CONT = continue to load new configuration
>> 
>> services[db] inode.changed action
>> 
>> NULL NO CONT - we need some config
>> NULL YES CONT - we need some config
>> non-null NO CONT - we need to reload
>> non-null YES TRUE - use old config
>> 
>> So I think the patch is correct. I'm seeing tst-reload2 fail also
>> (sigh, only tested tst-reload1)
>> 
>> It looks like we need to initialize local->root_* even if [services] is
>> NULL. I.e. we need to run that stat always, despite deferring its
>> check.
>> 
>> This is a little messier but I think the logic is all in the right
>> order. We might need to move the local-> if{} to before the NULL check,
>> although we must load the config at least once so it should be OK.
>> 
> 
> I'm glad I nearly got there -- I was worried about making the stat
> call at all, but the issue is referencing the result, of course, if
> it's not safe to do so.
> 
> Let me give this a whack!
>> 

Okay, it still fails (I put the check before and after, no difference, although I think after makes more sense), but bear with me:
```
FAIL: nss/tst-reload2
original exit status 1
error: tst-reload2.c:132: not true: pw->pw_uid != 2468
tst-reload2.c:138: numeric comparison failure
   left: 5 (0x5); from: pw->pw_uid
  right: 1234 (0x4d2); from: 1234
error: tst-reload2.c:140: not true: gr != NULL
error: tst-reload2.c:148: not true: he != NULL
error: 4 test failures
make[1]: Leaving directory '/home/sam/git/glibc'
```

Are we sure tst-reload2 is correct?

At line 140, shouldn't it be null (I mean, it is when the test fails, but isn't that okay?), because we don't want to load the inner config?

Especially given on line 142, we want the getgrnam() call to return null. And Group ID 5 is only defined in group_table_data2.

Now, where I'm less sure is the failure on line 148 (gethostbyname). test2 is in subdir/etc/nsswitch.conf and nothing about test2 is available in the outer configuration. *BUT* the comment above it says "... can still load the files DSO.", so I'm less confident I'm understanding that one.

> 
> Best,
> sam

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 358 bytes
Desc: Message signed with OpenPGP
URL: <https://sourceware.org/pipermail/libc-alpha/attachments/20220604/a224fef9/attachment.sig>