|
| 1 | +From f1b6fa6bb53480921fb0083d9c8b583b8df2e904 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Tony Cook < [email protected]> |
| 3 | +Date: Mon, 24 Feb 2025 15:56:09 +1100 |
| 4 | +Subject: [PATCH 1/3] regcomp: handle cloning the rexc cleanup in the scope |
| 5 | + stack |
| 6 | + |
| 7 | +Previous on Win32 this could cause a double-free of the RExC state if |
| 8 | +an emulated fork was done with the free of the state on the scope |
| 9 | +stack. |
| 10 | + |
| 11 | +Use a custom save type and prevent freeing in the cloned process to |
| 12 | +prevent the double-free. |
| 13 | + |
| 14 | +Fixes #23022 |
| 15 | + |
| 16 | +XXX scope_types.h is elided from original patch and applied separately |
| 17 | +after regen as it did not patch cleanly. |
| 18 | +--- |
| 19 | + embed.fnc | 4 ++- |
| 20 | + embed.h | 1 + |
| 21 | + proto.h | 6 ++++ |
| 22 | + regcomp.c | 16 +++++++---- |
| 23 | + regen/scope_types.pl | 1 + |
| 24 | + scope.c | 6 ++++ |
| 25 | + scope.h | 5 ++++ |
| 26 | + scope_types.h | 68 +++++++++++++++++++++++--------------------- |
| 27 | + sv.c | 5 ++++ |
| 28 | + 9 files changed, 73 insertions(+), 39 deletions(-) |
| 29 | + |
| 30 | +diff --git a/embed.fnc b/embed.fnc |
| 31 | +index 0332f63875ec..90b518a03f0a 100644 |
| 32 | +--- a/embed.fnc |
| 33 | ++++ b/embed.fnc |
| 34 | +@@ -2791,6 +2791,9 @@ Cp |char * |re_intuit_start|NN REGEXP * const rx \ |
| 35 | + |NULLOK re_scream_pos_data *data |
| 36 | + Cp |SV * |re_intuit_string \ |
| 37 | + |NN REGEXP * const r |
| 38 | ++ |
| 39 | ++p |void |release_RExC_state \ |
| 40 | ++ |NN void *vstate |
| 41 | + Xp |REGEXP *|re_op_compile |NULLOK SV ** const patternp \ |
| 42 | + |int pat_count \ |
| 43 | + |NULLOK OP *expr \ |
| 44 | +@@ -2799,7 +2802,6 @@ Xp |REGEXP *|re_op_compile |NULLOK SV ** const patternp \ |
| 45 | + |NULLOK bool *is_bare_re \ |
| 46 | + |const U32 rx_flags \ |
| 47 | + |const U32 pm_flags |
| 48 | +- |
| 49 | + ATdp |void |repeatcpy |NN char *to \ |
| 50 | + |NN const char *from \ |
| 51 | + |SSize_t len \ |
| 52 | +diff --git a/embed.h b/embed.h |
| 53 | +index e1e2bf4d02f1..d6926854177b 100644 |
| 54 | +--- a/embed.h |
| 55 | ++++ b/embed.h |
| 56 | +@@ -1200,6 +1200,7 @@ |
| 57 | + # define refcounted_he_new_pv(a,b,c,d,e) Perl_refcounted_he_new_pv(aTHX_ a,b,c,d,e) |
| 58 | + # define refcounted_he_new_pvn(a,b,c,d,e,f) Perl_refcounted_he_new_pvn(aTHX_ a,b,c,d,e,f) |
| 59 | + # define refcounted_he_new_sv(a,b,c,d,e) Perl_refcounted_he_new_sv(aTHX_ a,b,c,d,e) |
| 60 | ++# define release_RExC_state(a) Perl_release_RExC_state(aTHX_ a) |
| 61 | + # define report_evil_fh(a) Perl_report_evil_fh(aTHX_ a) |
| 62 | + # define report_wrongway_fh(a,b) Perl_report_wrongway_fh(aTHX_ a,b) |
| 63 | + # define rpeep(a) Perl_rpeep(aTHX_ a) |
| 64 | +diff --git a/proto.h b/proto.h |
| 65 | +index e132956f8ac8..6ded4aeb247f 100644 |
| 66 | +--- a/proto.h |
| 67 | ++++ b/proto.h |
| 68 | +@@ -3893,6 +3893,12 @@ PERL_CALLCONV void |
| 69 | + Perl_reginitcolors(pTHX); |
| 70 | + #define PERL_ARGS_ASSERT_REGINITCOLORS |
| 71 | + |
| 72 | ++PERL_CALLCONV void |
| 73 | ++Perl_release_RExC_state(pTHX_ void *vstate) |
| 74 | ++ __attribute__visibility__("hidden"); |
| 75 | ++#define PERL_ARGS_ASSERT_RELEASE_REXC_STATE \ |
| 76 | ++ assert(vstate) |
| 77 | ++ |
| 78 | + PERL_CALLCONV void |
| 79 | + Perl_repeatcpy(char *to, const char *from, SSize_t len, IV count); |
| 80 | + #define PERL_ARGS_ASSERT_REPEATCPY \ |
| 81 | +diff --git a/regcomp.c b/regcomp.c |
| 82 | +index a79221079269..14e1390062e5 100644 |
| 83 | +--- a/regcomp.c |
| 84 | ++++ b/regcomp.c |
| 85 | +@@ -1356,15 +1356,19 @@ S_is_ssc_worth_it(const RExC_state_t * pRExC_state, const regnode_ssc * ssc) |
| 86 | + return true; |
| 87 | + } |
| 88 | + |
| 89 | +-static void |
| 90 | +-release_RExC_state(pTHX_ void *vstate) { |
| 91 | +- RExC_state_t *pRExC_state = (RExC_state_t *)vstate; |
| 92 | ++#ifdef PERL_RE_BUILD_AUX |
| 93 | ++ |
| 94 | ++void |
| 95 | ++Perl_release_RExC_state(pTHX_ void *vstate) { |
| 96 | ++ PERL_ARGS_ASSERT_RELEASE_REXC_STATE; |
| 97 | + |
| 98 | ++ RExC_state_t *pRExC_state = (RExC_state_t *)vstate; |
| 99 | ++ |
| 100 | + /* Any or all of these might be NULL. |
| 101 | + |
| 102 | + There's no point in setting them to NULL after the free, since |
| 103 | + pRExC_state is about to be released. |
| 104 | +- */ |
| 105 | ++ */ |
| 106 | + SvREFCNT_dec(RExC_rx_sv); |
| 107 | + Safefree(RExC_open_parens); |
| 108 | + Safefree(RExC_close_parens); |
| 109 | +@@ -1374,6 +1378,8 @@ release_RExC_state(pTHX_ void *vstate) { |
| 110 | + Safefree(pRExC_state); |
| 111 | + } |
| 112 | + |
| 113 | ++#endif |
| 114 | ++ |
| 115 | + /* |
| 116 | + * Perl_re_op_compile - the perl internal RE engine's function to compile a |
| 117 | + * regular expression into internal code. |
| 118 | +@@ -1475,7 +1481,7 @@ Perl_re_op_compile(pTHX_ SV ** const patternp, int pat_count, |
| 119 | + * or error. */ |
| 120 | + Newxz(pRExC_state, 1, RExC_state_t); |
| 121 | + |
| 122 | +- SAVEDESTRUCTOR_X(release_RExC_state, pRExC_state); |
| 123 | ++ SAVE_FREE_REXC_STATE(pRExC_state); |
| 124 | + |
| 125 | + DEBUG_r({ |
| 126 | + /* and then initialize RExC_mysv1 and RExC_mysv2 early so if |
| 127 | +diff --git a/regen/scope_types.pl b/regen/scope_types.pl |
| 128 | +index 3a7522d734b9..a3f406951793 100644 |
| 129 | +--- a/regen/scope_types.pl |
| 130 | ++++ b/regen/scope_types.pl |
| 131 | +@@ -137,6 +137,7 @@ BEGIN |
| 132 | + SAVEt_FREEPADNAME |
| 133 | + SAVEt_STRLEN_SMALL |
| 134 | + SAVEt_FREERCPV |
| 135 | ++SAVEt_FREE_REXC_STATE |
| 136 | + |
| 137 | + /* two args */ |
| 138 | + |
| 139 | +diff --git a/scope.c b/scope.c |
| 140 | +index 210ea36da3e8..b8063c27760b 100644 |
| 141 | +--- a/scope.c |
| 142 | ++++ b/scope.c |
| 143 | +@@ -1391,6 +1391,12 @@ Perl_leave_scope(pTHX_ I32 base) |
| 144 | + Safefree(a0.any_ptr); |
| 145 | + break; |
| 146 | + |
| 147 | ++ case SAVEt_FREE_REXC_STATE: |
| 148 | ++ a0 = ap[0]; |
| 149 | ++ if (a0.any_ptr) |
| 150 | ++ release_RExC_state(a0.any_ptr); |
| 151 | ++ break; |
| 152 | ++ |
| 153 | + case SAVEt_CLEARPADRANGE: |
| 154 | + { |
| 155 | + I32 i; |
| 156 | +diff --git a/scope.h b/scope.h |
| 157 | +index 311c4a32ec1b..eccd3aaba2dc 100644 |
| 158 | +--- a/scope.h |
| 159 | ++++ b/scope.h |
| 160 | +@@ -183,6 +183,11 @@ scope has the given name. C<name> must be a literal string. |
| 161 | + #define SAVESETSVFLAGS(sv,mask,val) save_set_svflags(sv,mask,val) |
| 162 | + #define SAVEFREECOPHH(h) save_pushptr((void *)(h), SAVEt_FREECOPHH) |
| 163 | + |
| 164 | ++#if defined(PERL_CORE) || defined(PERL_EXT) |
| 165 | ++# define SAVE_FREE_REXC_STATE(p) \ |
| 166 | ++ save_pushptr((void *)(p), SAVEt_FREE_REXC_STATE) |
| 167 | ++#endif |
| 168 | ++ |
| 169 | + #define SAVEDELETE(h,k,l) \ |
| 170 | + save_delete(MUTABLE_HV(h), (char*)(k), (I32)(l)) |
| 171 | + #define SAVEHDELETE(h,s) \ |
| 172 | +diff --git a/sv.c b/sv.c |
| 173 | +index e8c6e65a2717..ae6d09dea28a 100644 |
| 174 | +--- a/sv.c |
| 175 | ++++ b/sv.c |
| 176 | +@@ -15515,6 +15515,11 @@ Perl_ss_dup(pTHX_ PerlInterpreter *proto_perl, CLONE_PARAMS* param) |
| 177 | + c = (char*)POPPTR(ss,ix); |
| 178 | + TOPPTR(nss,ix) = pv_dup_inc(c); |
| 179 | + break; |
| 180 | ++ case SAVEt_FREE_REXC_STATE: |
| 181 | ++ (void)POPPTR(ss, ix); |
| 182 | ++ /* free only once */ |
| 183 | ++ TOPPTR(nss, ix) = NULL; |
| 184 | ++ break; |
| 185 | + case SAVEt_FREERCPV: |
| 186 | + c = (char *)POPPTR(ss,ix); |
| 187 | + TOPPTR(nss,ix) = rcpv_copy(c); |
| 188 | + |
| 189 | +From c54f9ca07b8390e9b917ea3b66fa605c8cf24b09 Mon Sep 17 00:00:00 2001 |
| 190 | +From: Tony Cook < [email protected]> |
| 191 | +Date: Mon, 14 Apr 2025 10:46:13 +1000 |
| 192 | +Subject: [PATCH 3/3] regcomp: ensure the RExC_state freed earlier rather than |
| 193 | + later |
| 194 | + |
| 195 | +388bf71 (ignoring #23022 for now) ensured that the RExC_state |
| 196 | +and its controlled pointers were freed once and only once, |
| 197 | +regardless of whether re_op_compile() returned normally or threw |
| 198 | +an exception. |
| 199 | + |
| 200 | +Unfortunately that free could happen very later, surviving well beyond |
| 201 | +when the regexp was compiled. |
| 202 | + |
| 203 | +Add an ENTER/LEAVE pair to ensure the cleanup is done immediately |
| 204 | +on a normal return. |
| 205 | +--- |
| 206 | + regcomp.c | 9 ++++++++- |
| 207 | + 1 file changed, 8 insertions(+), 1 deletion(-) |
| 208 | + |
| 209 | +diff --git a/regcomp.c b/regcomp.c |
| 210 | +index 14e1390062e5..0a84ad07606f 100644 |
| 211 | +--- a/regcomp.c |
| 212 | ++++ b/regcomp.c |
| 213 | +@@ -1481,6 +1481,7 @@ Perl_re_op_compile(pTHX_ SV ** const patternp, int pat_count, |
| 214 | + * or error. */ |
| 215 | + Newxz(pRExC_state, 1, RExC_state_t); |
| 216 | + |
| 217 | ++ ENTER_with_name("re_op_compile"); |
| 218 | + SAVE_FREE_REXC_STATE(pRExC_state); |
| 219 | + |
| 220 | + DEBUG_r({ |
| 221 | +@@ -1578,6 +1579,8 @@ Perl_re_op_compile(pTHX_ SV ** const patternp, int pat_count, |
| 222 | + "Precompiled pattern%s\n", |
| 223 | + orig_rx_flags & RXf_SPLIT ? " for split" : "")); |
| 224 | + |
| 225 | ++ LEAVE_with_name("re_op_compile"); |
| 226 | ++ |
| 227 | + return (REGEXP*)re; |
| 228 | + } |
| 229 | + } |
| 230 | +@@ -1593,7 +1596,9 @@ Perl_re_op_compile(pTHX_ SV ** const patternp, int pat_count, |
| 231 | + pat = newSVpvn_flags(exp, plen, SVs_TEMP | |
| 232 | + (IN_BYTES ? 0 : SvUTF8(pat))); |
| 233 | + } |
| 234 | +- return CALLREGCOMP_ENG(eng, pat, orig_rx_flags); |
| 235 | ++ REGEXP *re = CALLREGCOMP_ENG(eng, pat, orig_rx_flags); |
| 236 | ++ LEAVE_with_name("re_op_compile"); |
| 237 | ++ return re; |
| 238 | + } |
| 239 | + |
| 240 | + /* ignore the utf8ness if the pattern is 0 length */ |
| 241 | +@@ -1643,6 +1648,7 @@ Perl_re_op_compile(pTHX_ SV ** const patternp, int pat_count, |
| 242 | + Perl_re_printf( aTHX_ "%sSkipping recompilation of unchanged REx%s %s\n", |
| 243 | + PL_colors[4], PL_colors[5], s); |
| 244 | + }); |
| 245 | ++ LEAVE_with_name("re_op_compile"); |
| 246 | + return old_re; |
| 247 | + } |
| 248 | + |
| 249 | +@@ -2477,6 +2483,7 @@ Perl_re_op_compile(pTHX_ SV ** const patternp, int pat_count, |
| 250 | + if (old_re && SvREADONLY(old_re)) |
| 251 | + SvREADONLY_on(Rx); |
| 252 | + #endif |
| 253 | ++ LEAVE_with_name("re_op_compile"); |
| 254 | + return Rx; |
| 255 | + } |
| 256 | + |
0 commit comments