Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible filedescriptor race condition in 5.8.3 and 5.8.6 #7869

Open
p5pRT opened this issue Apr 11, 2005 · 7 comments
Open

Possible filedescriptor race condition in 5.8.3 and 5.8.6 #7869

p5pRT opened this issue Apr 11, 2005 · 7 comments

Comments

@p5pRT
Copy link

p5pRT commented Apr 11, 2005

Migrated from rt.perl.org#34914 (status was 'open')

Searchable as RT34914$

@p5pRT
Copy link
Author

p5pRT commented Apr 11, 2005

From dam@baltic-online.de

Hi,

I am currently working with an embedded version of Perl 5.8.3 and
5.8.6 on Solaris together with OpenLDAP 2.2.23.

Perl seems to have a race condition which mixes up filehandles
from other threads. Perl was not compiled with thread support
(neither 5.005 nor ithreads) as all accesses to the Perl instance
have been serialized with mutexes. The problem arises when Perl
issues a close followed by a dup2 onto the closed filehandle and
another thread (unrelated to the Perl instance) opens the filehandle
just closed prior to dup2ing to it. The problem has been analyzed
by Howard Chu from the OpenLDAP team at
  http​://www.openldap.org/its/index.cgi/Incoming?id=3567;selectid=3567;usearchives=1
and he suggested dropping the explicit close as dup2 already
closes the target fd.

As the code in the IO systems seems mighty fragile I would
grately appreciate help removing this race condition.

Best regards

  -- Dagobert Michelsen

--
Dagobert Michelsen (Leiter IT) Baltic Online Computer GmbH
Alter Markt 1-2, 24103 Kiel, +49 431 54003-0 (Fon) -99 (Fax)
Flughafenstr. 52c, 22335 Hamburg, +49 40 53299-395 (Fon) -100 (Fax)

@p5pRT
Copy link
Author

p5pRT commented Apr 12, 2005

From @gsar

On 11 Apr 2005 16​:01​:06 -0000, Dagobert Michelsen wrote​:

# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=34914 >
[...]
I am currently working with an embedded version of Perl 5.8.3 and
5.8.6 on Solaris together with OpenLDAP 2.2.23.

Perl seems to have a race condition which mixes up filehandles
from other threads. Perl was not compiled with thread support
(neither 5.005 nor ithreads) as all accesses to the Perl instance
have been serialized with mutexes. The problem arises when Perl
issues a close followed by a dup2 onto the closed filehandle and
another thread (unrelated to the Perl instance) opens the filehandle
just closed prior to dup2ing to it. The problem has been analyzed
by Howard Chu from the OpenLDAP team at
http​://www.openldap.org/its/index.cgi/Incoming?id=3567;selectid=3567;usearch
ives=1
and he suggested dropping the explicit close as dup2 already
closes the target fd.

I remember fixing a problem similar to the one you've reported back in
2002. Perhaps the problem code has come back?

  http​://public.activestate.com/cgi-bin/perlbrowse?patch=16331

This fix may also be relevant​:

  http​://public.activestate.com/cgi-bin/perlbrowse?patch=16333

HTH,

Sarathy
gsar@​ActiveState.com

@p5pRT
Copy link
Author

p5pRT commented Apr 12, 2005

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

@p5pRT
Copy link
Author

p5pRT commented Apr 12, 2005

From dam@baltic-online.de

Hi,

Von "Gurusamy Sarathy via RT" <perlbug-followup@​perl.org> (12 Apr 2005 03​:26​:42 -0000)​:

On 11 Apr 2005 16​:01​:06 -0000, Dagobert Michelsen wrote​:

# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=34914 >
[...]
I am currently working with an embedded version of Perl 5.8.3 and
5.8.6 on Solaris together with OpenLDAP 2.2.23.

