-
Notifications
You must be signed in to change notification settings - Fork 560
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
Segmentation fault in Perl_pp_multiply (and other functions) #14902
Comments
From @dcollinsnGreetings Porters, I have compiled bleadperl with the afl-gcc compiler using: ./Configure -Dusedevel -Dprefix='/usr/local/perl-afl' -Dcc=afl-gcc -Duselongdouble -Duse64bitint -Doptimize=-g -Uversiononly -Uman1dir -Uman3dir -des And then fuzzed the resulting binary using: AFL_NO_VAR_CHECK=1 afl-fuzz -i in -o out bin/perl @@ After reducing testcases using `afl-tmin` and filtering out testcases that are merely iterations of "#!perl -u", I have located the following testcase that triggers a segmentation fault in the perl interpreter. The testcase is the 15-byte file: &{0==0}*&{0==0} Several other variations of this failed, but this is the simplest example. All have either two calls to "&{0==0}" or nested calls of that type "&{0==&{0==0}}". This looks vaguely similar to the bugs arising from the stack not being refcounted (c.f. 125840, 123710, 123804, 123997, but the failure point is different. Unfortunately, no valgrind is available, as I haven't yet had a chance to upgrade to a version of valgrind that's aware of dwarf 4 and I'm off to work. If you for whatever reason are unable to repro this, I'll try to add updated information early this week. **GDB** GNU gdb (GDB) 7.10 Program received signal SIGSEGV, Segmentation fault. **PERL -V** Characteristics of this binary (from libperl): |
From @dcollinsn%% VALGRIND OUTPUT FOR CANONICAL TESTCASE ABOVE %% ==6140== Memcheck, a memory error detector |
From @dcollinsnFor fun, some other examples of failing testcases after afl-tmin: &{0==&{0==0}}.0 |
From @cpansproutOn Sat Sep 12 05:36:20 2015, dcollinsn@gmail.com wrote:
No, it’s more like stack corruption (perl getting confused about where it’s stack pointer should be): $ ./miniperl -Ilib -le 'print 1, 2, 3,&{0==0}*&{0==0}, 4, 5, 6' -- Father Chrysostomos |
The RT System itself - Status changed from 'new' to 'open' |
From @cpansproutOn Sun Sep 20 15:22:29 2015, sprout wrote:
Specifically, it is related to the fact that PL_sv_yes->() silently does nothing, but its doing nothing is badly implemented. This whole PL_sv_yes call thing is wart resulting from implementation details. I wonder if we can just remove it from perl space. Surely nobody is relying on it! (Famous last words.) -- Father Chrysostomos |
From zefram@fysh.orgFather Chrysostomos via RT wrote:
We certainly should, because its exposure per se produces visible bugs: $ perl -lwe '*1 = sub { print @_; }; &1(2); &{!!1}(3)' Even if its do-nothing behaviour were implemented correctly, it's wrong -zefram |
From @tonycozOn Sat Sep 12 05:36:20 2015, dcollinsn@gmail.com wrote:
The attached fixes the handling of calls to &PL_sv_yes. I'm not sure how to approach eliminating the wart. Tony |
From @tonycoz0001-perl-126042-handle-scalar-context-for-a-missing-impo.patchFrom d3e254d742fc2792ecc5e19a1fa4f1a3fa79f57b Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Wed, 4 Nov 2015 11:03:36 +1100
Subject: [perl #126042] handle scalar context for a missing import/unimport
call
---
pp_hot.c | 2 ++
t/op/method.t | 19 ++++++++++++++++++-
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/pp_hot.c b/pp_hot.c
index 87e306c..366828a 100644
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -3275,6 +3275,8 @@ PP(pp_entersub)
SP = PL_stack_base + POPMARK;
else
(void)POPMARK;
+ if (GIMME_V == G_SCALAR)
+ PUSHs(&PL_sv_undef);
RETURN;
}
SvGETMAGIC(sv);
diff --git a/t/op/method.t b/t/op/method.t
index 1171f4a..0d7f254 100644
--- a/t/op/method.t
+++ b/t/op/method.t
@@ -13,7 +13,7 @@ BEGIN {
use strict;
no warnings 'once';
-plan(tests => 147);
+plan(tests => 148);
@A::ISA = 'B';
@B::ISA = 'C';
@@ -657,6 +657,23 @@ SKIP: {
like ($@, qr/Modification of a read-only value attempted/, 'RT #123619');
}
+{
+ # RT #126042 &{1==1} * &{1==1} would crash
+
+ # pp_entersub and pp_method_named cooperate to prevent calls to an
+ # undefined import() or unimport() method from croaking.
+ # If pp_method_named can't find the method it pushes &PL_sv_yes, and
+ # pp_entersub checks for that specific SV to avoid croaking.
+ # Ideally they wouldn't use that hack but...
+ # Unfortunately pp_entersub's handling of that case is broken in scalar context.
+
+ # Rather than using the test case from the ticket, since &{1==1}
+ # isn't documented (and may not be supported in future perls) test
+ # calls to undefined import method, which also crashes.
+ fresh_perl_is('Unknown->import() * Unknown->unimport(); print "ok\n"', "ok\n", {},
+ "check unknown import() methods don't corrupt the stack");
+}
+
__END__
#FF9900
#F78C08
--
2.1.4
|
From @bulk88On Tue Nov 03 16:17:43 2015, tonyc wrote:
That fix looks like it doesn't fix the problem. It needs to be fixed in the optree at compile tile, that multiply operator takes 1 argument, not empty list. There might be other ways to get rh and lh operands of multiply to be empty list and cause the same crash again without calling empty sub references (PL_sv_yes in this case). -- |
From zefram@fysh.orgbulk88 via RT wrote:
Any such way would be another bug. The operands of the multiply op -zefram |
From @tonycozOn Tue Nov 03 16:17:43 2015, tonyc wrote:
Applied as 0cd52e2.
Leaving the ticket open in case someone figures that out. Tony |
From @iabynOn Wed, Jan 06, 2016 at 09:28:17PM -0800, Tony Cook via RT wrote:
Maybe make the method-locating code return a stub XS CV rather than -- |
From zefram@fysh.orgDave Mitchell wrote:
Done in commit 0740a29. That resolves -zefram |
@iabyn - Status changed from 'open' to 'pending release' |
From @khwilliamsonThank you for filing this report. You have helped make Perl better. With the release yesterday of Perl 5.28.0, this and 185 other issues have been Perl 5.28.0 may be downloaded via: If you find that the problem persists, feel free to reopen this ticket. |
@khwilliamson - Status changed from 'pending release' to 'resolved' |
Migrated from rt.perl.org#126042 (status was 'resolved')
Searchable as RT126042$
The text was updated successfully, but these errors were encountered: