possible fd bug in PerlIOStdio_close

[p5pRT]

Migrated from rt.perl.org#46173 (status was 'resolved')

Searchable as RT46173$

[p5pRT]

From root@schmorp.de

Created by root@schmorp.de

While reading through perlio.c in current 5.8-devel\, I found these lines in PerlIOStdio_close​:

  int dupfd = 0;   ...   dupfd = PerlLIO_dup(fd);   ...   if (dupfd) {

The dup won't be executed on any of my systems\, but on those systems where it is\, it seems that if PerlLIO_dup returns 0\, the code leaks a filedescriptor. (Unless PerlLIO_dup does deep magic\, but it seems to simply call dup()\, which returns 0 if that fd happens to be free.

just a fyi\, really\, I don't grok perlio :)

Perl Info

``` Flags: category=core severity=low Site configuration information for perl v5.8.8: Configured by Marc Lehmann at Sat Jan 20 00:23:48 CET 2007. Summary of my perl5 (revision 5 version 8 subversion 8) configuration: Platform: osname=linux, osvers=2.6.17.6, archname=amd64-linux uname='linux cerebro 2.6.17.6 #1 smp fri oct 20 19:28:13 cest 2006 x86_64 gnulinux ' config_args='-Duselargefiles -Dxxxxuse64bitint -Uuse64bitall -Dusemymalloc=n -Dcc=gcc -Dccflags=-DPERL_DONT_CREATE_GVSV_disabled -ggdb -Dcppflags=-DPERL_DONT_CREATE_GVSV_disabled -D_GNU_SOURCE -I/opt/include -Doptimize=-O6 -march=opteron -mtune=opteron -funroll-loops -fno-strict-aliasing -Dcccdlflags=-fPIC -Dldflags=-L/opt/perl/lib -L/opt/lib -Dlibs=-ldl -lm -lcrypt -Darchname=amd64-linux -Dprefix=/opt/perl -Dprivlib=/opt/perl/lib/perl5 -Darchlib=/opt/perl/lib/perl5 -Dvendorprefix=/opt/perl -Dvendorlib=/opt/perl/lib/perl5 -Dvendorarch=/opt/perl/lib/perl5 -Dsiteprefix=/opt/perl -Dsitelib=/opt/perl/lib/perl5 -Dsitearch=/opt/perl/lib/perl5 -Dsitebin=/opt/perl/bin -Dman1dir=/opt/perl/man/man1 -Dman3dir=/opt/perl/man/man3 -Dsiteman1dir=/opt/perl/man/man1 -Dsiteman3dir=/opt/perl/man/man3 -Dman1ext=1 -Dman3ext=3 -Dpager=/usr/bin/less -Uafs -Uusesfio -Uusenm -Uuseshrplib -Dd_dosuid -Dusethreads=undef -Duse5005threads=undef -Duseithreads=undef -Dusemultiplicity=undef -Demail=perl-binary@plan9.de -Dcf_email=perl-binary@plan9.de -Dcf_by=Marc Lehmann -Dlocincpth=/opt/perl/include /opt/include -Dmyhostname=localhost -Dmultiarch=undef -Dbin=/opt/perl/bin -Dusedevel -des' hint=recommended, useposix=true, d_sigaction=define usethreads=undef use5005threads=undef useithreads=undef usemultiplicity=undef useperlio=define d_sfio=undef uselargefiles=define usesocks=undef use64bitint=define use64bitall=undef uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='gcc', ccflags ='-DPERL_DONT_CREATE_GVSV_disabled -ggdb -fno-strict-aliasing -pipe -I/opt/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O6 -march=opteron -mtune=opteron -funroll-loops -fno-strict-aliasing', cppflags='-DPERL_DONT_CREATE_GVSV_disabled -D_GNU_SOURCE -I/opt/include -DPERL_DONT_CREATE_GVSV_disabled -ggdb -fno-strict-aliasing -pipe -I/opt/include' ccversion='', gccversion='4.1.2 20061115 (prerelease) (Debian 4.1.1-21)', gccosandvers='' intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16 ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='gcc', ldflags ='-L/opt/perl/lib -L/opt/lib -L/usr/local/lib' libpth=/usr/local/lib /lib /usr/lib libs=-ldl -lm -lcrypt perllibs=-ldl -lm -lcrypt libc=/lib/libc-2.3.6.so, so=so, useshrplib=false, libperl=libperl.a gnulibc_version='2.3.6' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E' cccdlflags='-fPIC', lddlflags='-shared -L/opt/perl/lib -L/opt/lib -L/usr/local/lib' Locally applied patches: MAINT29832 @INC for perl v5.8.8: /root/src/sex /opt/perl/lib/perl5 /opt/perl/lib/perl5 /opt/perl/lib/perl5 /opt/perl/lib/perl5 /opt/perl/lib/perl5 . Environment for perl v5.8.8: HOME=/root LANG (unset) LANGUAGE (unset) LC_CTYPE=de_DE.UTF-8 LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/root/s2:/root/s:/opt/bin:/opt/sbin:/bin:/sbin:/usr/bin:/usr/sbin:/usr/X11/bin:/usr/games:/usr/local/bin:/usr/local/sbin:/root/src/uunet:. PERL5LIB=/root/src/sex PERL5_CPANPLUS_CONFIG=/root/.cpanplus/config PERLDB_OPTS=ornaments=0 PERL_BADLANG (unset) PERL_UNICODE=EAL SHELL=/bin/bash ```

[p5pRT]

From @smpeters

On Sat Oct 06 05​:12​:48 2007\, root@​schmorp.de wrote​:

This is a bug report for perl from root@​schmorp.de\, generated with the help of perlbug 1.35 running under perl v5.8.8.

----------------------------------------------------------------- [Please enter your report here]

While reading through perlio.c in current 5.8-devel\, I found these lines in PerlIOStdio_close​:

int dupfd = 0; ... dupfd = PerlLIO_dup(fd); ... if (dupfd) {

The dup won't be executed on any of my systems\, but on those systems where it is\, it seems that if PerlLIO_dup returns 0\, the code leaks a filedescriptor. (Unless PerlLIO_dup does deep magic\, but it seems to simply call dup()\, which returns 0 if that fd happens to be free.

just a fyi\, really\, I don't grok perlio :)

This does look like a problem since

  if (fd >= 0) {

is used elsewhere in the code. I'll leave well enough alone for now\, but I'll see if I can come up with an adequate test case.

Steve Peters steve@​fisharerojo.org

[p5pRT]

The RT System itself - Status changed from 'new' to 'open'

[p5pRT]

From @nwc10

On Thu\, Dec 20\, 2007 at 06​:42​:49AM -0800\, Steve Peters via RT wrote​:

On Sat Oct 06 05​:12​:48 2007\, root@​schmorp.de wrote​:

While reading through perlio.c in current 5.8-devel\, I found these lines in PerlIOStdio_close​:

int dupfd = 0; ... dupfd = PerlLIO_dup(fd); ... if (dupfd) {

The dup won't be executed on any of my systems\, but on those systems where it is\, it seems that if PerlLIO_dup returns 0\, the code leaks a filedescriptor. (Unless PerlLIO_dup does deep magic\, but it seems to simply call dup()\, which returns 0 if that fd happens to be free.

just a fyi\, really\, I don't grok perlio :)

I don't think that anyone does\, certainly no-one alive\, but I'm not convinced that Nick Ing-Simmons actually understood the interactions in it.

This does look like a problem since

if \(fd >= 0\) \{

is used elsewhere in the code. I'll leave well enough alone for now\, but I'll see if I can come up with an adequate test case.

I'm not sure that it's possible to make a viable test case\, but independent of this bug report I decided that it was wrong\, and fixed it with change 33491. (That code path is triggered on OS X)

Nicholas Clark

Change 33491 by nicholas@​mouse-mill on 2008/03/12 11​:46​:17

  Correct logic error in PerlIOStdio_close() - 0 is an acceptable value   from dup()\, so it can't also be the "don't do anything later" value.

Affected files ...

... //depot/perl/perlio.c#382 edit

Differences ...

==== //depot/perl/perlio.c#382 (text) ====

@​@​ -3130\,7 +3130\,7 @​@​   int invalidate = 0;   IV result = 0;   int saveerr = 0; - int dupfd = 0; + int dupfd = -1; #ifdef SOCKS5_VERSION_NAME   /* Socks lib overrides close() but stdio isn't linked to   that library (though we are) - so we must call close() @​@​ -3171\,7 +3171\,7 @​@​   /* in SOCKS' case\, let close() determine return value */   result = close(fd); #endif - if (dupfd) { + if (dupfd >= 0) {   PerlLIO_dup2(dupfd\,fd);   PerlLIO_close(dupfd);   }

[p5pRT]

@nwc10 - Status changed from 'open' to 'resolved'

[p5pRT]

From @craigberry

On Sun\, Mar 30\, 2008 at 9​:01 AM\, Nicholas Clark \nick@​ccl4\.org wrote​:

On Thu\, Dec 20\, 2007 at 06​:42​:49AM -0800\, Steve Peters via RT wrote​:

On Sat Oct 06 05​:12​:48 2007\, root@​schmorp.de wrote​:

just a fyi\, really\, I don't grok perlio :)

I don't think that anyone does\, certainly no-one alive\, but I'm not convinced that Nick Ing-Simmons actually understood the interactions in it.

While we are scratching our heads\, does anyone know if there is a reason PerlIOUnix_open hard-wires 0666 permissions? Shouldn't it consult umask or just omit permissions when calling open() so whatever defaults are in place take effect?

  if (narg > 0) {   if (*mode == IoTYPE_NUMERIC)   mode++;   else {   imode = PerlIOUnix_oflags(mode);   perm = 0666;   }   if (imode != -1) {   const char *path = SvPV_nolen_const(*args);   fd = PerlLIO_open3(path\, imode\, perm);   }   }

[p5pRT]

From @iabyn

On Sun\, Mar 30\, 2008 at 06​:43​:55PM -0500\, Craig A. Berry wrote​:

While we are scratching our heads\, does anyone know if there is a reason PerlIOUnix_open hard-wires 0666 permissions?

This is standard for UNIX I/O. The kernel applies the process's unmask to the permissions value passed in the open (or whatever) system call.

Eg on Linux​:

  $ strace -o /tmp/tr1 touch /tmp/a   $ grep 666 /tmp/tr1   open("/tmp/a"\, O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK|O_LARGEFILE\, 0666) = 0   $