Branch: refs/heads/yves/fix_av_store_magic_call
  Home:   https://github.com/Perl/perl5
  Commit: 450b1dcaf2694102c33a1179d2320bf577b290da
      
https://github.com/Perl/perl5/commit/450b1dcaf2694102c33a1179d2320bf577b290da
  Author: Yves Orton <[email protected]>
  Date:   2023-01-05 (Thu, 05 Jan 2023)

  Changed paths:
    M av.c
    M ext/XS-APItest/APItest.xs
    M ext/XS-APItest/t/magic.t

  Log Message:
  -----------
  av.c - av_store() do the refcount dance around magic av's

The api for av_store() says it is the callers responsibility to call
SvREFCNT_inc() on the stored item after the store is successful.

However inside of av_store() we store the item in the C level array before we
trigger magic. To a certain extent this is required because mg_set(av) needs
to be able to see the newly stored item.

But if the mg_set() or other magic associated with the av_store() operation
fails, we would end up with a double free situation, as we will long jump up
the stack above and out of the call to av_store(), freeing the mortal as we go
(via Perl_croak()), but leaving the reference to the now freed pointer in the
array. When the next SV is allocated the reference will be reused, and then we
are in a double free scenario.

I see comments in pp_aassign talking about defusing the temps stack for the
parameters it is passing in, and things like this, which at first looked
related. But that commentary doesn't seem that relevant to me, as this bug
could happen any time a scalar owned by one data structure was copied into an
array with set magic which could die. Eg, I can easily imagine XS code that
expected code like this (assume it handles magic properly) to work:

    SV **svp = av_fetch(av1,0,1);
    if (av_store(av2,0,*svp))
        SvREFCNT_inc(*svp);

but if av2 has set magic and it dies the end result will be that both av1 and
av2 contain a visible reference to *svp, but its refcount will be 1. So I think
this is a bug regardless of what pp_aassign does.

This fixes https://github.com/Perl/perl5/issues/20675


Reply via email to