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

ioctl: possible memory corruption error on formerly valid code #8280

Closed
p5pRT opened this issue Jan 13, 2006 · 8 comments
Closed

ioctl: possible memory corruption error on formerly valid code #8280

p5pRT opened this issue Jan 13, 2006 · 8 comments

Comments

@p5pRT
Copy link

p5pRT commented Jan 13, 2006

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

Searchable as RT38223$

@p5pRT
Copy link
Author

p5pRT commented Jan 13, 2006

From chowes@kirk.clients.pkts.ca

Created by chperl@pkts.ca

Hi...

I have a program that uses ioctl to check the status of a serial
port using ioctl. I just upgraded to Fedora Core 4, with perl 5.8.6.
My program doesn't work anymore, giving me this error​:

Possible memory corruption​: ioctl overflowed 3rd argument at dotemp line
65.

I then got an example program for ioctl, from the man page perlfaq8,
under the question "How do I get the screen size?". It also gave
the same error​:

Possible memory corruption​: ioctl overflowed 3rd argument at dowinsize
line 5.

#!/usr/bin/perl
require 'sys/ioctl.ph';
die "no TIOCGWINSZ " unless defined &TIOCGWINSZ;
open(TTY, "+</dev/tty") or die "No tty​: $!";
unless (ioctl(TTY, &TIOCGWINSZ, $winsize='')) {
  die sprintf "$0​: ioctl TIOCGWINSZ (%08x​: $!)\n", &TIOCGWINSZ;
}
($row, $col, $xpixel, $ypixel) = unpack('S4', $winsize);
print "(row,col) = ($row,$col)";
print " (xpixel,ypixel) = ($xpixel,$ypixel)" if $xpixel || $ypixel;
print "\n";

The example program works fully under perl 5.8.0.

Perl Info

Flags:
    category=core
    severity=high

This perlbug was built using Perl v5.8.6 in the Red Hat build system.
It is being executed now by Perl v5.8.6 - Wed Dec 14 14:12:26 EST 2005.

Site configuration information for perl v5.8.6:

Configured by Red Hat, Inc. at Wed Dec 14 14:12:26 EST 2005.

Summary of my perl5 (revision 5 version 8 subversion 6) configuration:
  Platform:
    osname=linux, osvers=2.6.9-22.18.bz155725.elsmp,
archname=i386-linux-thread-multi
    uname='linux hs20-bc1-5.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 -Wp,-D_FORTIFY_SOURCE=2
-fexceptions -m32 -march=i386 -mtune=pentium4
-fasynchronous-unwind-tables -Dversion=5.8.6 -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=n -Ubincompat5005 -Uversiononly
-Dpager=/usr/bin/less -isr -Dd_gethostent_r_proto -Ud_endhostent_r_proto
-Ud_endprotoent_r_proto -Ud_endservent_r_proto -Ud_sethostent_r_proto
-Ud_setprotoent_r_proto -Ud_setservent_r_proto -Dinc_version_list=5.8.5
5.8.4 5.8.3'
    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 -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -m32
-march=i386 -mtune=pentium4 -fasynchronous-unwind-tables',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBUGGING
-fno-strict-aliasing -pipe -I/usr/local/include -I/usr/include/gdbm'
    ccversion='', gccversion='4.0.2 20051125 (Red Hat 4.0.2-8)',
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=-lresolv -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread
-lc
    perllibs=-lresolv -lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
    libc=/lib/libc-2.3.5.so, so=so, useshrplib=true, libperl=libperl.so
    gnulibc_version='2.3.5'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E
-Wl,-rpath,/usr/lib/perl5/5.8.6/i386-linux-thread-multi/CORE'
    cccdlflags='-fPIC', lddlflags='-shared -L/usr/local/lib'

Locally applied patches:
    


@INC for perl v5.8.6:
    /usr/lib/perl5/site_perl/5.8.6/i386-linux-thread-multi
    /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.6
    /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
    /usr/lib/perl5/vendor_perl/5.8.6/i386-linux-thread-multi
    /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.6
    /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
    /usr/lib/perl5/5.8.6/i386-linux-thread-multi
    /usr/lib/perl5/5.8.6
    .


Environment for perl v5.8.6:
    HOME=/home/croot
    LANG=POSIX
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)

PATH=/home/croot/bin:/home/croot/bin/LINUX:/home/croot/pubbin/LINUX:/usr/depot/ccache-2.2/mybin:/usr/depot/distcc/mybin:/usr/local/bin:/bin:/usr/local/etc:/usr/sbin:/usr/ucb:/sbin:/usr/5bin:/usr/X11/bin:/usr/bin:/usr/bin/X11:/usr/bsd:/usr/ccs/bin:/usr/etc:/usr/games:/usr/lib:/usr/libexec:/usr/X11R6/bin:/usr/local/sbin:.
    PERL_BADLANG (unset)
    SHELL=/bin/csh


@p5pRT
Copy link
Author

p5pRT commented Jan 13, 2006

From @gisle

Charles Howes (via RT) <perlbug-followup@​perl.org> writes​:

I have a program that uses ioctl to check the status of a serial
port using ioctl. I just upgraded to Fedora Core 4, with perl 5.8.6.
My program doesn't work anymore, giving me this error​:

Possible memory corruption​: ioctl overflowed 3rd argument at dotemp line
65.

I then got an example program for ioctl, from the man page perlfaq8,
under the question "How do I get the screen size?". It also gave
the same error​:

Possible memory corruption​: ioctl overflowed 3rd argument at dowinsize
line 5.

#!/usr/bin/perl
require 'sys/ioctl.ph';
die "no TIOCGWINSZ " unless defined &TIOCGWINSZ;
open(TTY, "+</dev/tty") or die "No tty​: $!";
unless (ioctl(TTY, &TIOCGWINSZ, $winsize='')) {
die sprintf "$0​: ioctl TIOCGWINSZ (%08x​: $!)\n", &TIOCGWINSZ;
}
($row, $col, $xpixel, $ypixel) = unpack('S4', $winsize);
print "(row,col) = ($row,$col)";
print " (xpixel,ypixel) = ($xpixel,$ypixel)" if $xpixel || $ypixel;
print "\n";

The example program works fully under perl 5.8.0.

On my Gentoo box this program also breaks with blead. I tracked it
down to change #25852 which has also been integrated into maint to
appear in 5.8.8. Does this mean that Fedora has applied this patch to
their 5.8.6?

I suggest that we contiue to provide as least 256 byte buffer to
ioctl, that is use 256 if _IOC_SIZE(x) returns something smaller.

--Gisle

Change 25852 by rgs@​bloom on 2005/10/26 09​:37​:25

  Subject​: [perl #37535] [PATCH] ioctl IOCPARM_LEN(x) should be _IOC_SIZE(x) on Linux, not 256
  From​: Jason Vas Dias (via RT) <perlbug-followup@​perl.org>
  Date​: Tue, 25 Oct 2005 15​:27​:28 -0700
  Message-ID​: <rt-3.0.11-37535-123290.14.118037538994@​perl.org>

Affected files ...

... //depot/perl/perl.h#638 edit

Differences ...

==== //depot/perl/perl.h#638 (text) ====
Index​: perl/perl.h

Inline Patch
--- perl/perl.h.~1~     Fri Jan 13 12:26:06 2006
+++ perl/perl.h Fri Jan 13 12:26:06 2006
@@ -2959,11 +2959,16 @@

 #ifndef IOCPARM_LEN
 #   ifdef IOCPARM_MASK
-       /* on BSDish systes we're safe */
+       /* on BSDish systems we're safe */
 #      define IOCPARM_LEN(x)  (((x) >> 16) & IOCPARM_MASK)
 #   else
+#      if defined(_IOC_SIZE) && defined(__GLIBC__)
+       /* on Linux systems we're safe */
+#          define IOCPARM_LEN(x) _IOC_SIZE(x)
+#      else
        /* otherwise guess at what's safe */
-#      define IOCPARM_LEN(x)   256
+#          define IOCPARM_LEN(x)       256
+#      endif
 #   endif
 #endif

End of Patch.

@p5pRT
Copy link
Author

p5pRT commented Jan 13, 2006

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

@p5pRT
Copy link
Author

p5pRT commented Jan 13, 2006

From @rgs

Gisle Aas wrote​:

On my Gentoo box this program also breaks with blead. I tracked it
down to change #25852 which has also been integrated into maint to
appear in 5.8.8. Does this mean that Fedora has applied this patch to
their 5.8.6?

Well, this patch comes from Fedora :

  https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=171111

I suggest that we contiue to provide as least 256 byte buffer to
ioctl, that is use 256 if _IOC_SIZE(x) returns something smaller.

OK, but that looks like working around a glibc bug.

@p5pRT
Copy link
Author

p5pRT commented Jan 13, 2006

From @gisle

Rafael Garcia-Suarez <rgarciasuarez@​mandriva.com> writes​:

I suggest that we contiue to provide as least 256 byte buffer to
ioctl, that is use 256 if _IOC_SIZE(x) returns something smaller.

OK, but that looks like working around a glibc bug.

Perhaps. Is _IOC_SIZE() supposed to be part of the public API?

The TIOCGWINSZ constant is 0x5413 on my system. _IOC_SIZE() basically
just shift the bits right by 16, so this ends up as 0 for this 'func'.

--Gisle

@p5pRT
Copy link
Author

p5pRT commented Jan 13, 2006

From @gisle

Gisle Aas <gisle@​ActiveState.com> writes​:

Rafael Garcia-Suarez <rgarciasuarez@​mandriva.com> writes​:

I suggest that we contiue to provide as least 256 byte buffer to
ioctl, that is use 256 if _IOC_SIZE(x) returns something smaller.

OK, but that looks like working around a glibc bug.

Perhaps. Is _IOC_SIZE() supposed to be part of the public API?

The TIOCGWINSZ constant is 0x5413 on my system. _IOC_SIZE() basically
just shift the bits right by 16, so this ends up as 0 for this 'func'.

I've now applied the following patch to blead, and I do recommend it
for 'maint' as well.

An alternative could be to only grow IOCPARM_LEN() when _IOC_SIZE
actually returns 0. That could potentially make some small requests
more efficient since perl would not have to grow the SV passed in.

--Gisle

Change 26815 by gisle@​gisle-ask on 2006/01/13 12​:10​:28

  Fix [perl #38223]; _IOC_SIZE() not always safe.

Affected files ...

... //depot/perl/perl.h#657 edit

Differences ...

==== //depot/perl/perl.h#657 (text) ====
Index​: perl/perl.h

Inline Patch
--- perl/perl.h.~1~	Fri Jan 13 04:10:49 2006
+++ perl/perl.h	Fri Jan 13 04:10:49 2006
@@ -2977,8 +2977,8 @@
 #	define IOCPARM_LEN(x)  (((x) >> 16) & IOCPARM_MASK)
 #   else
 #	if defined(_IOC_SIZE) && defined(__GLIBC__)
-	/* on Linux systems we're safe */
-#	    define IOCPARM_LEN(x) _IOC_SIZE(x)
+	/* on Linux systems we're safe; except when we're not [perl #38223] */
+#	    define IOCPARM_LEN(x) (_IOC_SIZE(x) < 256 ? 256 : _IOC_SIZE(x))
 #	else
 	/* otherwise guess at what's safe */
 #	    define IOCPARM_LEN(x)	256
End of Patch.

@p5pRT p5pRT closed this as completed Jan 13, 2006
@p5pRT
Copy link
Author

p5pRT commented Jan 13, 2006

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

@p5pRT
Copy link
Author

p5pRT commented Mar 13, 2006

From iankko@mail.muni.cz

[gisle@​ActiveState.com - Fri Jan 13 04​:21​:50 2006]​:

Gisle Aas <gisle@​ActiveState.com> writes​:

Rafael Garcia-Suarez <rgarciasuarez@​mandriva.com> writes​:

I suggest that we contiue to provide as least 256 byte buffer to
ioctl, that is use 256 if _IOC_SIZE(x) returns something smaller.

OK, but that looks like working around a glibc bug.

Perhaps. Is _IOC_SIZE() supposed to be part of the public API?

The TIOCGWINSZ constant is 0x5413 on my system. _IOC_SIZE() basically
just shift the bits right by 16, so this ends up as 0 for this 'func'.

I've now applied the following patch to blead, and I do recommend it
for 'maint' as well.

An alternative could be to only grow IOCPARM_LEN() when _IOC_SIZE
actually returns 0. That could potentially make some small requests
more efficient since perl would not have to grow the SV passed in.

--Gisle

Change 26815 by gisle@​gisle-ask on 2006/01/13 12​:10​:28

Fix \[perl \#38223\]; \_IOC\_SIZE\(\) not always safe\.

Affected files ...

... //depot/perl/perl.h#657 edit

Differences ...

==== //depot/perl/perl.h#657 (text) ====
Index​: perl/perl.h
--- perl/perl.h.1 Fri Jan 13 04​:10​:49 2006
+++ perl/perl.h Fri Jan 13 04​:10​:49 2006
@​@​ -2977,8 +2977,8 @​@​
# define IOCPARM_LEN(x) (((x) >> 16) & IOCPARM_MASK)
# else
# if defined(_IOC_SIZE) && defined(__GLIBC__)
- /* on Linux systems we're safe */
-# define IOCPARM_LEN(x) _IOC_SIZE(x)
+ /* on Linux systems we're safe; except when we're not [perl #38223] */
+# define IOCPARM_LEN(x) (_IOC_SIZE(x) < 256 ? 256 : _IOC_SIZE(x))
# else
/* otherwise guess at what's safe */
# define IOCPARM_LEN(x) 256
End of Patch.

Have run the test script​: (from this page)

#!/usr/bin/perl
require 'sys/ioctl.ph';
die "no TIOCGWINSZ " unless defined &TIOCGWINSZ;
...
print " (xpixel,ypixel) = ($xpixel,$ypixel)" if $xpixel || $ypixel;
print "\n";

on 3 systems with 3 format of the "perl.h" file​:

1, perl 5.8.8

./test.perl
(row,col) = (54,155) => PASS

Form of the patch​:

#ifndef IOCPARM_LEN
# ifdef IOCPARM_MASK
  /* on BSDish systems we're safe */
# define IOCPARM_LEN(x) (((x) >> 16) & IOCPARM_MASK)
# else
# if defined(_IOC_SIZE) && defined(__GLIBC__)
  /* on Linux systems we're safe; except when we're not [perl
#38223] */
# define IOCPARM_LEN(x) (_IOC_SIZE(x) < 256 ? 256 : _IOC_SIZE(x))
# else
  /* otherwise guess at what's safe */
# define IOCPARM_LEN(x) 256
# endif
# endif
#endif

2, perl 5.8.6

./test.perl
(row,col) = (54,155) => PASS

Form of the patch​:

#ifndef IOCPARM_LEN
# ifdef IOCPARM_MASK
  /* on BSDish systes we're safe */
# define IOCPARM_LEN(x) (((x) >> 16) & IOCPARM_MASK)
# else
  /* otherwise guess at what's safe */
# define IOCPARM_LEN(x) 256
# endif
#endif

=> seems it's enough to hardly set the value of IOCPARM_LEN(x) to 256
and don't take into account the Linux system's branch, because​:

3, perl 5.8.6

./test.perl
Possible memory corruption​: ioctl overflowed 3rd argument at ./test.perl
line 5.

=> FAIL

Form of the patch​:

#ifndef IOCPARM_LEN
# ifdef IOCPARM_MASK
  /* on BSDish systems we're safe */
# define IOCPARM_LEN(x) (((x) >> 16) & IOCPARM_MASK)
# else
# ifdef _IOC_SIZE
  /* on Linux systems we're safe */
# define IOCPARM_LEN(x) _IOC_SIZE(x)
# else
  /* otherwise guess at what's safe (we're UNSAFE!) */
# warning "unsafe assumption of IOCPARM_LEN=256"
# define IOCPARM_LEN(x) 256
# endif
# endif
#endif

This patch doesn't work.

So the solution is either to use patch without the linux branch, and hardly
set value of IOCPARM_LEN(x) to 256 (patch 2,), or take into account
_IOC_SIZE and use the newest patch ( patch 1,).

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

No branches or pull requests

1 participant