Perl seems to have a race condition which mixes up filehandles
from other threads. Perl was not compiled with thread support
(neither 5.005 nor ithreads) as all accesses to the Perl instance
have been serialized with mutexes. The problem arises when Perl
issues a close followed by a dup2 onto the closed filehandle and
another thread (unrelated to the Perl instance) opens the filehandle
just closed prior to dup2ing to it. The problem has been analyzed
by Howard Chu from the OpenLDAP team at
http​://www.openldap.org/its/index.cgi/Incoming?id=3567;selectid=3567;usearch
ives=1
and he suggested dropping the explicit close as dup2 already
closes the target fd.

I remember fixing a problem similar to the one you've reported back in
2002. Perhaps the problem code has come back?

http&#8203;://public\.activestate\.com/cgi\-bin/perlbrowse?patch=16331

This fix may also be relevant​:

http&#8203;://public\.activestate\.com/cgi\-bin/perlbrowse?patch=16333

The code in 5.8.3 looks quite similar to your fix, however it
only applies if Perl was compiled with threads or ithreads. To
my understanding it is not necessary to compile Perl with thread
support if Perl itself is not threaded, but is used in a single
thread as embedded component (this is the scenario I am using).
Do you see a disadvantage of applying your new code even if
Perl is not compiled with thread-support? Why did you limit
your code to a threaded environment?

Best regards

  -- Dagobert

--
Dagobert Michelsen (Leiter IT) Baltic Online Computer GmbH
Alter Markt 1-2, 24103 Kiel, +49 431 54003-0 (Fon) -99 (Fax)
Flughafenstr. 52c, 22335 Hamburg, +49 40 53299-395 (Fon) -100 (Fax)

@p5pRT
Copy link
Author

p5pRT commented Apr 13, 2005

From @gsar

On Tue, 12 Apr 2005 10​:35​:51 +0200, "Dagobert Michelsen" wrote​:

Von "Gurusamy Sarathy via RT" <perlbug-followup@​perl.org> (12 Apr 2005 03​:26​:4
2 -0000)​:

I remember fixing a problem similar to the one you've reported back in
2002. Perhaps the problem code has come back?

http&#8203;://public\.activestate\.com/cgi\-bin/perlbrowse?patch=16331

This fix may also be relevant​:

http&#8203;://public\.activestate\.com/cgi\-bin/perlbrowse?patch=16333

The code in 5.8.3 looks quite similar to your fix, however it
only applies if Perl was compiled with threads or ithreads. To
my understanding it is not necessary to compile Perl with thread
support if Perl itself is not threaded,

This is not correct. To be completely safe, you need to build
libperl with threads support if you want to run even a single
perl interpreter within a threaded program. Otherwise things
like errno will not work right, since libperl might update a
static copy of errno, while the rest of your program that is
compiled with -D_REENTRANT or similar might do something else,
like indirecting through __errno_location().

Another potential problem is all the foo()/foo_r() functions in
libc and elsewhere that perl links with. I don't think you're
guaranteed to get sane results if the same program calls both
foo() and foo_r() functions from different threads.

