Bleadperl v5.31.2-54-g8c47b5bce7 breaks RIBASUSHI/DBIx-Class-0.082841.tar.gz

[p5pRT]

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

Searchable as RT134344$

[p5pRT]

From @andk

XRef​: https://rt.cpan.org/Ticket/Display.html?id=130271
--
andreas
PS​: perl, bleadperl, BBC

[p5pRT]

From @andk

Also affected​: SREZIC/Tk-804.034.tar.gz
  http​://www.cpantesters.org/cpan/report/dfd60ef0-b7b6-11e9-b621-3d22a536eef6

--
andreas

[p5pRT]

From @andk

Also affected​: SKAJI/Text-Xslate-v3.5.6.tar.gz
  http​://www.cpantesters.org/cpan/report/5c554182-b7b5-11e9-bbe7-78faa436eef6

--
andreas

[p5pRT]

From @nwc10

On Thu, Aug 08, 2019 at 03​:21​:58PM +0200, Andreas Koenig wrote​:

Also affected​: SKAJI/Text-Xslate-v3.5.6.tar.gz
http​://www.cpantesters.org/cpan/report/5c554182-b7b5-11e9-bbe7-78faa436eef6

commit 8c47b5b
Author​: David Mitchell <davem@​iabyn.com>
Date​: Tue Jul 16 16​:14​:58 2019 +0100

  OPSLOT​: replace opslot_next with opslot_size

  Currently, each allocated opslot has a pointer to the opslot that was
  allocated immediately above it. Replace this with a U16 opslot_size field
  giving the size of the opslot. The next opslot can then be found by
  adding slot->opslot_size * sizeof(void*) to slot.

  This saves space.

ASAN is very excited (blead at 21dce8f)​:

:~/Perl/p5-Text-Xslate$ ~/Sandpit/snap-v5.31.2-65-g21dce8f4eb-ASAN/bin/perl5.31.3 -T -Mblib t/010_internals/028_taint.t

==30795==ERROR​: AddressSanitizer​: heap-buffer-overflow on address 0x6210002c8870 at pc 0x55a1eacf4e53 bp 0x7ffe0a628f50 sp 0x7ffe0a628f40
READ of size 8 at 0x6210002c8870 thread T0
  #0 0x55a1eacf4e52 in Perl_op_free /home/nick/Perl/perl/op.c​:864
  #1 0x55a1eb089c78 in Perl_leave_scope /home/nick/Perl/perl/scope.c​:1127
  #2 0x55a1eb0981a5 in S_pop_eval_context_maybe_croak /home/nick/Perl/perl/pp_ctl.c​:1633
  #3 0x55a1eb0d4981 in Perl_pp_leaveeval /home/nick/Perl/perl/pp_ctl.c​:4555
  #4 0x55a1eaf01914 in Perl_runops_debug /home/nick/Perl/perl/dump.c​:2557
  #5 0x55a1ead55f95 in Perl_call_sv /home/nick/Perl/perl/perl.c​:3039
  #6 0x55a1ead5f1cf in Perl_call_list /home/nick/Perl/perl/perl.c​:5080
  #7 0x55a1eacf0864 in S_process_special_blocks /home/nick/Perl/perl/op.c​:10803
  #8 0x55a1ead41135 in Perl_newATTRSUB_x /home/nick/Perl/perl/op.c​:10728
  #9 0x55a1ead464e0 in Perl_utilize /home/nick/Perl/perl/op.c​:7896
  #10 0x55a1eae23420 in Perl_yyparse /home/nick/Perl/perl/perly.y​:346
  #11 0x55a1eb09a407 in S_doeval_compile /home/nick/Perl/perl/pp_ctl.c​:3502
  #12 0x55a1eb0bbbe9 in S_require_file /home/nick/Perl/perl/pp_ctl.c​:4322
  #13 0x55a1eb0bdaa4 in Perl_pp_require /home/nick/Perl/perl/pp_ctl.c​:4346
  #14 0x55a1eaf01914 in Perl_runops_debug /home/nick/Perl/perl/dump.c​:2557
  #15 0x55a1ead55f95 in Perl_call_sv /home/nick/Perl/perl/perl.c​:3039
  #16 0x55a1ead5f1cf in Perl_call_list /home/nick/Perl/perl/perl.c​:5080
  #17 0x55a1eacf0864 in S_process_special_blocks /home/nick/Perl/perl/op.c​:10803
  #18 0x55a1ead41135 in Perl_newATTRSUB_x /home/nick/Perl/perl/op.c​:10728
  #19 0x55a1ead464e0 in Perl_utilize /home/nick/Perl/perl/op.c​:7896
  #20 0x55a1eae23420 in Perl_yyparse /home/nick/Perl/perl/perly.y​:346
  #21 0x55a1ead6874f in S_parse_body /home/nick/Perl/perl/perl.c​:2527
  #22 0x55a1ead6a195 in perl_parse /home/nick/Perl/perl/perl.c​:1818
  #23 0x55a1eace6465 in main /home/nick/Perl/perl/perlmain.c​:126
  #24 0x7f5308618b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
  #25 0x55a1eace6249 in _start (/home/nick/Sandpit/snap-v5.31.2-65-g21dce8f4eb-ASAN/bin/perl5.31.3+0x18a249)

