Mailing List Archive

gnupg reopen std - descriptor state problem
Hi there

I have perl web application running on FreeBSD that calls gnupg.
During tests I started getting an error:
fatal: unable to reopen standard input
from gnupg_reopen_std.

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

I don't really know what happens to stdin - the perl application binds
CGI.pm input
to stdin so it may possibly reach eof (but it shouldn't cause EBADF I
guess).

I made a simple patch adding close():

562 if (fstat (STDIN_FILENO, &statbuf) == -1 && errno ==EBADF)
563 {
close(STDIN_FILENO); <<<--- here
564 if (open ("/dev/null",O_RDONLY) == STDIN_FILENO)
565 did_stdin = 1;
566 else
567 did_stdin = 2;
568 }

and it seems to work properly:

57475 gpg2 CALL fstat(0,0x7fffffffdf70)
57475 gpg2 RET fstat -1 errno 9 Bad file descriptor <-- fstat returns
EBADF
57475 gpg2 CALL close(0)
57475 gpg2 RET close 0 <-- close returns OK!
57475 gpg2 CALL openat(AT_FDCWD,0x4abb5b,0<O_RDONLY>,<unused>0)
57475 gpg2 NAMI "/dev/null"
57475 gpg2 RET openat 0

when calling gpg with already closed stdin it behaves as expected:

$ ktrace gpg2 --version <&-

17007 gpg2 CALL fstat(0,0x7fffffffe3e0)
17007 gpg2 RET fstat -1 errno 9 Bad file descriptor <-- fstat returns
EBADF
17007 gpg2 CALL close(0)
17007 gpg2 RET close -1 errno 9 Bad file descriptor <-- close returns
EBADF as well
17007 gpg2 CALL openat(AT_FDCWD,0x4abb5b,0<O_RDONLY>,<unused>0)
17007 gpg2 NAMI "/dev/null"
17007 gpg2 RET openat 0 <-- reopened stdin


Now to the point: I would provide patch if you think that it can get
accepted (would be
useful no to remember to patch gpg :) ).

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

best regards
--
Marcin Gryszkalis, PGP 0xA5DBEEC7 http://fork.pl/gpg.txt


_______________________________________________
Gnupg-devel mailing list
Gnupg-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gnupg-devel
Re: gnupg_reopen_std - descriptor state problem [ In reply to ]
Hello,

Marcin Gryszkalis <mg@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;
--

_______________________________________________
Gnupg-devel mailing list
Gnupg-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gnupg-devel
Re: gnupg_reopen_std - descriptor state problem [ In reply to ]
Hi

On 2018-09-07 04:58, NIIBE Yutaka wrote:
> Interesting. Probably, the stdin is connected to the socket (which may
> be already shutdown-ed), I don't know.

That's possible.

> It seems that stdout is connected to named pipe, btw.
That's possible too.

>> I can't see any problems additional close() could cause - but I'm not
>> sure
>> about all unix plaftorms.
> I think that doing any operation on the fd (which retured EBADF) sounds
> not healthy (even if it's close(2)).

I had similar feeling :)

> 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.

I'll test the patch and report - but I wonder why you decided to use
F_GETFD (and
not for example F_GETFL?

best regards
--
Marcin Gryszkalis, PGP 0xA5DBEEC7
jabber jid:mg@fork.pl

_______________________________________________
Gnupg-devel mailing list
Gnupg-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gnupg-devel
Re: gnupg_reopen_std - descriptor state problem [ In reply to ]
Marcin Gryszkalis <mg@fork.pl> wrote:
> I'll test the patch and report - but I wonder why you decided to use
> F_GETFD (and not for example F_GETFL?

I thought that it is the cheapest. The result of F_GETFD is not related
to any meta data for file or socket. The result can be determined by
kernel, only examining the object of the file descriptor. That's my
theory.

And it's portable enough.
--

_______________________________________________
Gnupg-devel mailing list
Gnupg-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gnupg-devel
Re: gnupg_reopen_std - descriptor state problem [ In reply to ]
On 2018-09-10 11:12, NIIBE Yutaka wrote:
> Marcin Gryszkalis <mg@fork.pl> wrote:
>> I'll test the patch and report - but I wonder why you decided to use
>> F_GETFD (and not for example F_GETFL?
>
> I thought that it is the cheapest. The result of F_GETFD is not
> related
> to any meta data for file or socket. The result can be determined by
> kernel, only examining the object of the file descriptor. That's my
> theory.

On FreeBSD F_GETFD acquires lock whike F_GETFL doesn't - but I don't
know why, it's too deep in the kernel internals for me.
Anyway - 3 calls per exec won't make real overhead.

best regards
--
Marcin Gryszkalis, PGP 0xA5DBEEC7
jabber jid:mg@fork.pl

_______________________________________________
Gnupg-devel mailing list
Gnupg-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gnupg-devel
Re: gnupg reopen std - descriptor state problem [ In reply to ]
On Friday, September 7, 2018 4:58:04 AM CEST, NIIBE Yutaka wrote:
> 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.

I tested your patch:
- in case of manually closing stdin, ie. gpg2 --version <&-

44239 gpg2 CALL fcntl(0,F_GETFD,<invalid>0xffffeb88)
44239 gpg2 RET fcntl -1 errno 9 Bad file descriptor
44239 gpg2 CALL openat(AT_FDCWD,0x4abb8b,0<O_RDONLY>,<unused>0)
44239 gpg2 NAMI "/dev/null"
44239 gpg2 RET openat 0
44239 gpg2 CALL fcntl(0x1,F_GETFD,0)
44239 gpg2 RET fcntl 0
44239 gpg2 CALL fcntl(0x2,F_GETFD,0)
44239 gpg2 RET fcntl 0

- when calling from my app fcntl always returns 0 - and it's good.
I prepared patch version that uses both tests (fcntl and fstat) and it
behaves
as expected (when called from the perl app):

42591 gpg2 CALL fcntl(0,F_GETFD,<invalid>0xffffe720)
42591 gpg2 RET fcntl 0 <=== fcntl is right
42591 gpg2 CALL fstat(0,0x7fffffffdf40)
42591 gpg2 RET fstat -1 errno 9 Bad file descriptor <=== fstat is
wrong :)
42591 gpg2 CALL close(0)
42591 gpg2 RET close 0
42591 gpg2 CALL openat(AT_FDCWD,0x4abc1b,0<O_RDONLY>,<unused>0)
42591 gpg2 NAMI "/dev/null"
42591 gpg2 RET openat 0
42591 gpg2 CALL fcntl(0x1,F_GETFD,0)
42591 gpg2 RET fcntl 0
42591 gpg2 CALL fcntl(0x2,F_GETFD,0)
42591 gpg2 RET fcntl 0

So I guess you could merge the patch.
Thanks for help with this unusual case :)

best regards
--
Marcin Gryszkalis, PGP 0xA5DBEEC7 http://fork.pl/gpg.txt


_______________________________________________
Gnupg-devel mailing list
Gnupg-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gnupg-devel
Re: gnupg_reopen_std - descriptor state problem [ In reply to ]
Marcin Gryszkalis <mg@fork.pl> writes:
> So I guess you could merge the patch.

Thanks a lot for your confirmation. I applied it to 2.2 branch as well
as master.


In GnuPG, to check fd, fstat had been used for years. (GnuPG 1.4 still
uses fstat.) I don't know the reason why we haven't got a bug report
until today. Perhaps, it may be unusual case, as you said.
--

_______________________________________________
Gnupg-devel mailing list
Gnupg-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gnupg-devel