gnupg_reopen_std - descriptor state problem

[NIIBE Yutaka]

Hello,

Marcin Gryszkalis <mg at fork.pl> wrote:
> I did some ktrace-ing and it seems that stdin has unexpected state:
> fstat(0) indicated EBADF but the desccriptor was not closed.
> After EBADF gnupg_reopen_std assumes it is closed and tries to reopen it to 
> /dev/null 
> (and in my case it gets new descriptor):
>
> 54974 gpg2     CALL  fstat(0,0x7fffffffdf70)
> 54974 gpg2     RET   fstat -1 errno 9 Bad file descriptor	
> 54974 gpg2     CALL  openat(AT_FDCWD,0x4aac8a,0<O_RDONLY>,<unused>0)
> 54974 gpg2     NAMI  "/dev/null"
> 54974 gpg2     RET   openat 5 <-- we don't get 0 but 5 (new file opened)
>
> 54974 gpg2     CALL  fstat(0x1,0x7fffffffdf70) <-- for stdout it's ok
> 54974 gpg2     STRU  struct stat {dev=74, ino=2, mode=010000, nlink=0, 
> uid=1004, gid=1003, rdev=0, atime=1536171587.677756000, 
> mtime=1536171587.677756000, ctime=1536171587.677756000, birthtime=0, 
> size=0, blksize=4096, blocks=0, flags=0x0 }
> 54974 gpg2     RET   fstat 0

Interesting.  Probably, the stdin is connected to the socket (which may
be already shutdown-ed), I don't know.

It seems that stdout is connected to named pipe, btw.

> I can't see any problems additional close() could cause - but I'm not sure 
> about all unix plaftorms. 
> Any comments?

I think that doing any operation on the fd (which retured EBADF) sounds
not healthy (even if it's close(2)).

I wonder if changes like following make sense to see if fd is valid or
not.  My theory is that fcntl is cheaper/simpler operation then fstat,
and might be better in this case.

diff --git a/common/sysutils.c b/common/sysutils.c
index 55a7ee9ec..899850403 100644
--- a/common/sysutils.c
+++ b/common/sysutils.c
@@ -552,13 +552,12 @@ void
 gnupg_reopen_std (const char *pgmname)
 {
 #if defined(HAVE_STAT) && !defined(HAVE_W32_SYSTEM)
-  struct stat statbuf;
   int did_stdin = 0;
   int did_stdout = 0;
   int did_stderr = 0;
   FILE *complain;
 
-  if (fstat (STDIN_FILENO, &statbuf) == -1 && errno ==EBADF)
+  if (fcntl (STDIN_FILENO, F_GETFD) == -1 && errno ==EBADF)
     {
       if (open ("/dev/null",O_RDONLY) == STDIN_FILENO)
 	did_stdin = 1;
@@ -566,7 +565,7 @@ gnupg_reopen_std (const char *pgmname)
 	did_stdin = 2;
     }
 
-  if (fstat (STDOUT_FILENO, &statbuf) == -1 && errno == EBADF)
+  if (fcntl (STDOUT_FILENO, F_GETFD) == -1 && errno == EBADF)
     {
       if (open ("/dev/null",O_WRONLY) == STDOUT_FILENO)
 	did_stdout = 1;
@@ -574,7 +573,7 @@ gnupg_reopen_std (const char *pgmname)
 	did_stdout = 2;
     }
 
-  if (fstat (STDERR_FILENO, &statbuf)==-1 && errno==EBADF)
+  if (fcntl (STDERR_FILENO, F_GETFD)==-1 && errno==EBADF)
     {
       if (open ("/dev/null", O_WRONLY) == STDERR_FILENO)
 	did_stderr = 1;
--