perlio.c has a problem (both 5.8 and 5.10) #9347

p5pRT · 2008-05-26T06:10:39Z

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

Searchable as RT54828$

p5pRT · 2008-05-26T06:10:41Z

From g.psy.va@gmail.com

Hi.

I think perlio.c has an issue in PerlIORaw_pushed().
It may cause segmentation fault or invalid abortion in (at least)
MSWin32 ActivePerl 5.10.0 and Cygwin Perl 5.8.8 and 5.10.0.

The problem is that PerlIORaw_pushed() may supply a layer's Binmode()
with an invalid function table. See the line 1293 at perlio.c of
5.10.0. At the line, Binmode() is called with "f", not "t". It seems a
mistake, because the argument is the method recerver, namely "self",
which has to match with the function. This is "t"'s Binmode() (==
"&l"'s) function, so the argument should be "t".

I have written a patch and test. Please see the attached.

Thanks.

Goro Fuji gfuji@cpan.org

Flags:
category=core
severity=low

This perlbug was built using Perl v5.8.5 in the Red Hat build system.
It is being executed now by Perl v5.8.5 - Fri Dec 16 14:05:59 EST 2005.

Site configuration information for perl v5.8.5:

Configured by Red Hat, Inc. at Fri Dec 16 14:05:59 EST 2005.

Summary of my perl5 (revision 5 version 8 subversion 5) configuration:
Platform:
osname=linux, osvers=2.6.9-22.18.bz155725.elsmp,
archname=i386-linux-thread-multi
uname='linux hs20-bc1-4.build.redhat.com
2.6.9-22.18.bz155725.elsmp #1 smp thu nov 17 15:34:08 est 2005 i686
i686 i386 gnulinux '
config_args='-des -Doptimize=-O2 -g -pipe -m32 -march=i386
-mtune=pentium4 -Dversion=5.8.5 -Dmyhostname=localhost
-Dperladmin=root@localhost -Dcc=gcc -Dcf_by=Red Hat, Inc.
-Dinstallprefix=/usr -Dprefix=/usr -Darchname=i386-linux
-Dvendorprefix=/usr -Dsiteprefix=/usr -Duseshrplib -Dusethreads
-Duseithreads -Duselargefiles -Dd_dosuid -Dd_semctl_semun -Di_db
-Ui_ndbm -Di_gdbm -Di_shadow -Di_syslog -Dman3ext=3pm -Duseperlio
-Dinstallusrbinperl -Ubincompat5005 -Uversiononly
-Dpager=/usr/bin/less -isr -Dinc_version_list=5.8.4 5.8.3 5.8.2 5.8.1
5.8.0'
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='gcc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBUGGING
-fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE
-D_FILE_OFFSET_BITS=64 -I/usr/include/gdbm',
optimize='-O2 -g -pipe -m32 -march=i386 -mtune=pentium4',
cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBUGGING
-fno-strict-aliasing -pipe -I/usr/local/include -I/usr/include/gdbm'
ccversion='', gccversion='3.4.5 20051201 (Red Hat 3.4.5-1)', gccosandvers=''
intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t',
lseeksize=8
alignbytes=4, prototype=define
Linker and Libraries:
ld='gcc', ldflags =' -L/usr/local/lib'
libpth=/usr/local/lib /lib /usr/lib
libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc
perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
libc=/lib/libc-2.3.4.so, so=so, useshrplib=true, libperl=libperl.so
gnulibc_version='2.3.4'
Dynamic Linking:
dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E
-Wl,-rpath,/usr/lib/perl5/5.8.5/i386-linux-thread-multi/CORE'
cccdlflags='-fPIC', lddlflags='-shared -L/usr/local/lib'

Locally applied patches:

@INC for perl v5.8.5:
/home1/s0710509/perl5/lib/perl5/5.8.5/5.8.5/i386-linux-thread-multi
/home1/s0710509/perl5/lib/perl5/5.8.5/5.8.5
/home1/s0710509/perl5/lib/perl5/5.8.5/i386-linux-thread-multi
/home1/s0710509/perl5/lib/perl5/5.8.5/5.8.4
/home1/s0710509/perl5/lib/perl5/5.8.5/5.8.3
/home1/s0710509/perl5/lib/perl5/5.8.5/5.8.2
/home1/s0710509/perl5/lib/perl5/5.8.5/5.8.1
/home1/s0710509/perl5/lib/perl5/5.8.5/5.8.0
/home1/s0710509/perl5/lib/perl5/5.8.5
/home1/s0710509/perl5/lib/perl5/site_perl/5.8.5/5.8.5/i386-linux-thread-multi
/home1/s0710509/perl5/lib/perl5/site_perl/5.8.5/5.8.5
/home1/s0710509/perl5/lib/perl5/site_perl/5.8.5/i386-linux-thread-multi
/home1/s0710509/perl5/lib/perl5/site_perl/5.8.5/5.8.4
/home1/s0710509/perl5/lib/perl5/site_perl/5.8.5/5.8.3
/home1/s0710509/perl5/lib/perl5/site_perl/5.8.5/5.8.2
/home1/s0710509/perl5/lib/perl5/site_perl/5.8.5/5.8.1
/home1/s0710509/perl5/lib/perl5/site_perl/5.8.5/5.8.0
/home1/s0710509/perl5/lib/perl5/site_perl/5.8.5
/usr/lib/perl5/5.8.5/i386-linux-thread-multi
/usr/lib/perl5/5.8.5
/usr/lib/perl5/site_perl/5.8.5/i386-linux-thread-multi
/usr/lib/perl5/site_perl/5.8.4/i386-linux-thread-multi
/usr/lib/perl5/site_perl/5.8.3/i386-linux-thread-multi
/usr/lib/perl5/site_perl/5.8.2/i386-linux-thread-multi
/usr/lib/perl5/site_perl/5.8.1/i386-linux-thread-multi
/usr/lib/perl5/site_perl/5.8.0/i386-linux-thread-multi
/usr/lib/perl5/site_perl/5.8.5
/usr/lib/perl5/site_perl/5.8.4
/usr/lib/perl5/site_perl/5.8.3
/usr/lib/perl5/site_perl/5.8.2
/usr/lib/perl5/site_perl/5.8.1
/usr/lib/perl5/site_perl/5.8.0
/usr/lib/perl5/site_perl
/usr/lib/perl5/vendor_perl/5.8.5/i386-linux-thread-multi
/usr/lib/perl5/vendor_perl/5.8.4/i386-linux-thread-multi
/usr/lib/perl5/vendor_perl/5.8.3/i386-linux-thread-multi
/usr/lib/perl5/vendor_perl/5.8.2/i386-linux-thread-multi
/usr/lib/perl5/vendor_perl/5.8.1/i386-linux-thread-multi
/usr/lib/perl5/vendor_perl/5.8.0/i386-linux-thread-multi
/usr/lib/perl5/vendor_perl/5.8.5
/usr/lib/perl5/vendor_perl/5.8.4
/usr/lib/perl5/vendor_perl/5.8.3
/usr/lib/perl5/vendor_perl/5.8.2
/usr/lib/perl5/vendor_perl/5.8.1
/usr/lib/perl5/vendor_perl/5.8.0
/usr/lib/perl5/vendor_perl
.

Environment for perl v5.8.5:
HOME=/home1/s0710509
LANG=ja_JP.UTF-8
LANGUAGE (unset)
LD_LIBRARY_PATH (unset)
LOGDIR (unset)
PATH=/home1/s0710509/bin:/usr/kerberos/bin:/usr/java/jdk1.5.0_11/bin:/usr/java/jre1.5.0_11/bin:/usr/local/eclipse:/usr/local/netbeans-5.0/bin:/usr/local/Adobe/Acrobat7.0/bin:/usr/local/thunderbird:/usr/local/firefox:/usr/local/bin:/bin:/usr/bin:/usr/X11R6/bin
PERL5LIB=/home1/s0710509/perl5/lib/perl5/5.8.5:/home1/s0710509/perl5/lib/perl5/site_perl/5.8.5
PERL_BADLANG (unset)
SHELL=/bin/bash

p5pRT · 2008-05-26T06:10:41Z

From g.psy.va@gmail.com

perlio.c.diff

p5pRT · 2008-05-26T06:10:41Z

From g.psy.va@gmail.com

#!perl -w

use Test::More tests => 3;

use PerlIO::via::QuotedPrint;

# (1) push any layer with its own binmode() that touches its own fields
ok binmode STDIN, ':via(QuotedPrint)';

# (2) push any layer that is not the layer above
ok binmode STDIN, ':unix'; # or ':scalar(foo)' etc.

#diag join ' ', PerlIO::get_layers(*STDIN);

# (3) calls PerlIORaw_pushed()
ok binmode STDIN, ':raw';

p5pRT · 2008-06-01T14:07:34Z

From @rgs

2008/5/26 via RT Goro Fuji <perlbug-followup@perl.org>:

I think perlio.c has an issue in PerlIORaw_pushed().
It may cause segmentation fault or invalid abortion in (at least)
MSWin32 ActivePerl 5.10.0 and Cygwin Perl 5.8.8 and 5.10.0.

The problem is that PerlIORaw_pushed() may supply a layer's Binmode()
with an invalid function table. See the line 1293 at perlio.c of
5.10.0. At the line, Binmode() is called with "f", not "t". It seems a
mistake, because the argument is the method recerver, namely "self",
which has to match with the function. This is "t"'s Binmode() (==
"&l"'s) function, so the argument should be "t".

I agree with this.

I have written a patch and test. Please see the attached.

Good catch. I've applied the first chunk of the patch (see below). I
don't understand why the 2nd chunk is needed, and I don't have a Win32
machine to test with, so I'd like another pair of eyes to look at
this.