0x6210002c8870 is located 144 bytes to the left of 4096-byte region [0x6210002c8900,0x6210002c9900)
allocated by thread T0 here​:
  #0 0x7f53094bfd38 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded38)
  #1 0x55a1eacebfcc in S_new_slab /home/nick/Perl/perl/op.c​:240

SUMMARY​: AddressSanitizer​: heap-buffer-overflow /home/nick/Perl/perl/op.c​:864 in Perl_op_free
Shadow bytes around the buggy address​:
  0x0c42800510b0​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c42800510c0​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c42800510d0​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c42800510e0​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c42800510f0​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c4280051100​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa[fa]fa
  0x0c4280051110​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280051120​: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4280051130​: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4280051140​: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4280051150​: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes)​:
  Addressable​: 00
  Partially addressable​: 01 02 03 04 05 06 07
  Heap left redzone​: fa
  Freed heap region​: fd
  Stack left redzone​: f1
  Stack mid redzone​: f2
  Stack right redzone​: f3
  Stack after return​: f5
  Stack use after scope​: f8
  Global redzone​: f9
  Global init order​: f6
  Poisoned by user​: f7
  Container overflow​: fc
  Array cookie​: ac
  Intra object redzone​: bb
  ASan internal​: fe
  Left alloca redzone​: ca
  Right alloca redzone​: cb
==30795==ABORTING

(don't have any time to investigate further - need to get the bus to the
perlcon dinner)

Nicholas Clark

[p5pRT]

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

[p5pRT]

From @nwc10

On Thu, Aug 08, 2019 at 05​:16​:30PM +0100, Nicholas Clark wrote​:

ASAN is very excited (blead at 21dce8f)​:

:~/Perl/p5-Text-Xslate$ ~/Sandpit/snap-v5.31.2-65-g21dce8f4eb-ASAN/bin/perl5.31.3 -T -Mblib t/010_internals/028_taint.t

==30795==ERROR​: AddressSanitizer​: heap-buffer-overflow on address 0x6210002c8870 at pc 0x55a1eacf4e53 bp 0x7ffe0a628f50 sp 0x7ffe0a628f40
READ of size 8 at 0x6210002c8870 thread T0
#0 0x55a1eacf4e52 in Perl_op_free /home/nick/Perl/perl/op.c​:864
#1 0x55a1eb089c78 in Perl_leave_scope /home/nick/Perl/perl/scope.c​:1127

which is a bit strange because it seems that the memory access is somewhere
completely "wrong" with respect to actually allocated slabs.

0x6210002c8870 is located 144 bytes to the left of 4096-byte region [0x6210002c8900,0x6210002c9900)
allocated by thread T0 here​:
#0 0x7f53094bfd38 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded38)
#1 0x55a1eacebfcc in S_new_slab /home/nick/Perl/perl/op.c​:240

And to confirm, this failure case is not yet covered by any core regression
test, because we had​:

  All tests successful.
  Elapsed​: 2321 sec
  u=39.64 s=30.99 cu=1709.43 cs=358.73 scripts=2440 tests=1218647

I hope this is useful to others.
Sorry, won't have time to reduce this any further.

Nicholas Clark

[p5pRT]

From @iabyn

On Fri, Aug 09, 2019 at 07​:25​:05AM +0100, Nicholas Clark wrote​:

I hope this is useful to others.
Sorry, won't have time to reduce this any further.

I'm looking at it today

--
Please note that ash-trays are provided for the use of smokers,
whereas the floor is provided for the use of all patrons.
  -- Bill Royston

[p5pRT]

From @iabyn

On Fri, Aug 09, 2019 at 09​:18​:16AM +0100, Dave Mitchell wrote​:

On Fri, Aug 09, 2019 at 07​:25​:05AM +0100, Nicholas Clark wrote​:

I hope this is useful to others.
Sorry, won't have time to reduce this any further.

I'm looking at it today

Now fixed (and the three mentioned distributions are passing) with​:

commit 5d26d78
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Fri Aug 9 11​:11​:19 2019 +0100
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Fri Aug 9 11​:11​:19 2019 +0100

  fix size-miscalculation upgrading LISTOP TO LOOPOP
 
  RT #134344
 
  My recent commit v5.31.2-54-g8c47b5bce7 broke some CAN modules because
  the code in Perl_newFOROP() wasn't accounting for the overhead in the
  opslot struct when deciding whether an allocated LISTOP was large enough
  to be upgraded in-place to a LOOPOP.

Affected files ...
  M op.c

Differences ...

Inline Patch

diff --git a/op.c b/op.c
index 5d0b1dae3a..86251047b6 100644
--- a/op.c
+++ b/op.c
@@ -9287,7 +9287,8 @@ Perl_newFOROP(pTHX_ I32 flags, OP *sv, OP *expr, OP *block, OP *cont)
     /* upgrade loop from a LISTOP to a LOOPOP;
      * keep it in-place if there's space */
     if (loop->op_slabbed
-        && OpSLOT(loop)->opslot_size < SIZE_TO_PSIZE(sizeof(LOOP)))
+        &&    OpSLOT(loop)->opslot_size
+            < SIZE_TO_PSIZE(sizeof(LOOP)) + OPSLOT_HEADER_P)
     {
         /* no space; allocate new op */
 	LOOP *tmp;


-- 

"I do not resent criticism, even when, for the sake of emphasis,
it parts for the time with reality".
  -- Winston Churchill, House of Commons, 22nd Jan 1941.

[p5pRT]

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