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

recursion in overload sub results in segfault #16333

Open
p5pRT opened this issue Dec 21, 2017 · 11 comments
Open

recursion in overload sub results in segfault #16333

p5pRT opened this issue Dec 21, 2017 · 11 comments

Comments

@p5pRT
Copy link

p5pRT commented Dec 21, 2017

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

Searchable as RT132638$

@p5pRT
Copy link
Author

p5pRT commented Dec 21, 2017

From data@cpan.org

Created by data@cpan.org

Hi,

i've run accross a segmentation fault with the following script​:

package Clock;
use 5.010;
use strict;

use overload
'+' => \&add_time,
'-' => \&sub_time,
'eq' => \&equals_to,
'""' => \&to_string;

sub equals_to{ $_[0] eq $_[1] }
sub to_string { sprintf '%02d​:%02d', $_[0][0] // 0, $_[0][1] // 0; }
sub new { bless( $_[1], $_[0] ); }

package main;
Clock->new([8,0]) eq "08​:00"
__END__

The SIGSEGV goes away when I change equals_to to​:

  sub equals_to{ $_[0]->to_string eq $_[1] }

bye,

Danijel

Perl Info

Flags:
    category=core
    severity=low

Site configuration information for perl 5.26.1:

Configured by danielt at Thu Dec 21 07:13:42 CET 2017.

Summary of my perl5 (revision 5 version 26 subversion 1) configuration:
   
  Platform:
    osname=linux
    osvers=4.9.57
    archname=x86_64-linux
    uname='linux banjo.rbfh.de 4.9.57 #1 smp sat oct 21 15:46:05 cest 2017 x86_64 gnulinux '
    config_args='-de -Dprefix=/opt/perl/perls/perl-5.26.1 -Aeval:scriptdir=/opt/perl/perls/perl-5.26.1/bin'
    hint=recommended
    useposix=true
    d_sigaction=define
    useithreads=undef
    usemultiplicity=undef
    use64bitint=define
    use64bitall=define
    uselongdouble=undef
    usemymalloc=n
    default_inc_excludes_dot=define
    bincompat5005=undef
  Compiler:
    cc='cc'
    ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2'
    optimize='-O2'
    cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
    ccversion=''
    gccversion='4.7.2'
    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='cc'
    ldflags =' -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/4.7/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 -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
    perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
    libc=
    so=so
    useshrplib=false
    libperl=libperl.a
    gnulibc_version='2.13'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=so
    d_dlsymun=undef
    ccdlflags='-Wl,-E'
    cccdlflags='-fPIC'
    lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector'



@INC for perl 5.26.1:
    /opt/perl/perls/perl-5.26.1/lib/site_perl/5.26.1/x86_64-linux
    /opt/perl/perls/perl-5.26.1/lib/site_perl/5.26.1
    /opt/perl/perls/perl-5.26.1/lib/5.26.1/x86_64-linux
    /opt/perl/perls/perl-5.26.1/lib/5.26.1


Environment for perl 5.26.1:
    HOME=/home/danielt
    LANG=C
    LANGUAGE=C
    LC_CTYPE=de_DE.UTF-8
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/opt/perl/bin:/opt/perl/perls/perl-5.26.1/bin:/usr/local/go/bin:/home/danielt/.nvm/v8.1.4/bin:/opt/python/2_007_008/bin:/opt/aperl/bin:/home/danielt/bin:/home/danielt/unix/bin:/usr/opt/bin:/usr/local/bin:/usr/local/sbin:/usr/X11R6/bin/xscreensaver-hacks/:/sbin:/bin:/usr/sbin:/usr/bin:/usr/games:/usr/local/bin:/usr/X11R6/bin:/home/danielt/bin:/usr/pkg/bin:/usr/pkg/sbin:/home/danielt/.rvm/bin
    PERLBREW_BASHRC_VERSION=0.73
    PERLBREW_HOME=/home/danielt/.perlbrew
    PERLBREW_MANPATH=/opt/perl/perls/perl-5.26.1/man
    PERLBREW_PATH=/opt/perl/bin:/opt/perl/perls/perl-5.26.1/bin
    PERLBREW_PERL=perl-5.26.1
    PERLBREW_ROOT=/opt/perl
    PERLBREW_SHELLRC_VERSION=0.82
    PERLBREW_VERSION=0.82
    PERL_BADLANG (unset)
    PERL_CPANM_OPT=--skip-installed --prompt --mirror http://cpan.cpantesters.org
    SHELL=/bin/zsh

@p5pRT
Copy link
Author

p5pRT commented Dec 22, 2017

From @jkeenan

On Thu, 21 Dec 2017 09​:52​:59 GMT, DaTa wrote​:

This is a bug report for perl from data@​cpan.org,
generated with the help of perlbug 1.40 running under perl 5.26.1.

-----------------------------------------------------------------
[Please describe your issue here]

Hi,

i've run accross a segmentation fault with the following script​:

package Clock;
use 5.010;
use strict;

use overload
'+' => \&add_time,
'-' => \&sub_time,
'eq' => \&equals_to,
'""' => \&to_string;

sub equals_to{ $_[0] eq $_[1] }
sub to_string { sprintf '%02d​:%02d', $_[0][0] // 0, $_[0][1] // 0; }
sub new { bless( $_[1], $_[0] ); }

package main;
Clock->new([8,0]) eq "08​:00"
__END__

The SIGSEGV goes away when I change equals_to to​:

sub equals_to{ $_[0]->to_string eq $_[1] }

bye,

Danijel

Confirmed. Note that the problem is older than the perl-5.010 needed to make use of the '//' operator in sub to_string(). If we switch those to '||', we can reproduce the segfault going back at least to perl 5.6. See attached.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Dec 22, 2017

From @jkeenan

132638-double-bar.pl

@p5pRT
Copy link
Author

p5pRT commented Dec 22, 2017

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

@p5pRT
Copy link
Author

p5pRT commented Dec 23, 2017

From zefram@fysh.org

Danijel Tasov wrote​:

use overload
'eq' => \&equals_to,
...
sub equals_to{ $_[0] eq $_[1] }

Here's your problem you've defined the "eq" overload to apply "eq" to
its operands, which invokes the "eq" overload in an infinite recursion.
The segv is just C stack overflow. You probably want to write the "eq"
overload as

  sub equals_to{ "$_[0]" eq $_[1] }

or just remove that overload and enable overload fallback, so that Perl
will synthesise that "eq" overload for you.

There is no Perl bug here.

-zefram

@p5pRT
Copy link
Author

p5pRT commented Dec 23, 2017

From @jkeenan

On Sat, 23 Dec 2017 07​:56​:23 GMT, zefram@​fysh.org wrote​:

Danijel Tasov wrote​:

use overload
'eq' => \&equals_to,
...
sub equals_to{ $_[0] eq $_[1] }

Here's your problem you've defined the "eq" overload to apply "eq" to
its operands, which invokes the "eq" overload in an infinite recursion.
The segv is just C stack overflow. You probably want to write the "eq"
overload as

sub equals\_to\{ "$\_\[0\]" eq $\_\[1\] \}

or just remove that overload and enable overload fallback, so that Perl
will synthesise that "eq" overload for you.

There is no Perl bug here.

-zefram

There may not be a bug in perl here -- but there's certainly a deficiency in our documentation.

I've been writing Perl for a long time but have never had occasion to 'use overload'. I stared at the original poster's code and couldn't see anything obviously wrong. I then went and read the documentation. Again, I couldn't find anything that would lead me to guess that the OP's code would segfault.

So we have to improve the documentation. Otherwise, people will repeatedly stumble into this problem. How should we change the docs?

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Dec 23, 2017

From @eserte

"James E Keenan via RT" <perlbug-followup@​perl.org> writes​:

On Sat, 23 Dec 2017 07​:56​:23 GMT, zefram@​fysh.org wrote​:

Danijel Tasov wrote​:

use overload
'eq' => \&equals_to,
...
sub equals_to{ $_[0] eq $_[1] }

Here's your problem you've defined the "eq" overload to apply "eq" to
its operands, which invokes the "eq" overload in an infinite recursion.
The segv is just C stack overflow. You probably want to write the "eq"
overload as

sub equals\_to\{ "$\_\[0\]" eq $\_\[1\] \}

or just remove that overload and enable overload fallback, so that Perl
will synthesise that "eq" overload for you.

There is no Perl bug here.

-zefram

There may not be a bug in perl here -- but there's certainly a
deficiency in our documentation.

I've been writing Perl for a long time but have never had occasion to
'use overload'. I stared at the original poster's code and couldn't
see anything obviously wrong. I then went and read the documentation.
Again, I couldn't find anything that would lead me to guess that the
OP's code would segfault.

So we have to improve the documentation. Otherwise, people will
repeatedly stumble into this problem. How should we change the docs?

With "use warnings" the script emits

  Useless use of string eq in void context at ...
  Deep recursion on subroutine "Clock​::equals_to" at ...

At least the "deep recursion" warning should be a note to the
experienced programmer that something's going wrong here, and out of
memory errors and stack overflows are to be expected.

But maybe a sentence in perldiag.pod could be added.

Regards,
  Slaven

--
Slaven Rezic - slaven <at> rezic <dot> de

  Berlin Perl Mongers - http​://berlin.pm.org

@p5pRT
Copy link
Author

p5pRT commented Dec 23, 2017

From gm@qwurx.de

From the keyboard of Zefram [23.12.17,07​:56]​:

Danijel Tasov wrote​:

use overload
'eq' => \&equals_to,
...
sub equals_to{ $_[0] eq $_[1] }

Here's your problem you've defined the "eq" overload to apply "eq" to
its operands, which invokes the "eq" overload in an infinite recursion.

That's right; on my machine that segfaults at recursion 26186, gdb bt
shows 104775 stack frames.

The segv is just C stack overflow. You probably want to write the "eq"
overload as

sub equals_to{ "$_[0]" eq $_[1] }

or just remove that overload and enable overload fallback, so that Perl
will synthesise that "eq" overload for you.

There is no Perl bug here.

You're probably right here, but​:
Why should a buffer overflow be a bug but a stack overflow should not?

I guess this could be handled more gracefully by determining the stack
limit before runtime and die with an appropriate message, instead of
running into a SIGSEGV.

Is it feasible to implement an overloading short-circuit detection?

best regards,
0--gg-

--
_($_=" "x(1<<5)."?\n".q·/)Oo. G°\ /
  /\_¯/(q /
---------------------------- \__(m.====·.(_("always off the crowd"))."·
");sub _{s./.($e="'Itrs `mnsgdq Gdbj O`qkdq")=~y/"-y/#-z/;$e.e && print}

@p5pRT
Copy link
Author

p5pRT commented Dec 23, 2017

From zefram@fysh.org

shmem wrote​:

Why should a buffer overflow be a bug but a stack overflow should not?

For two reasons. Firstly, buffer overflow is always an implementation
mistake​: Perl doesn't expose fixed-size buffers that the user can
overflow. Whereas stack overflow is a natural result of the Perl
program​: it is the user that coded an infinite recursion. Secondly,
buffer overflow can cause all kinds of erroneous behaviour, often
exploitable to defeat security measures. Whereas stack overflow just
causes a segv that terminates the program.

It would be nicer if deep recursion that currently occurs on the C stack
were to be able to use more of the available memory, and eventually fail
in some slightly cleaner way that cites lack of memory. Simple cases
of Perl recursion do avoid the C stack and get such behaviour. But it
would be quite infeasible to avoid C stack recursion in all cases, and
very difficult to give C stack recursion nicer behaviour. So we have
made a tacit design decision that we put up with the limited C stack
size and segv as the result of overflowing it, and thus we let that be
the behaviour we offer to Perl programs.

-zefram

@p5pRT
Copy link
Author

p5pRT commented Dec 23, 2017

From @cpansprout

On Sat, 23 Dec 2017 07​:42​:52 -0800, zefram@​fysh.org wrote​:

So we have
made a tacit design decision that we put up with the limited C stack
size and segv as the result of overflowing it, and thus we let that be
the behaviour we offer to Perl programs.

This is the first I’ve heard of it being a design decision, rather than an unfortunate unfixable flaw.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Dec 24, 2017

From gm@qwurx.de

From the keyboard of Zefram [23.12.17,15​:42]​:

shmem wrote​:

Why should a buffer overflow be a bug but a stack overflow should not?

For two reasons. Firstly, buffer overflow is always an implementation
mistake​: Perl doesn't expose fixed-size buffers that the user can
overflow. Whereas stack overflow is a natural result of the Perl
program​: it is the user that coded an infinite recursion. Secondly,
buffer overflow can cause all kinds of erroneous behaviour, often
exploitable to defeat security measures. Whereas stack overflow just
causes a segv that terminates the program.

Arguably the buffer overflow is an implementation mistake of the C
language implementation, which is known and must carefully be avoided.
C places the burden on the programmer here, as it does with stack
overflow. Perl has made sure buffer overflows don't occur, whereas wrt
stack overflows, perl doesn't care & passes the burden to the perl user.

It would be nicer if deep recursion that currently occurs on the C stack
were to be able to use more of the available memory, and eventually fail
in some slightly cleaner way that cites lack of memory. Simple cases
of Perl recursion do avoid the C stack and get such behaviour. But it
would be quite infeasible to avoid C stack recursion in all cases, and
very difficult to give C stack recursion nicer behaviour. So we have
made a tacit design decision that we put up with the limited C stack
size and segv as the result of overflowing it, and thus we let that be
the behaviour we offer to Perl programs.

It is not a design decision of perl, because it is not perl which emits
the SIGSEGV, it is the kernel - at least on Linux, and I bet it is no
different with other unixish sytems. The C stack size depends on the
current ulimit values which - again, on Linux - can be retrieved with
the getrlimit(2) syscall, and the current stack size can be obtained via
getrusage(2), if I am not mistaken altogether.

I know far too little of the perl internals to be able to tell at which
conditions a stack check along the lines of the above would be nice.

An alternative would be to set up an alternate signal stack as described
in getrlimit(2) and sigaltstack(2) to be able to mask SIGSEGV and make an
educated guess about where the condition occured, which could lead to
better error messages whenever a SIGSEGV is triggered.

The current stack size limit is a perl runtime condition. If I run the
short-circuited overload code in a root shell having set "ulimit -s
unlimited", I can sit and wait until the C stack has gobbled up all
memory (and swap) until the kernel finally emits an emergency kill (or
not, I didn't wait).

Running it with my current ulimit settings (-s being 8192 kbytes) it
segfaults at recursion 26196; if I decrease the stack size limit to 64
kbytes, it is blown off at recursion 171.

The simple recursion 'sub r{&r} r' instead seems to make the heap grow,
which is much harder to handle wrt limits.

Does that make sense?

best regards,
0--gg-

--
_($_=" "x(1<<5)."?\n".q·/)Oo. G°\ /
  /\_¯/(q /
---------------------------- \__(m.====·.(_("always off the crowd"))."·
");sub _{s./.($e="'Itrs `mnsgdq Gdbj O`qkdq")=~y/"-y/#-z/;$e.e && print}

@rjbs rjbs changed the title I've discovered a segfault recursion in overload sub results in segfault Feb 28, 2021
@xenu xenu removed the affects-5.26 label Nov 19, 2021
@xenu xenu removed the Severity Low label Dec 29, 2021
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

2 participants