avoid leak with local $h{foo}, $a[n]
authorDavid Mitchell <davem@iabyn.com>
Tue, 26 Mar 2019 11:04:07 +0000 (11:04 +0000)
committerDavid Mitchell <davem@iabyn.com>
Tue, 26 Mar 2019 11:04:07 +0000 (11:04 +0000)
When SAVEt_DELETE / SAVEt_ADELETE deletes a hash/array entry on scope
exit, they also decrement the refcount of the hash/array, and for the
hash, also free the saved key.

However, if the call to hv_delete() or av_delete() dies (e.g. when
calling a tied DELETE method) then the hash/array and key will leak
because leave_scope() calls av/hv_delete(), *then* does the
SvREFCNT_dec() etc.

The fix is to push new FREEPV/FREESV actions just before calling
av/hv_delete().

scope.c
t/op/tie.t

diff --git a/scope.c b/scope.c
index 83a7b76..3e4ee43 100644 (file)
--- a/scope.c
+++ b/scope.c
@@ -1253,15 +1253,26 @@ Perl_leave_scope(pTHX_ I32 base)
 
        case SAVEt_DELETE:
             a0 = ap[0]; a1 = ap[1]; a2 = ap[2];
+            /* hv_delete could die, so free the key and SvREFCNT_dec the
+             * hv by pushing new save actions
+             */
+            /* ap[0] is the key */
+            ap[1].any_uv = SAVEt_FREEPV; /* was len */
+            /* ap[2] is the hv */
+            ap[3].any_uv = SAVEt_FREESV; /* was SAVEt_DELETE */
+            PL_savestack_ix += 4;
            (void)hv_delete(a2.any_hv, a0.any_pv, a1.any_i32, G_DISCARD);
-           SvREFCNT_dec(a2.any_hv);
-           Safefree(a0.any_ptr);
            break;
 
        case SAVEt_ADELETE:
             a0 = ap[0]; a1 = ap[1];
+            /* av_delete could die, so SvREFCNT_dec the av by pushing a
+             * new save action
+             */
+            ap[0].any_av = a1.any_av;
+            ap[1].any_uv = SAVEt_FREESV;
+            PL_savestack_ix += 2;
            (void)av_delete(a1.any_av, a0.any_iv, G_DISCARD);
-           SvREFCNT_dec(a1.any_av);
            break;
 
        case SAVEt_DESTRUCTOR_X:
index a2d771a..bfcafce 100644 (file)
@@ -1585,3 +1585,40 @@ print "[$x][$f][$n][$s]\n";
 EXPECT
 [3][1][3][0]
 [0][2][3][0]
+########
+# dying while doing a SAVEt_DELETE dureing scope exit leaked a copy of the
+# key. Give ASan something to play with
+sub TIEHASH { bless({}, $_[0]) }
+sub EXISTS { 0 }
+sub DELETE { die; }
+sub DESTROY { print "destroy\n"; }
+
+eval {
+    my %h;
+    tie %h, "main";
+    local $h{foo};
+    print "leaving\n";
+};
+print "left\n";
+EXPECT
+leaving
+destroy
+left
+########
+# ditto for SAVEt_DELETE with an array
+sub TIEARRAY { bless({}, $_[0]) }
+sub EXISTS { 0 }
+sub DELETE { die; }
+sub DESTROY { print "destroy\n"; }
+
+eval {
+    my @a;
+    tie @a, "main";
+    delete local $a[0];
+    print "leaving\n";
+};
+print "left\n";
+EXPECT
+leaving
+destroy
+left