Bug 23737 - [bisected] nss_files returns EINVAL on getpwent()
Summary: [bisected] nss_files returns EINVAL on getpwent()
Status: RESOLVED INVALID
Alias: None
Product: glibc
Classification: Unclassified
Component: nss (show other bugs)
Version: 2.28
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-10-04 08:13 UTC by Sergei Trofimovich
Modified: 2018-10-04 18:36 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sergei Trofimovich 2018-10-04 08:13:02 UTC
Originally observed by Reuben Farrelly in
    https://bugs.gentoo.org/667118

The reproducer:

// $ cat a.c
#include <sys/types.h>
#include <pwd.h>
#include <unistd.h>
#include <stdio.h>
#include <errno.h>

int main() {
    struct passwd * pw;

    for (;;) {
        errno = 0;
        pw = getpwent ();
        if (pw == NULL)
            break;
        if (errno != 0)
            break;
    }
    if (errno != 0) {
        printf("fail: %m (errno=%u)\n", errno);
    }
}
$ gcc a.c -o a && ./a
fail: Invalid argument (errno=22)

Also reproducible on git master when running as:
    gcc a.c -o a && ./elf/ld.so --inhibit-cache --library-path .:nss ./a

glibc-2.27 used to work. I bisected it down to:

  916124ed841745b7a1e0fbc43f9909340b47d373 is the first bad commit
  commit 916124ed841745b7a1e0fbc43f9909340b47d373
  Author: Florian Weimer <fweimer@redhat.com>
  Date:   Fri Jul 6 14:23:15 2018 +0200

    nss_files: Fix re-reading of long lines [BZ #18991]
    
    Use the new __libc_readline_unlocked function to pick up
    reading at the same line in case the buffer needs to be enlarged.

Sounds relevant.

/etc/nsswitch.conf used:

  passwd:      files
  shadow:      files
  group:       files

  hosts:       files dns
  networks:    files dns

  services:    db files
  protocols:   db files
  rpc:         db files
  ethers:      db files
  netmasks:    files
  netgroup:    files
  bootparams:  files

  automount:   files
  aliases:     files
Comment 1 Andreas Schwab 2018-10-04 08:27:21 UTC
You are checking errno when getpwent didn't fail.  That doesn't produce a defined value.
Comment 2 Florian Weimer 2018-10-04 09:09:44 UTC
(In reply to Andreas Schwab from comment #1)
> You are checking errno when getpwent didn't fail.  That doesn't produce a
> defined value.

I agree, this doesn't look like something that can be supported, and I'm pretty sure the old code could clobber errno under the right circumstances, too.

We have other errno-related problems in NSS functions (see bug 21898), but this does not seem to be one of them.
Comment 3 Sergei Trofimovich 2018-10-04 18:36:21 UTC
(In reply to Florian Weimer from comment #2)
> (In reply to Andreas Schwab from comment #1)
> > You are checking errno when getpwent didn't fail.  That doesn't produce a
> > defined value.
> 
> I agree, this doesn't look like something that can be supported, and I'm
> pretty sure the old code could clobber errno under the right circumstances,
> too.
> 
> We have other errno-related problems in NSS functions (see bug 21898), but
> this does not seem to be one of them.

Oh, that is very subtle. But makes sense.

Apologies for the noise and thanks for quick response!