According to strace -k, this call comes from nss_sss:

fstat(-1, 0x7ffcda2c4620)               = -1 EBADF (Bad file descriptor)
 > /usr/lib64/libc.so.6(__fstat64+0xb) [0xe462b]
 > /usr/lib64/libnss_sss.so.2(sss_cli_check_socket.constprop.0+0x5ac) [0x17dc]
 > /usr/lib64/libnss_sss.so.2(sss_nss_make_request+0x5f) [0x248f]
 > /usr/lib64/libnss_sss.so.2(_nss_sss_getpwnam_r+0x156) [0x47b6]
 > /usr/lib64/libc.so.6(getpwnam_r@@GLIBC_2.2.5+0xd4) [0x129604]
 > /usr/lib64/libc.so.6(getpwnam+0x9d) [0x12945d]
 > /usr/bin/getent(passwd_keys+0xa1) [0x2b11]
 > /usr/bin/getent(main+0xe2) [0x1062]
 > /usr/lib64/libc.so.6(__libc_start_call_main+0x74) [0x35b4]
 > /usr/lib64/libc.so.6(__libc_start_main@@GLIBC_2.34+0x87) [0x3667]
 > /usr/bin/getent(_start+0x24) [0x1134]

Hello Noah and Florian,

I have checked the code and the described behavior is expected.

We leverage the fact that fstat fails for us in such case. Code example:

        ret = fstat(sss_cli_sd_get(), &mypid_sb);
        if (ret == 0) {
            if (S_ISSOCK(mypid_sb.st_mode) &&
                mypid_sb.st_dev == sss_cli_sb->st_dev &&
                mypid_sb.st_ino == sss_cli_sb->st_ino) {
                sss_cli_close_socket();
            }
        }
        sss_cli_sd_set(-1);

Does it actually causes any problems?

(In reply to Tomas Halman from comment #2)
> Does it actually causes any problems?

I think it may cause problems with certain file descriptor debugging tools because using -1 as a file descriptor certainly looks like a missed error check somewhere else. System calls are also a bit slow even if all what they is to report an error.

But I haven't reported this. Hopefully Noah can provide some context.

Hello, no this does not cause any security issues, but it certainly shouldn't be expected behaviour. I encountered this while using valgrind and originally thought something terrible had occurred because my program raised a system call fault within valgrind because -1 was being passed to fstat. I never used fstat in the program and barely allocated any data. But, the issue was being raised and data was left allocated. The data is fine, though annoying because I prefer my apps freeing everything and I can't free data I didn't allocate, but that's all normal behaviour as it wasn't leaked data just unfreed data.

What isn't normal behaviour is passing -1 to a system call. Why exactly are you doing it this way? Why are you checking if a value is -1 by passing it to the kernel and letting it yell at you?

In my opinion, passing -1 as an fd to the kernel should never be expect. It causes debuggers -- specifically valgrind -- to raise one of its deepest faults; no stack trace could be built, it was just a ghost error in my program that made no sense to be there.

If this truly is expected behaviour, and you really do want the kernel to check an integer for you, then you should at least create a valgrind suppression file so people don't freak out when their program's passing -1 to a system call. 

Thank you for your time,
  Noah

To be precise we do not pass that to kernel but to glibc. First thing that fstat in glibc does is a check `fd < 0`. Checking it on SSSD is of course possible but this will be done just for the sake of debugging tools to make them happy. It simply means that fd is checked 2 times for every valid fd.

Considering our capacity, we will not work on this issue but you could submit a patch upstream if you think that it is worth the effort.

Regards
Tomas

There is no conditional check in glibc. You must have looked at the wrong sources (sorry, it's complicated due to the sysdeps overrides).

(gdb) disassemble fstat
Dump of assembler code for function __GI___fstat64:
   0x00000000000e4620 <+0>:	endbr64
   0x00000000000e4624 <+4>:	mov    $0x5,%eax
   0x00000000000e4629 <+9>:	syscall
   0x00000000000e462b <+11>:	cmp    $0xfffffffffffff000,%rax
   0x00000000000e4631 <+17>:	ja     0xe4638 <__GI___fstat64+24>
   0x00000000000e4633 <+19>:	ret
   0x00000000000e4634 <+20>:	nopl   0x0(%rax)
   0x00000000000e4638 <+24>:	mov    0x1047a1(%rip),%rdx        # 0x1e8de0
   0x00000000000e463f <+31>:	neg    %eax
   0x00000000000e4641 <+33>:	mov    %eax,%fs:(%rdx)
   0x00000000000e4644 <+36>:	mov    $0xffffffff,%eax
   0x00000000000e4649 <+41>:	ret
End of assembler dump.

I see, it depends on many things. Anyway as I said, we will not fix that due to our limited capacity. It should not be hard to fix even for someone outside SSSD team. We will appreciate upstream contribution.

T.

Hello, I definitely do not have the right experience to fix this but from my efforts I've found two areas that may be the issue:

```
diff --git a/nscd/nscd_helper.c b/nscd/nscd_helper.c
index 6319fde6f2..4dfb59291b 100644
--- a/nscd/nscd_helper.c
+++ b/nscd/nscd_helper.c
@@ -315,6 +315,9 @@ __nscd_get_mapping (request_type type, const char *key,
   int *ip = (void *) CMSG_DATA (cmsg);
   mapfd = *ip;
 
+  if (mapfd < 0)
+    goto out_close;
+
   if (__glibc_unlikely (n != keylen && n != keylen + sizeof (mapsize)))
     goto out_close;
 
diff --git a/nss/makedb.c b/nss/makedb.c
index efb8f4af30..acff59a0ba 100644
--- a/nss/makedb.c
+++ b/nss/makedb.c
@@ -829,7 +829,7 @@ static int
 print_database (int fd)
 {
   struct stat64 st;
-  if (fstat64 (fd, &st) != 0)
+  if (fd < 0 || fstat64 (fd, &st) != 0)
     error (EXIT_FAILURE, errno, gettext ("cannot stat database file"));
 
   const struct nss_db_header *header = mmap (NULL, st.st_size, PROT_READ,
```

These are both off of glibc latest, neither check fd before use. I am not capable of testing if this patch works, though I tried. If any one in the future thinks this error is error-ish enough then hopefully this gives them a start and less of a headache tracking down fstat calls.

Thanks for you guy's time, 
  Noah