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
null pointer deref Perl_mess_sv (util.c:1508) #15538
Comments
From @geeknikThe following script triggers a null pointer deref (Perl_mess_sv (util.c:1508)) in Perl v5.25.4 (v5.25.3-305-g8c6b0c7). May only affect -DDEBUGGING builds because removing the `D` flag stifles the crash. #!perl -D200000 free op at 61900000db00, recorded in slab 61500000fa80 at (eval 1) line 1.
|
From @cpansproutOn Sat Aug 20 21:53:50 2016, brian.carpenter@gmail.com wrote:
Equivalent to -DTSuxfP, but -DS alone is sufficient.
Reduced and slightly readable: ./miniperl -DS -e '$_="f"; s/./"&".$&/ee' The string assigned to $_ can be anything with length to it. -- Father Chrysostomos |
The RT System itself - Status changed from 'new' to 'open' |
From riusksk@qq.comgdb-peda$ run poc.pl root@Ubuntu:~/perl# valgrind perl poc.pl ==22051== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0) |
From riusksk@qq.com |
From @dcollinsnLooks to me like the following patch is enough to resolve this issue. Valgrind is clean, and no failures with make test. From 76d06fab84e6fdc64a1c24a335bbd3f40ee4a32e Mon Sep 17 00:00:00 2001 util.c | 3 +++ Inline Patchdiff --git a/util.c b/util.c
index 7748c6c..17dd69c 100644
--- a/util.c
+++ b/util.c
@@ -1504,6 +1504,7 @@ Perl_mess_sv(pTHX_ SV *basemsg, bool consume)
* from the sibling of PL_curcop.
*/
+ if (PL_curcop) {
const COP *cop =
closest_cop(PL_curcop, OpSIBLING(PL_curcop), PL_op, FALSE);
if (!cop)
@@ -1512,6 +1513,8 @@ Perl_mess_sv(pTHX_ SV *basemsg, bool consume)
if (CopLINE(cop))
Perl_sv_catpvf(aTHX_ sv, " at %s line %"IVdf,
OutCopFILE(cop), (IV)CopLINE(cop));
+ }
+
/* Seems that GvIO() can be untrustworthy during global destruction. */
if (GvIO(PL_last_in_gv) && (SvTYPE(GvIOp(PL_last_in_gv)) == SVt_PVIO)
&& IoLINES(GvIOp(PL_last_in_gv)))
--
Thoughts? (Should there be a PL_curcop here, or is it OK for it to be null? I found this old thread which may be relevant: http://www.nntp.perl.org/group/perl.perl5.porters/2013/08/msg205604.html ) -- |
From @dcollinsnThis appears to be a very complicated duplicate of RT #129027. This one spews debug output for about 30 seconds, and it doesn't actually segfault for me. It's a null deref, the pointer is correctly zeroed and then incorrectly used (it's dereferencing 0x21 because that's the offset into the struct), so not a security bug. My patch in that ticket should also fix this, but I'm not sure because I can't actually repro. -- |
The RT System itself - Status changed from 'new' to 'open' |
From @cpansproutOn Thu Aug 25 06:58:45 2016, dcollinsn@gmail.com wrote:
Yes, it is relevant. The backtrace shows that the PL_curcop is being read when ops are being freed. One of those ops is almost certainly a cop (too lazy to check) which happened to be PL_curcop, so freeing it set PL_curcop to null. I do wonder, though, whether we should be adding more conditions to normal code for the sake of debugging code. That said, errors and warnings are certainly not hot code, so your fix, the simpler fix, is probably fine. (gdb) bt -- Father Chrysostomos |
From @cpansproutOn Thu Aug 25 13:43:19 2016, sprout wrote:
This jogged my memory a bit.... When I added -DS, I did wonder whether it was appropriate to use SV-using diagnostic code for such low-level functionality. I decided not to worry about it, since I doubted that anyone would be using that option except during the first few months after the op slab allocator was added. It seems that the misgivings that I ignored were correct. I do think a more appropriate fix would be to change DEBUG_S_warn in op.c to be smarter and bypass mess_sv and anything SVish. It’s more work though. Fixing anything related to -D is certainly not high priority. -- Father Chrysostomos |
From riusksk@qq.com在2016-八月-25 07:16:34 星期四时,dcollinsn@gmail.com写到:
ok,thx, close it. |
From @khwilliamsonOn 08/25/2016 07:50 PM, riusksk via RT wrote:
Instead of closing it, I merged it into #129027. You will then get |
From @atoomica simpler way to patch this would be to add a protection on the PL_curcop Inline Patchdiff --git a/util.c b/util.c
index a69ddad..a1e44b4 100644
--- a/util.c
+++ b/util.c
@@ -1522,7 +1522,7 @@ Perl_mess_sv(pTHX_ SV *basemsg, bool consume)
sv_copypv(sv, basemsg);
}
- if (!SvCUR(sv) || *(SvEND(sv) - 1) != '\n') {
+ if (PL_curcop && (!SvCUR(sv) || *(SvEND(sv) - 1) != '\n')) {
/*
* Try and find the file and line for PL_op. This will usually be
* PL_curcop, but it might be a cop that has been optimised away. We
|
From @atoomic0001-protect-Perl_mess_sv-against-undefined-PL_curcop.patchFrom 3a2d2f9878ea8751e32098b5301c342b74013600 Mon Sep 17 00:00:00 2001
From: Nicolas R <atoomic@cpan.org>
Date: Mon, 12 Sep 2016 09:26:39 -0600
Subject: [PATCH 1/1] protect Perl_mess_sv against undefined PL_curcop
---
util.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/util.c b/util.c
index a69ddad..a1e44b4 100644
--- a/util.c
+++ b/util.c
@@ -1522,7 +1522,7 @@ Perl_mess_sv(pTHX_ SV *basemsg, bool consume)
sv_copypv(sv, basemsg);
}
- if (!SvCUR(sv) || *(SvEND(sv) - 1) != '\n') {
+ if (PL_curcop && (!SvCUR(sv) || *(SvEND(sv) - 1) != '\n')) {
/*
* Try and find the file and line for PL_op. This will usually be
* PL_curcop, but it might be a cop that has been optimised away. We
--
2.10.0
|
This was fixed by
|
Migrated from rt.perl.org#129027 (status was 'open')
Searchable as RT129027$
The text was updated successfully, but these errors were encountered: