Skip Menu |
Report information
Id: 130098
Status: resolved
Priority: 0/
Queue: perl5

Owner: jkeenan <jkeenan [at] cpan.org>
Requestors: john [at] nixnuts.net
Cc:
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: low
Type: unknown
Perl Version: (no value)
Fixed In: 5.25.9



Subject: Multiple segfaults in Storable.
From: john [...] nixnuts.net
Date: Mon, 14 Nov 2016 12:15:11 +0100
To: perlbug [...] perl.org
Download (untitled) / with headers
text/plain 3.7k
This is a bug report for perl from john@nixnuts.net, generated with the help of perlbug 1.40 running under perl 5.25.7. ----------------------------------------------------------------- AFL pointed out multiple null pointer dereference bugs in Storable. retrieve_code() will dereference a null "text" variable, and retrieve_other() will dereference a null "cxt" variable. I'll attach a patch that fixes the issues AFL identified. ----------------------------------------------------------------- --- Flags: category=library severity=low module=Storable --- Site configuration information for perl 5.25.7: Configured by jd at Wed Nov 9 09:22:44 CST 2016. Summary of my perl5 (revision 5 version 25 subversion 7) configuration: Snapshot of: cc2b46b563e1a9147700b04d0d2e529ae63e2e12 Platform: osname=linux osvers=4.8.0-1-amd64 archname=x86_64-linux uname='linux slug 4.8.0-1-amd64 #1 smp debian 4.8.5-1 (2016-10-28) x86_64 gnulinux ' config_args='-de -Dprefix=/home/jd/perl5/perlbrew/perls/perl-blead -Dcc=/usr/bin/afl-gcc -DDEBUGGING -Dusedevel -Aeval:scriptdir=/home/jd/perl5/perlbrew/perls/perl-blead/bin' hint=recommended useposix=true d_sigaction=define useithreads=undef usemultiplicity=undef use64bitint=define use64bitall=define uselongdouble=undef usemymalloc=n bincompat5005=undef Compiler: cc='/usr/bin/afl-gcc' ccflags ='-fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64' optimize='-O2 -g' cppflags='-fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include' ccversion='' gccversion='6.2.0 20161103' gccosandvers='' intsize=4 longsize=8 ptrsize=8 doublesize=8 byteorder=12345678 doublekind=3 d_longlong=define longlongsize=8 d_longdbl=define longdblsize=16 longdblkind=3 ivtype='long' ivsize=8 nvtype='double' nvsize=8 Off_t='off_t' lseeksize=8 alignbytes=8 prototype=define Linker and Libraries: ld='/usr/bin/afl-gcc' ldflags =' -fstack-protector-strong -L/usr/local/lib' libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/6/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib libs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc libc=libc-2.24.so so=so useshrplib=false libperl=libperl.a gnulibc_version='2.24' Dynamic Linking: dlsrc=dl_dlopen.xs dlext=so d_dlsymun=undef ccdlflags='-Wl,-E' cccdlflags='-fPIC' lddlflags='-shared -O2 -g -L/usr/local/lib -fstack-protector-strong' Locally applied patches: Devel::PatchPerl 1.44 --- @INC for perl 5.25.7: /home/jd/perl5/perlbrew/perls/perl-blead/lib/site_perl/5.25.7/x86_64-linux /home/jd/perl5/perlbrew/perls/perl-blead/lib/site_perl/5.25.7 /home/jd/perl5/perlbrew/perls/perl-blead/lib/5.25.7/x86_64-linux /home/jd/perl5/perlbrew/perls/perl-blead/lib/5.25.7 --- Environment for perl 5.25.7: HOME=/home/jd LANG=en_US.UTF-8 LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/jd/perl5/perlbrew/bin:/home/jd/perl5/perlbrew/perls/perl-blead/bin:/home/jd/.gems/bin:/home/jd/bin:/home/jd/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games PERLBREW_BASHRC_VERSION=0.76 PERLBREW_HOME=/home/jd/.perlbrew PERLBREW_MANPATH=/home/jd/perl5/perlbrew/perls/perl-blead/man PERLBREW_PATH=/home/jd/perl5/perlbrew/bin:/home/jd/perl5/perlbrew/perls/perl-blead/bin PERLBREW_PERL=perl-blead PERLBREW_ROOT=/home/jd/perl5/perlbrew PERLBREW_VERSION=0.76 PERL_BADLANG (unset) SHELL=/bin/bash
Patch for this issue.
Subject: 0001-Fix-Storable-segfaults.patch
From fecd3be8dbdb747b9cbf4cbb9299ce40faabc8e6 Mon Sep 17 00:00:00 2001 From: John Lightsey <lightsey@debian.org> Date: Mon, 14 Nov 2016 11:56:15 +0100 Subject: [PATCH] Fix Storable segfaults. Fix a null pointed dereference segfault in storable when the retrieve_code logic was unable to read the string that contained the code. Also fix several locations where retrieve_other was called with a null context pointer. This also resulted in a null pointer dereference. --- dist/Storable/Storable.xs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/dist/Storable/Storable.xs b/dist/Storable/Storable.xs index 053951c..caa489c 100644 --- a/dist/Storable/Storable.xs +++ b/dist/Storable/Storable.xs @@ -5647,6 +5647,10 @@ static SV *retrieve_code(pTHX_ stcxt_t *cxt, const char *cname) CROAK(("Unexpected type %d in retrieve_code\n", type)); } + if (!text) { + CROAK(("Unable to retrieve code\n")); + } + /* * prepend "sub " to the source */ @@ -5767,7 +5771,7 @@ static SV *old_retrieve_array(pTHX_ stcxt_t *cxt, const char *cname) continue; /* av_extend() already filled us with undef */ } if (c != SX_ITEM) - (void) retrieve_other(aTHX_ (stcxt_t *) 0, 0); /* Will croak out */ + (void) retrieve_other(aTHX_ cxt, 0); /* Will croak out */ TRACEME(("(#%d) item", i)); sv = retrieve(aTHX_ cxt, 0); /* Retrieve item */ if (!sv) @@ -5844,7 +5848,7 @@ static SV *old_retrieve_hash(pTHX_ stcxt_t *cxt, const char *cname) if (!sv) return (SV *) 0; } else - (void) retrieve_other(aTHX_ (stcxt_t *) 0, 0); /* Will croak out */ + (void) retrieve_other(aTHX_ cxt, 0); /* Will croak out */ /* * Get key. @@ -5855,7 +5859,7 @@ static SV *old_retrieve_hash(pTHX_ stcxt_t *cxt, const char *cname) GETMARK(c); if (c != SX_KEY) - (void) retrieve_other(aTHX_ (stcxt_t *) 0, 0); /* Will croak out */ + (void) retrieve_other(aTHX_ cxt, 0); /* Will croak out */ RLEN(size); /* Get key size */ KBUFCHK((STRLEN)size); /* Grow hash key read pool if needed */ if (size) -- 2.10.2
From: John Lightsey <john [...] nixnuts.net>
To: perlbug-followup [...] perl.org
Date: Wed, 30 Nov 2016 20:35:31 -0600
Subject: Re: [perl #130098] Multiple segfaults in Storable.
Updated patch to correct several null pointer deference bugs in Storable is attached.

Message body is not shown because sender requested not to inline it.

RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 346b
On Thu, 01 Dec 2016 02:36:15 GMT, john@nixnuts.net wrote: Show quoted text
> Updated patch to correct several null pointer deference bugs in Storable is > attached.
Can you provide a bit more specific evidence of the problem? Are there any tests we could write for this that would expose regressions? Thank you very much. -- James E Keenan (jkeenan@cpan.org)
Subject: Re: [perl #130098] Multiple segfaults in Storable.
To: perlbug-followup [...] perl.org
Date: Thu, 01 Dec 2016 20:31:44 -0600
From: John Lightsey <john [...] nixnuts.net>
Download (untitled) / with headers
text/plain 1.1k
On Wed, 2016-11-30 at 19:19 -0800, James E Keenan via RT wrote: Show quoted text
> On Thu, 01 Dec 2016 02:36:15 GMT, john@nixnuts.net wrote:
> > Updated patch to correct several null pointer deference bugs in Storable is > > attached.
> > Can you provide a bit more specific evidence of the problem? > > Are there any tests we could write for this that would expose regressions? > > Thank you very much. >
I've bundled up three of the crashing Storable files into the attached test script. These segfault reliably on my x86_64 system in three of the patched locations. $ gdb -ex run -ex bt -batch -args perl ./null_crashes.pl old_retrieve_array [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". Loading old_retrieve_array... Program received signal SIGSEGV, Segmentation fault. retrieve_other (cxt=cxt@entry=0x0, cname=0x0) at Storable.xs:3984 3984                    cxt->ver_major != STORABLE_BIN_MAJOR && #0  retrieve_other (cxt=cxt@entry=0x0, cname=0x0) at Storable.xs:3984 #1  0x00007ffff6872967 in old_retrieve_array (cxt=0x555556355020, cname=<optimized out>) at Storable.xs:5783 ...
Download null_crashes.pl
text/x-perl 706b

Message body is not shown because sender requested not to inline it.

Download signature.asc
application/pgp-signature 833b

Message body not shown because it is not plain text.

RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.7k
On Fri, 02 Dec 2016 02:32:34 GMT, john@nixnuts.net wrote: Show quoted text
> On Wed, 2016-11-30 at 19:19 -0800, James E Keenan via RT wrote:
> > On Thu, 01 Dec 2016 02:36:15 GMT, john@nixnuts.net wrote:
> > > Updated patch to correct several null pointer deference bugs in > > > Storable is > > > attached.
> > > > Can you provide a bit more specific evidence of the problem? > > > > Are there any tests we could write for this that would expose > > regressions? > > > > Thank you very much. > >
> > I've bundled up three of the crashing Storable files into the attached > test > script. These segfault reliably on my x86_64 system in three of the > patched > locations. > > > $ gdb -ex run -ex bt -batch -args perl ./null_crashes.pl > old_retrieve_array > [Thread debugging using libthread_db enabled] > Using host libthread_db library "/lib/x86_64-linux- > gnu/libthread_db.so.1". > Loading old_retrieve_array... > > Program received signal SIGSEGV, Segmentation fault. > retrieve_other (cxt=cxt@entry=0x0, cname=0x0) at Storable.xs:3984 > 3984                    cxt->ver_major != STORABLE_BIN_MAJOR && > #0  retrieve_other (cxt=cxt@entry=0x0, cname=0x0) at Storable.xs:3984 > #1  0x00007ffff6872967 in old_retrieve_array (cxt=0x555556355020, > cname=<optimized out>) at Storable.xs:5783 > ...
John, In the smoke-me/jkeenan/130098-storable branch I applied your patch for Storable.xs. Storable::$VERSION had already been incremented, so I manually re-incremented. I worked your test program into 3 regression tests in t/store.t -- though you might want to suggest better descriptions than mine. P5P: Could this be reviewed by someone more familiar with Storable than I? (Otherwise, I will push to blead within 7 days.) Thank you very much. -- James E Keenan (jkeenan@cpan.org)
RT-Send-CC: perl5-porters [...] perl.org
On Sun, 25 Dec 2016 02:46:38 GMT, jkeenan wrote: Show quoted text
> On Fri, 02 Dec 2016 02:32:34 GMT, john@nixnuts.net wrote:
> > On Wed, 2016-11-30 at 19:19 -0800, James E Keenan via RT wrote:
> > > On Thu, 01 Dec 2016 02:36:15 GMT, john@nixnuts.net wrote:
> > > > Updated patch to correct several null pointer deference bugs in > > > > Storable is > > > > attached.
> > > > > > Can you provide a bit more specific evidence of the problem? > > > > > > Are there any tests we could write for this that would expose > > > regressions? > > > > > > Thank you very much. > > >
> > > > I've bundled up three of the crashing Storable files into the > > attached > > test > > script. These segfault reliably on my x86_64 system in three of the > > patched > > locations. > > > > > > $ gdb -ex run -ex bt -batch -args perl ./null_crashes.pl > > old_retrieve_array > > [Thread debugging using libthread_db enabled] > > Using host libthread_db library "/lib/x86_64-linux- > > gnu/libthread_db.so.1". > > Loading old_retrieve_array... > > > > Program received signal SIGSEGV, Segmentation fault. > > retrieve_other (cxt=cxt@entry=0x0, cname=0x0) at Storable.xs:3984 > > 3984                    cxt->ver_major != STORABLE_BIN_MAJOR && > > #0  retrieve_other (cxt=cxt@entry=0x0, cname=0x0) at Storable.xs:3984 > > #1  0x00007ffff6872967 in old_retrieve_array (cxt=0x555556355020, > > cname=<optimized out>) at Storable.xs:5783 > > ...
> > John, > > In the smoke-me/jkeenan/130098-storable branch I applied your patch > for Storable.xs. Storable::$VERSION had already been incremented, so > I manually re-incremented. I worked your test program into 3 > regression tests in t/store.t -- though you might want to suggest > better descriptions than mine. > > P5P: Could this be reviewed by someone more familiar with Storable > than I? (Otherwise, I will push to blead within 7 days.) > > Thank you very much.
Can you confirm that commit adf9095d629bebb27169b0f3b03f75ee974da100 remedies this situation? Thank you very much. -- James E Keenan (jkeenan@cpan.org)
To: perlbug-followup [...] perl.org
Date: Wed, 04 Jan 2017 22:53:58 -0600
From: John Lightsey <john [...] nixnuts.net>
Subject: Re: [perl #130098] Multiple segfaults in Storable.
Download (untitled) / with headers
text/plain 762b
On Sun, 2017-01-01 at 07:19 -0800, James E Keenan via RT wrote: Show quoted text
> > John, > >  > > In the smoke-me/jkeenan/130098-storable branch I applied your patch > > for Storable.xs.  Storable::$VERSION had already been incremented, so > > I manually re-incremented.  I worked your test program into 3 > > regression tests in t/store.t -- though you might want to suggest > > better descriptions than mine. > >  > > P5P:  Could this be reviewed by someone more familiar with Storable > > than I?  (Otherwise, I will push to blead within 7 days.) > >  > > Thank you very much.
> > Can you confirm that commit adf9095d629bebb27169b0f3b03f75ee974da100 remedies > this situation?
The changes look good and the segfaults are no longer reproducible on my test system.
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 167b
Since the other ticket cited in this one has been marked Resolved, I am marking this one Resolved as well. Thank you very much. -- James E Keenan (jkeenan@cpan.org)


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org