Allocate temporary PCRE match data using ZMM

Create a separate general context that uses ZMM as allocator and use it to allocate temporary PCRE match data (there is still one global match data). There is no requirement that the match data and the compiled regex / match context use the same general context. This makes sure that we do not leak persistent memory on bailout and fixes oss-fuzz #25296, on which half the libfuzzer runs currently get stuck.
php · Sep 7, 2020 · f4b2497 · f4b2497
1 parent 9475bcb
commit f4b2497
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 11 deletions.
diff --git a/ext/pcre/php_pcre.c b/ext/pcre/php_pcre.c
@@ -59,6 +59,7 @@ PHPAPI ZEND_DECLARE_MODULE_GLOBALS(pcre)
 #define PCRE_JIT_STACK_MAX_SIZE (192 * 1024)
 ZEND_TLS pcre2_jit_stack *jit_stack = NULL;
 #endif
+/* General context using (infallible) system allocator. */
 ZEND_TLS pcre2_general_context *gctx = NULL;
 /* These two are global per thread for now. Though it is possible to use these
  	per pattern. Either one can copy it and use in pce, or one does no global
@@ -173,15 +174,24 @@ static void php_efree_pcre_cache(zval *data) /* {{{ */
 /* }}} */
 
 static void *php_pcre_malloc(PCRE2_SIZE size, void *data)
-{/*{{{*/
-	void *p = pemalloc(size, 1);
-	return p;
-}/*}}}*/
+{
+	return pemalloc(size, 1);
+}
 
 static void php_pcre_free(void *block, void *data)
-{/*{{{*/
+{
 	pefree(block, 1);
-}/*}}}*/
+}
+
+static void *php_pcre_emalloc(PCRE2_SIZE size, void *data)
+{
+	return emalloc(size);
+}
+
+static void php_pcre_efree(void *block, void *data)
+{
+	efree(block);
+}
 
 #define PHP_PCRE_PREALLOC_MDATA_SIZE 32
 
@@ -476,6 +486,11 @@ static PHP_RINIT_FUNCTION(pcre)
 	mdata_used = 0;
 #endif
 
+	PCRE_G(gctx_zmm) = pcre2_general_context_create(php_pcre_emalloc, php_pcre_efree, NULL);
+	if (!PCRE_G(gctx_zmm)) {
+		return FAILURE;
+	}
+
 	if (PCRE_G(per_request_cache)) {
 		zend_hash_init(&PCRE_G(pcre_cache), 0, NULL, php_efree_pcre_cache, 0);
 	}
@@ -486,6 +501,9 @@ static PHP_RINIT_FUNCTION(pcre)
 
 static PHP_RSHUTDOWN_FUNCTION(pcre)
 {
+	pcre2_general_context_free(PCRE_G(gctx_zmm));
+	PCRE_G(gctx_zmm) = NULL;
+
 	if (PCRE_G(per_request_cache)) {
 		zend_hash_destroy(&PCRE_G(pcre_cache));
 	}
@@ -1246,7 +1264,7 @@ PHPAPI void php_pcre_match_impl(pcre_cache_entry *pce, zend_string *subject_str,
 	if (!mdata_used && num_subpats <= PHP_PCRE_PREALLOC_MDATA_SIZE) {
 		match_data = mdata;
 	} else {
-		match_data = pcre2_match_data_create_from_pattern(pce->re, gctx);
+		match_data = pcre2_match_data_create_from_pattern(pce->re, PCRE_G(gctx_zmm));
 		if (!match_data) {
 			PCRE_G(error_code) = PHP_PCRE_INTERNAL_ERROR;
 			if (subpat_names) {
@@ -1617,7 +1635,7 @@ PHPAPI zend_string *php_pcre_replace_impl(pcre_cache_entry *pce, zend_string *su
 	if (!mdata_used && num_subpats <= PHP_PCRE_PREALLOC_MDATA_SIZE) {
 		match_data = mdata;
 	} else {
-		match_data = pcre2_match_data_create_from_pattern(pce->re, gctx);
+		match_data = pcre2_match_data_create_from_pattern(pce->re, PCRE_G(gctx_zmm));
 		if (!match_data) {
 			PCRE_G(error_code) = PHP_PCRE_INTERNAL_ERROR;
 			return NULL;
@@ -1871,7 +1889,7 @@ static zend_string *php_pcre_replace_func_impl(pcre_cache_entry *pce, zend_strin
 		mdata_used = 1;
 		match_data = mdata;
 	} else {
-		match_data = pcre2_match_data_create_from_pattern(pce->re, gctx);
+		match_data = pcre2_match_data_create_from_pattern(pce->re, PCRE_G(gctx_zmm));
 		if (!match_data) {
 			PCRE_G(error_code) = PHP_PCRE_INTERNAL_ERROR;
 			if (subpat_names) {
@@ -2519,7 +2537,7 @@ PHPAPI void php_pcre_split_impl(pcre_cache_entry *pce, zend_string *subject_str,
 	if (!mdata_used && num_subpats <= PHP_PCRE_PREALLOC_MDATA_SIZE) {
 		match_data = mdata;
 	} else {
-		match_data = pcre2_match_data_create_from_pattern(pce->re, gctx);
+		match_data = pcre2_match_data_create_from_pattern(pce->re, PCRE_G(gctx_zmm));
 		if (!match_data) {
 			PCRE_G(error_code) = PHP_PCRE_INTERNAL_ERROR;
 			zval_ptr_dtor(return_value);
@@ -2853,7 +2871,7 @@ PHPAPI void  php_pcre_grep_impl(pcre_cache_entry *pce, zval *input, zval *return
 	if (!mdata_used && num_subpats <= PHP_PCRE_PREALLOC_MDATA_SIZE) {
 		match_data = mdata;
 	} else {
-		match_data = pcre2_match_data_create_from_pattern(pce->re, gctx);
+		match_data = pcre2_match_data_create_from_pattern(pce->re, PCRE_G(gctx_zmm));
 		if (!match_data) {
 			PCRE_G(error_code) = PHP_PCRE_INTERNAL_ERROR;
 			return;

diff --git a/ext/pcre/php_pcre.h b/ext/pcre/php_pcre.h
@@ -84,6 +84,8 @@ ZEND_BEGIN_MODULE_GLOBALS(pcre)
 	/* Used for unmatched subpatterns in OFFSET_CAPTURE mode */
 	zval unmatched_null_pair;
 	zval unmatched_empty_pair;
+	/* General context using per-request allocator (ZMM). */
+	pcre2_general_context *gctx_zmm;
 ZEND_END_MODULE_GLOBALS(pcre)
 
 PHPAPI ZEND_EXTERN_MODULE_GLOBALS(pcre)

diff --git a/ext/pcre/tests/preg_replace_callback_fatal_error_leak.phpt b/ext/pcre/tests/preg_replace_callback_fatal_error_leak.phpt
@@ -0,0 +1,18 @@
+--TEST--
+preg_replace_callback() should not leak persistent memory on fatal error
+--FILE--
+<?php
+
+function test() {}
+
+preg_replace_callback('/a/', function($matches) {
+    preg_replace_callback('/x/', function($matches) {
+        function test() {} // Trigger a fatal error.
+        return 'y';
+    }, 'x');
+    return 'b';
+}, 'a');
+
+?>
+--EXPECTF--
+Fatal error: Cannot redeclare test() (previously declared in %s on line %d