(PS. Please send patches as the output of diff -u, for easier review
and application).

Change 33978 by rgs@scipion on 2008/06/01 14:05:16

Subject: [perl #54828] perlio.c has a problem (both 5.8 and 5.10)
From: "Goro Fuji" (via RT) <perlbug-followup@perl.org>
Date: Sun, 25 May 2008 23:10:42 -0700
Message-ID: <rt-3.6.HEAD-11257-1211782242-1590.54828-75-0@perl.org>

First chunk of the patch only

Affected files ...

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

Differences ...

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

@@ -1295,7 +1295,7 @@
while (t && (l = *t)) {
if (l->tab->Binmode) {
/* Has a handler - normal case */
- if ((*l->tab->Binmode)(aTHX_ f) == 0) {
+ if ((*l->tab->Binmode)(aTHX_ t) == 0) {
if (*t == l) {
/* Layer still there - move down a layer */
t = PerlIONext(t);

p5pRT · 2008-06-01T14:07:36Z

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

p5pRT · 2008-06-03T00:59:59Z

From g.psy.va@gmail.com

In fact, the second chunk of the patch is not necessarily needed, but
it removes redundancy.

(copied from the patch)
4588,4591c4588,4589
< /* CRLF is unusual case - if this is just the :crlf layer pop it */
< if (PerlIOBase(f)->tab == &PerlIO_crlf) {
< PerlIO_pop(aTHX_ f);
< }

/\* CRLF is unusual case \- pop it \*/
PerlIO\_pop\(aTHX\_ f\);

The first chunk of the patch always guarantees PerlIOBase(f)->tab ==
&PerlIO_crlf, so the validity check is unnecessary.

Thanks,
--
Goro Fuji (�$BF#�(B �$B8cO:�(B)

p5pRT · 2008-11-08T04:23:39Z

From @smpeters

On Mon Jun 02 17:59:59 2008, GFUJI wrote:

In fact, the second chunk of the patch is not necessarily needed, but
it removes redundancy.

(copied from the patch)
4588,4591c4588,4589
< /* CRLF is unusual case - if this is just the :crlf layer pop it */
< if (PerlIOBase(f)->tab == &PerlIO_crlf) {
< PerlIO_pop(aTHX_ f);
< }
---
/\* CRLF is unusual case \- pop it \*/
PerlIO\_pop$aTHX\_ f$;
The first chunk of the patch always guarantees PerlIOBase(f)->tab ==
&PerlIO_crlf, so the validity check is unnecessary.

OK, I think the confusion was that the second part wasn't really related directly to the first
part. I have added the second part as change #34774. Thanks!

Steve Peters

p5pRT · 2008-11-08T04:23:43Z

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

p5pRT · 2008-11-08T04:23:44Z

From @smpeters

On Mon Jun 02 17:59:59 2008, GFUJI wrote:

In fact, the second chunk of the patch is not necessarily needed, but
it removes redundancy.

(copied from the patch)
4588,4591c4588,4589
< /* CRLF is unusual case - if this is just the :crlf layer pop it */
< if (PerlIOBase(f)->tab == &PerlIO_crlf) {
< PerlIO_pop(aTHX_ f);
< }
---
/\* CRLF is unusual case \- pop it \*/
PerlIO\_pop$aTHX\_ f$;
The first chunk of the patch always guarantees PerlIOBase(f)->tab ==
&PerlIO_crlf, so the validity check is unnecessary.

OK, I think the confusion was that the second part wasn't really related directly to the first
part. I have added the second part as change #34774. Thanks!

Steve Peters

p5pRT closed this as completed Nov 8, 2008

p5pRT added the Severity Low label Oct 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perlio.c has a problem (both 5.8 and 5.10) #9347

perlio.c has a problem (both 5.8 and 5.10) #9347

p5pRT commented May 26, 2008

p5pRT commented May 26, 2008

p5pRT commented May 26, 2008

p5pRT commented May 26, 2008

p5pRT commented Jun 1, 2008

p5pRT commented Jun 1, 2008

p5pRT commented Jun 3, 2008

p5pRT commented Nov 8, 2008

p5pRT commented Nov 8, 2008

p5pRT commented Nov 8, 2008

perlio.c has a problem (both 5.8 and 5.10) #9347

perlio.c has a problem (both 5.8 and 5.10) #9347

Comments

p5pRT commented May 26, 2008

p5pRT commented May 26, 2008

From g.psy.va@gmail.com

p5pRT commented May 26, 2008

From g.psy.va@gmail.com

p5pRT commented May 26, 2008

From g.psy.va@gmail.com

p5pRT commented Jun 1, 2008

From @rgs

p5pRT commented Jun 1, 2008

p5pRT commented Jun 3, 2008

From g.psy.va@gmail.com

p5pRT commented Nov 8, 2008

From @smpeters

p5pRT commented Nov 8, 2008

p5pRT commented Nov 8, 2008

From @smpeters