From 339441efdfcbaef5dbf131db2224abb35f5ae920 Mon Sep 17 00:00:00 2001 From: David Mitchell Date: Fri, 7 Mar 2014 17:45:27 +0000 Subject: [PATCH] make core safe against HvAUX() realloc Since the HvAUX structure is just tacked onto the end of the HvARRAY() struct, code like this can do bad things: aux = HvAUX(); ... something that might split hv ... aux->foo = ...; /* SEGV! */ So I've visually audited core for places where HbAUX() is saved and then re-used, and re-initialised the var if it looks like HvARRAY() could have changed in the meantime. I've been very conservative about what might be unsafe. For example, destructors or __WARN__ handlers could call perl code that modifies the hash. --- gv.c | 10 +++------- hv.c | 19 +++++++++++++------ 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/gv.c b/gv.c index ec05284..e402f6b 100644 --- a/gv.c +++ b/gv.c @@ -2263,7 +2263,6 @@ Perl_gv_check(pTHX_ HV *stash) { dVAR; I32 i; - struct xpvhv_aux *aux; PERL_ARGS_ASSERT_GV_CHECK; @@ -2271,12 +2270,11 @@ Perl_gv_check(pTHX_ HV *stash) return; assert(SvOOK(stash)); - aux = HvAUX(stash); for (i = 0; i <= (I32) HvMAX(stash); i++) { const HE *entry; /* mark stash is being scanned, to avoid recursing */ - aux->xhv_aux_flags |= HvAUXf_SCAN_STASH; + HvAUX(stash)->xhv_aux_flags |= HvAUXf_SCAN_STASH; for (entry = HvARRAY(stash)[i]; entry; entry = HeNEXT(entry)) { GV *gv; HV *hv; @@ -2310,7 +2308,7 @@ Perl_gv_check(pTHX_ HV *stash) HEKfARG(GvNAME_HEK(gv))); } } - aux->xhv_aux_flags &= ~HvAUXf_SCAN_STASH; + HvAUX(stash)->xhv_aux_flags &= ~HvAUXf_SCAN_STASH; } } @@ -2493,7 +2491,6 @@ Perl_Gv_AMupdate(pTHX_ HV *stash, bool destructing) { int filled = 0; int i; - struct xpvhv_aux *aux; bool deref_seen = 0; @@ -2527,9 +2524,8 @@ Perl_Gv_AMupdate(pTHX_ HV *stash, bool destructing) } assert(SvOOK(stash)); - aux = HvAUX(stash); /* initially assume the worst */ - aux->xhv_aux_flags &= ~HvAUXf_NO_DEREF; + HvAUX(stash)->xhv_aux_flags &= ~HvAUXf_NO_DEREF; for (i = 1; i < NofAMmeth; i++) { const char * const cooky = PL_AMG_names[i]; diff --git a/hv.c b/hv.c index a4624f0..91d4c39 100644 --- a/hv.c +++ b/hv.c @@ -1687,6 +1687,7 @@ Perl_hfree_next_entry(pTHX_ HV *hv, STRLEN *indexp) /* warning: at this point HvARRAY may have been * re-allocated, HvMAX changed etc */ } + iter = HvAUX(hv); /* may have been realloced */ iter->xhv_riter = -1; /* HvRITER(hv) = -1 */ iter->xhv_eiter = NULL; /* HvEITER(hv) = NULL */ #ifdef PERL_HASH_RANDOMIZE_KEYS @@ -1785,7 +1786,6 @@ Perl_hv_undef_flags(pTHX_ HV *hv, U32 flags) } hfreeentries(hv); if (SvOOK(hv)) { - struct xpvhv_aux * const aux = HvAUX(hv); struct mro_meta *meta; const char *name; @@ -1802,7 +1802,7 @@ Perl_hv_undef_flags(pTHX_ HV *hv, U32 flags) /* If this call originated from sv_clear, then we must check for * effective names that need freeing, as well as the usual name. */ name = HvNAME(hv); - if (flags & HV_NAME_SETALL ? !!aux->xhv_name_u.xhvnameu_name : !!name) { + if (flags & HV_NAME_SETALL ? !!HvAUX(hv)->xhv_name_u.xhvnameu_name : !!name) { if (name && PL_stashcache) { DEBUG_o(Perl_deb(aTHX_ "hv_undef_flags clearing PL_stashcache for name '%" HEKf"'\n", HvNAME_HEK(hv))); @@ -1810,7 +1810,7 @@ Perl_hv_undef_flags(pTHX_ HV *hv, U32 flags) } hv_name_set(hv, NULL, 0, flags); } - if((meta = aux->xhv_mro_meta)) { + if((meta = HvAUX(hv)->xhv_mro_meta)) { if (meta->mro_linear_all) { SvREFCNT_dec_NN(meta->mro_linear_all); /* mro_linear_current is just acting as a shortcut pointer, @@ -1824,9 +1824,9 @@ Perl_hv_undef_flags(pTHX_ HV *hv, U32 flags) SvREFCNT_dec(meta->isa); SvREFCNT_dec(meta->super); Safefree(meta); - aux->xhv_mro_meta = NULL; + HvAUX(hv)->xhv_mro_meta = NULL; } - if (!aux->xhv_name_u.xhvnameu_name && ! aux->xhv_backreferences) + if (!HvAUX(hv)->xhv_name_u.xhvnameu_name && ! HvAUX(hv)->xhv_backreferences) SvFLAGS(hv) &= ~SVf_OOK; } if (!SvOOK(hv)) { @@ -2009,12 +2009,13 @@ Perl_hv_iterinit(pTHX_ HV *hv) Perl_croak(aTHX_ "Bad hash"); if (SvOOK(hv)) { - struct xpvhv_aux * const iter = HvAUX(hv); + struct xpvhv_aux * iter = HvAUX(hv); HE * const entry = iter->xhv_eiter; /* HvEITER(hv) */ if (entry && HvLAZYDEL(hv)) { /* was deleted earlier? */ HvLAZYDEL_off(hv); hv_free_ent(hv, entry); } + iter = HvAUX(hv); /* may have been reallocated */ iter->xhv_riter = -1; /* HvRITER(hv) = -1 */ iter->xhv_eiter = NULL; /* HvEITER(hv) = NULL */ #ifdef PERL_HASH_RANDOMIZE_KEYS @@ -2146,6 +2147,7 @@ Perl_hv_name_set(pTHX_ HV *hv, const char *name, U32 len, U32 flags) /* The first elem may be null. */ if(*name) unshare_hek_or_pvn(*name, 0, 0, 0); Safefree(name); + iter = HvAUX(hv); /* may been realloced */ spot = &iter->xhv_name_u.xhvnameu_name; iter->xhv_name_count = 0; } @@ -2167,6 +2169,7 @@ Perl_hv_name_set(pTHX_ HV *hv, const char *name, U32 len, U32 flags) } else if (flags & HV_NAME_SETALL) { unshare_hek_or_pvn(iter->xhv_name_u.xhvnameu_name, 0, 0, 0); + iter = HvAUX(hv); /* may been realloced */ spot = &iter->xhv_name_u.xhvnameu_name; } else { @@ -2311,6 +2314,7 @@ Perl_hv_ename_delete(pTHX_ HV *hv, const char *name, U32 len, U32 flags) : (HEK_LEN(*victim) == (I32)len && memEQ(HEK_KEY(*victim), name, len)) ) { unshare_hek_or_pvn(*victim, 0, 0, 0); + aux = HvAUX(hv); /* may been realloced */ if (count < 0) ++aux->xhv_name_count; else --aux->xhv_name_count; if ( @@ -2463,6 +2467,7 @@ Perl_hv_iternext_flags(pTHX_ HV *hv, I32 flags) SvREFCNT_dec(HeVAL(entry)); Safefree(HeKEY_hek(entry)); del_HE(entry); + iter = HvAUX(hv); /* may been realloced */ iter->xhv_eiter = NULL; /* HvEITER(hv) = NULL */ HvLAZYDEL_off(hv); return NULL; @@ -2509,6 +2514,7 @@ Perl_hv_iternext_flags(pTHX_ HV *hv, I32 flags) pTHX__FORMAT pTHX__VALUE); } + iter = HvAUX(hv); /* may been realloced */ iter->xhv_last_rand = iter->xhv_rand; } #endif @@ -2553,6 +2559,7 @@ Perl_hv_iternext_flags(pTHX_ HV *hv, I32 flags) hv_free_ent(hv, oldentry); } + iter = HvAUX(hv); /* may been realloced */ iter->xhv_eiter = entry; /* HvEITER(hv) = entry */ return entry; } -- 2.7.4