Still another problem is mixing malloc()s (either perl's or
libc's) that was built with/without threads support.

There are literally hundreds of other similar cases. It might
be instructive to grep your libc headers for _REENTRANT (or
whatever your libc calls it).

but is used in a single
thread as embedded component (this is the scenario I am using).
Do you see a disadvantage of applying your new code even if
Perl is not compiled with thread-support? Why did you limit
your code to a threaded environment?

To be clear, change#16331 wasn't specific to USE_THREADS.
change#16333 was made USE_THREADS specific because it relies
on libc/stdio internals knowledge, and it doesn't make sense
to add that onerous requirement to the vanilla perl binary
(which does not run multi-threaded).

It might be a good idea to add a separate #define for that code
and enable it by default in USE_THREADS mode. People who want
it can then enable it without USE_THREADS. However, if you're
linking libperl into a threaded program and you didn't compile
libperl with threads support, you're probably going to see
other problems (as in the cases above).

If you're not a perl internals whiz to know whether caveats like
the above matter for your application, I highly recommend the
simple and obvious fix​: build your perl with -Duseithreads.

Incidentally, I see this in 5.8.6's PerlIOStdio_invalidate_fileno()​:

  #if 0
  /* Sarathy's code did this - we fall back to a dup/dup2 hack
  (which isn't thread safe) instead
  */
  # error "Don't know how to set FILE.fileno on your platform"
  #endif

which means that platforms that don't have support in
PerlIOStdio_invalidate_fileno() are silently getting a thread-unsafe
perl when building with -Duseithreads. That seems very broken to me.
How about disabling support for the "stdio" layer on such platforms
when building with -Duseithreads?

Sarathy
gsar@​ActiveState.com

@p5pRT
Copy link
Author

p5pRT commented Apr 13, 2005

From dam@baltic-online.de

Hi,

Von "Gurusamy Sarathy via RT" <perlbug-followup@​perl.org> (13 Apr 2005 02​:17​:50 -0000)​:

On Tue, 12 Apr 2005 10​:35​:51 +0200, "Dagobert Michelsen" wrote​:

Von "Gurusamy Sarathy via RT" <perlbug-followup@​perl.org> (12 Apr 2005 03​:26​:4
2 -0000)​:

I remember fixing a problem similar to the one you've reported back in
2002. Perhaps the problem code has come back?

http&#8203;://public\.activestate\.com/cgi\-bin/perlbrowse?patch=16331

This fix may also be relevant​:

http&#8203;://public\.activestate\.com/cgi\-bin/perlbrowse?patch=16333

The code in 5.8.3 looks quite similar to your fix, however it
only applies if Perl was compiled with threads or ithreads. To
my understanding it is not necessary to compile Perl with thread
support if Perl itself is not threaded,

This is not correct. To be completely safe, you need to build
libperl with threads support if you want to run even a single
perl interpreter within a threaded program. Otherwise things
like errno will not work right, since libperl might update a
static copy of errno, while the rest of your program that is
compiled with -D_REENTRANT or similar might do something else,
like indirecting through __errno_location().

Ok, I see the problem.

If you're not a perl internals whiz to know whether caveats like
the above matter for your application, I highly recommend the
simple and obvious fix​: build your perl with -Duseithreads.

I am just finished recompiling everything with​:

Summary of my perl5 (revision 5.0 version 8 subversion 3) configuration​:
  Platform​:
  osname=solaris, osvers=2.9, archname=sun4-solaris-thread-multi
  uname='sunos bopack9 5.9 generic_112233-12 sun4u sparc sunw,ultra-5_10 '
  config_args='-Dprefix=/opt -Dinstallprefix=/opt -Uinstallusrbinperl -Dusethreads -des'
  hint=recommended, useposix=true, d_sigaction=define
  usethreads=define use5005threads=undef useithreads=define usemultiplicity=define
  useperlio=define d_sfio=undef uselargefiles=define usesocks=undef
  use64bitint=undef use64bitall=undef uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='/opt/SUNWspro/bin/cc', ccflags ='-D_REENTRANT -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
  optimize='-O',
  cppflags='-D_REENTRANT'
  ccversion='Sun WorkShop 6 update 2 C 5.3 Patch 111679-12 2003/05/18', gccversion='', gccosandvers=''
  intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=4321
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
  ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
  alignbytes=8, prototype=define
  Linker and Libraries​:
  ld='/opt/SUNWspro/bin/cc', ldflags =' -L/usr/lib -L/usr/ccs/lib -L/opt/SUNWspro/WS6U2/lib -L/usr/local/lib '
  libpth=/usr/lib /usr/ccs/lib /opt/SUNWspro/WS6U2/lib /usr/local/lib
  libs=-lsocket -lnsl -ldl -lm -lpthread -lc
  perllibs=-lsocket -lnsl -ldl -lm -lpthread -lc
  libc=/lib/libc.so, so=so, useshrplib=false, libperl=libperl.a
  gnulibc_version=''
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags=' '
  cccdlflags='-KPIC', lddlflags='-G -L/usr/lib -L/usr/ccs/lib -L/opt/SUNWspro/WS6U2/lib -L/usr/local/lib'

Characteristics of this binary (from libperl)​:
  Compile-time options​: MULTIPLICITY USE_ITHREADS USE_LARGE_FILES PERL_IMPLICIT_CONTEXT
  Built under solaris
  Compiled at Apr 13 2005 14​:05​:12
  @​INC​:
  ../../opt/lib/perl5/5.8.3/sun4-solaris-thread-multi
  /opt/lib/perl5/5.8.3/sun4-solaris-thread-multi
  /opt/lib/perl5/5.8.3
  /opt/lib/perl5/site_perl/5.8.3/sun4-solaris-thread-multi
  /opt/lib/perl5/site_perl/5.8.3
  /opt/lib/perl5/site_perl

  .

Incidentally, I see this in 5.8.6's PerlIOStdio_invalidate_fileno()​:

\#if 0
/\* Sarathy's code did this \- we fall back to a dup/dup2 hack
   \(which isn't thread safe\) instead
 \*/
\#    error "Don't know how to set FILE\.fileno on your platform"
\#endif

which means that platforms that don't have support in
PerlIOStdio_invalidate_fileno() are silently getting a thread-unsafe
perl when building with -Duseithreads. That seems very broken to me.

I would like to see a big warning when compiling Perl about
this issue.

How about disabling support for the "stdio" layer on such platforms
when building with -Duseithreads?

Do I need to install sfio for this on Solaris? When I start to
trace the program containing the Perl engine (OpenLDAP in this case)
with the compile options above I still get the bad close/dup2 combination
causing the error, even after compiling with ithread-support.
Is this due to the issue you just mentioned above or must I disable
stdio / use sfio instead?

29143/4@​4​: 585.2316 -> back_perl-2.2​:Perl_pp_close(0x1483b0, 0x0, 0x2c7378, 0x74c748)
29143/4@​4​: 585.2318 -> back_perl-2.2​:Perl_do_close(0x1483b0, 0xa02804, 0x1, 0x1f)
29143/4@​4​: 585.2320 -> back_perl-2.2​:Perl_io_close(0x1483b0, 0x76c208, 0x1, 0x8fe0f0)
29143/4@​4​: 585.2322 -> back_perl-2.2​:Perl_PerlIO_close(0x1483b0, 0x175ddc, 0x175dd8, 0x0)
29143/4@​4​: 585.2325 -> back_perl-2.2​:PerlIO__close(0x1483b0, 0x175ddc, 0x0, 0x0)
29143/4@​4​: 585.2327 -> back_perl-2.2​:PerlIOStdio_close(0x1483b0, 0x175ddc, 0x0, 0x0)
29143/4@​4​: 585.2329 -> libc​:fileno(0xf7808, 0x0, 0x0, 0x9006e0)
29143/4@​4​: 585.2331 -> libc​:_flockget(0xf7808, 0x2aa10, 0x0, 0x7f73c000)
29143/4@​4​: 585.2334 -> libthread​:mutex_lock(0x7f73ff20, 0x7f73fdd0, 0x0, 0x7f73c000)
29143/4@​4​: 585.2337 <- libthread​:mutex_lock() = 0
29143/4@​4​: 585.2340 <- libc​:_flockget() = 0x7f73ff20
29143/4@​4​: 585.2342 -> libthread​:mutex_unlock(0x7f73ff20, 0x2aa10, 0x0, 0x7f73c000)
29143/4@​4​: 585.2346 <- libthread​:mutex_unlock() = 0
29143/4@​4​: 585.2349 <- libc​:fileno() = 14
29143/4@​4​: 585.2352 -> back_perl-2.2​:PerlIOUnix_refcnt_dec(0xe, 0x0, 0x0, 0x9006e0)
29143/4@​4​: 585.2355 -> back_perl-2.2​:PerlIO_debug(0x7f3652fc, 0xe, 0x1, 0x0)
29143/4@​4​: 585.2358 -> libthread​:pthread_getspecific(0x4, 0x0, 0x0, 0x0)
29143/4@​4​: 585.2362 <- libthread​:pthread_getspecific() = 0x1483b0
29143/4@​4​: 585.2365 <- back_perl-2.2​:PerlIO_debug() = 0x7f3652fc
29143/4@​4​: 585.2368 <- back_perl-2.2​:PerlIOUnix_refcnt_dec() = 1
29143/4@​4​: 585.2371 -> back_perl-2.2​:Perl_PerlIO_flush(0x1483b0, 0x175ddc, 0x0, 0x9006e0
)
29143/4@​4​: 585.2375 -> back_perl-2.2​:PerlIOStdio_flush(0x1483b0, 0x175ddc, 0x0, 0x9006e0
)
29143/4@​4​: 585.2378 -> libc​:fflush(0xf7808, 0x175ddc, 0x0, 0x9006e0)
29143/4@​4​: 585.2380 -> libc​:_flockget(0xf7808, 0x2a2b4, 0x0, 0x7f73c000)
29143/4@​4​: 585.2383 -> libthread​:mutex_lock(0x7f73ff20, 0x7f73fdd0, 0x0, 0x7f73c000)
29143/4@​4​: 585.2386 <- libthread​:mutex_lock() = 0
29143/4@​4​: 585.2388 <- libc​:_flockget() = 0x7f73ff20
29143/4@​4​: 585.2391 -> libc​:_fflush_u(0xf7808, 0x2a2b4, 0x0, 0x7f73c000)
29143/4@​4​: 585.2393 <- libc​:_fflush_u() = 0
29143/4@​4​: 585.2396 -> libthread​:mutex_unlock(0x7f73ff20, 0x2a2b4, 0x0, 0x7f73c000)
29143/4@​4​: 585.2399 <- libthread​:mutex_unlock() = 0
29143/4@​4​: 585.2401 <- back_perl-2.2​:Perl_PerlIO_flush() = 0
29143/4@​4​: 585.2405 -> libc​:___errno(0x0, 0x175ddc, 0x0, 0x9006e0)
29143/4@​4​: 585.2407 -> libthread​:thr_main(0x0, 0x242dc, 0x0, 0x0)
29143/4@​4​: 585.2409 <- libthread​:thr_main() = 0
29143/4@​4​: 585.2416 -> libc​:thr_errnop(0x7f743654, 0x175ddc, 0x0, 0x9006e0)
29143/4@​4​: 585.2418 -> libthread​:_thr_errnop(0x7f649d6c, 0x175ddc, 0x2185c, 0x7f3471e4)
29143/4@​4​: 585.2421 <- libc​:___errno() = 0x7f630674
29143/4@​4​: 585.2425 -> back_perl-2.2​:PerlIOStdio_invalidate_fileno(0x1483b0, 0xf7808, 0x
2185c, 0x7f3471e4)
29143/4@​4​: 585.2428 <- back_perl-2.2​:PerlIOStdio_invalidate_fileno() = 0
29143/4@​4​: 585.2430 -> libc​:dup(0xe, 0xf7808, 0x2185c, 0x7f3471e4)
29143/4​: 585.2433 dup(14) = 19
29143/4@​4​: 585.2434 <- libc​:dup() = 19
29143/4@​4​: 585.2436 -> libc​:fclose(0xf7808, 0x0, 0x2185c, 0x7f3471e4)
29143/4@​4​: 585.2440 -> libc​:_flockget(0xf7808, 0x2a08c, 0x0, 0x0)
29143/4@​4​: 585.2445 -> libthread​:mutex_lock(0x7f73ff20, 0x7f73fdd0, 0x0, 0x7f73c000)
29143/4@​4​: 585.2449 <- libthread​:mutex_lock() = 0
29143/4@​4​: 585.2452 <- libc​:_flockget() = 0x7f73ff20
29143/4@​4​: 585.2455 -> libc​:_fflush_u(0xf7808, 0x2a08c, 0x0, 0x0)
29143/4@​4​: 585.2457 <- libc​:_fflush_u() = 0
29143/4@​4​: 585.2460 -> libc​:_close(0xe, 0x2a08c, 0x0, 0x0)
29143/4​: 585.2464 close(14) = 0
  =========
29143/4@​4​: 585.2465 <- libc​:_close() = 0
29143/4@​4​: 585.2468 -> libthread​:mutex_unlock(0x7f73ff20, 0x2a08c, 0x0, 0x0)
29143/4@​4​: 585.2471 <- libthread​:mutex_unlock() = 0
29143/4@​4​: 585.2473 -> libthread​:rw_wrlock(0x7f7403d0, 0x2a08c, 0x0, 0x0)
29143/4@​4​: 585.2477 -> libthread​:mutex_lock_internal(0x7f7403d8, 0x7f7403d0, 0x1, 0x
0)
29143/4@​4​: 585.2479 <- libthread​:mutex_lock_internal() = 0
29143/4@​4​: 585.2481 <- libthread​:rw_wrlock() = 0
29143/4@​4​: 585.2485 -> libthread​:rw_unlock(0x7f7403d0, 0xa72, 0x0, 0x0)
29143/4@​4​: 585.2488 -> libthread​:mutex_unlock(0x7f7403d8, 0x5257, 0x0, 0x0)
29143/4@​4​: 585.2491 <- libthread​:mutex_unlock() = 0
29143/4@​4​: 585.2494 <- libthread​:rw_unlock() = 0
29143/4@​4​: 585.2496 <- libc​:fclose() = 0
29143/4@​4​: 585.2499 -> libc​:dup2(0x13, 0xe, 0x2185c, 0x7f3471e4)
29143/4@​4​: 585.2502 -> libc​:_fcntl(0x13, 0x9, 0xe, 0x7f3471e4)
29143/4@​4​: 585.2504 -> libc​:__fcntl(0x13, 0x9, 0xe, 0x0)
29143/4​: 585.2506 fcntl(19, F_DUP2FD, 0x0000000E) = 14
  ===== race condition when another thread uses fd#14 after close
29143/4@​4​: 585.2508 <- libc​:__fcntl() = 14
29143/4@​4​: 585.2510 <- libc​:dup2() = 14
29143/4@​4​: 585.2514 -> libthread​:close(0x13, 0x9, 0xe, 0x7f3471e4)
29143/4@​4​: 585.2518 -> libthread​:_save_nv_regs(0x7f6307c0, 0x0, 0x0, 0x0)
29143/4@​4​: 585.2520 <- libthread​:_save_nv_regs() = 0x7f6307c0
29143/4@​4​: 585.2524 -> libc​:_close(0x13, 0x7f64dd9c, 0x0, 0x0)
29143/4​: 585.2527 close(19) = 0
29143/4@​4​: 585.2528 <- libc​:_close() = 0
29143/4@​4​: 585.2531 <- libthread​:close() = 0
29143/4@​4​: 585.2534 <- back_perl-2.2​:PerlIO__close() = 0

Best regards

  -- Dagobert

--
Dagobert Michelsen (Leiter IT) Baltic Online Computer GmbH
Alter Markt 1-2, 24103 Kiel, +49 431 54003-0 (Fon) -99 (Fax)
Flughafenstr. 52c, 22335 Hamburg, +49 40 53299-395 (Fon) -100 (Fax)
Big Servers, Fat Pipes, Massive Storage, Objects Everywhere

@p5pRT
Copy link
Author

p5pRT commented Apr 13, 2005

From nick@ing-simmons.net

Gurusamy Sarathy <gsar@​ActiveState.com> writes​:

Incidentally, I see this in 5.8.6's PerlIOStdio_invalidate_fileno()​:

#if 0
/* Sarathy's code did this - we fall back to a dup/dup2 hack
(which isn't thread safe) instead
*/
# error "Don't know how to set FILE.fileno on your platform"
#endif

which means that platforms that don't have support in
PerlIOStdio_invalidate_fileno() are silently getting a thread-unsafe
perl when building with -Duseithreads. That seems very broken to me.
How about disabling support for the "stdio" layer on such platforms
when building with -Duseithreads?

Fine. :stdio has "inhertited" that code from pre-layered code.

IIRC at the time I wrote that "such platforms" were​:
  A. Linux
  B. Then-new Solaris
  C. Win32/Borland

which were only ones I had ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants