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