diff --git a/src/traffic_layout/info.cc b/src/traffic_layout/info.cc index 91b0677e042..d023a755d01 100644 --- a/src/traffic_layout/info.cc +++ b/src/traffic_layout/info.cc @@ -170,6 +170,25 @@ produce_features(bool json) print_feature("TS_IP_TRANSPARENT", TS_IP_TRANSPARENT, json); print_feature("TS_HAS_128BIT_CAS", TS_HAS_128BIT_CAS, json); print_feature("TS_HAS_TESTS", TS_HAS_TESTS, json); + // Whether PCRE2 can run a pattern on the just-in-time engine. This is a property of the + // PCRE2 that ATS is linked against, not of ATS, and it decides which resource limit a + // pathological pattern reaches: the JIT stack, or the interpreter's far larger match, + // depth and heap limits, because the interpreter keeps its backtracking frames on the + // heap. Tests that assert on one of those limits need to know. + // + // PCRE2_JIT_TEST_ALLOC (PCRE2 10.45) also confirms the JIT can allocate executable + // memory. PCRE2_CONFIG_JIT does not, and reports success on a hardened runtime where + // every JIT compile then fails, which is the direction that misleads a test gate. + { + uint32_t has_jit = 0; + +#ifdef PCRE2_JIT_TEST_ALLOC + has_jit = pcre2_jit_compile(nullptr, PCRE2_JIT_TEST_ALLOC) == 0; +#else + pcre2_config(PCRE2_CONFIG_JIT, &has_jit); +#endif + print_feature("TS_HAS_PCRE2_JIT", has_jit != 0, json); + } print_feature("TS_MAX_THREADS_IN_EACH_THREAD_TYPE", TS_MAX_THREADS_IN_EACH_THREAD_TYPE, json); print_feature("TS_MAX_NUMBER_EVENT_THREADS", TS_MAX_NUMBER_EVENT_THREADS, json); print_feature("TS_MAX_HOST_NAME_LEN", TS_MAX_HOST_NAME_LEN, json); diff --git a/src/tsutil/Regex.cc b/src/tsutil/Regex.cc index 56632dc6de3..2d7acff1800 100644 --- a/src/tsutil/Regex.cc +++ b/src/tsutil/Regex.cc @@ -26,6 +26,7 @@ #define PCRE2_CODE_UNIT_WIDTH 8 #include +#include #include #include @@ -79,6 +80,63 @@ my_free(void *ptr, void * /*caller*/) free(ptr); } +//---------------------------------------------------------------------------- +// One match context is shared by every thread that matches through it, and PCRE2 +// requires a distinct JIT stack per thread, so the stack comes from a callback +// invoked at match time rather than a pointer baked in when the context is built. +// +// The per thread stack is held in a pthread key rather than a thread_local. A +// thread_local with a destructor registers it through __cxa_thread_atexit, which +// takes the dynamic loader lock; doing that from a match would invert lock order +// against a dlopen caller running a plugin's static initialization. See the same +// hazard described at Diags::tag_activated. A pthread key registers its destructor +// once, at key creation, and never from the matching path. +pthread_key_t jit_stack_key; +bool jit_stack_key_valid = false; +pthread_once_t jit_stack_key_once = PTHREAD_ONCE_INIT; + +void +destroy_jit_stack(void *stack) +{ + if (stack != nullptr) { + pcre2_jit_stack_free(static_cast(stack)); + } +} + +void +make_jit_stack_key() +{ + jit_stack_key_valid = pthread_key_create(&jit_stack_key, destroy_jit_stack) == 0; +} + +pcre2_jit_stack * +jit_stack_for_this_thread(void *) +{ + pthread_once(&jit_stack_key_once, make_jit_stack_key); + if (!jit_stack_key_valid) { + // Without a key there is nowhere to keep a stack, and jit_stack_key holds a + // default value that may name an unrelated key. Returning null tells PCRE2 to + // use its own default stack, which pcre2jit documents as thread safe. + return nullptr; + } + + auto *stack = static_cast(pthread_getspecific(jit_stack_key)); + if (stack == nullptr) { + // One page to start, one mebibyte at most. The maximum is address space reserved at + // creation and made resident only as deep as a match actually goes, so a larger one + // costs nothing per match, and a mebibyte already resolves a longer subject than + // proxy.config.http.request_header_max_size lets a client send. + stack = pcre2_jit_stack_create(4096, 1024 * 1024, nullptr); + if (pthread_setspecific(jit_stack_key, stack) != 0) { + // Nothing holds the stack now, so it would leak once per match. Give it back and + // let PCRE2 use its own default stack for this call. + pcre2_jit_stack_free(stack); + return nullptr; + } + } + return stack; +} + //---------------------------------------------------------------------------- class RegexContext { @@ -100,9 +158,6 @@ class RegexContext if (_match_context != nullptr) { pcre2_match_context_free(_match_context); } - if (_jit_stack != nullptr) { - pcre2_jit_stack_free(_jit_stack); - } } pcre2_general_context * get_general_context() @@ -126,13 +181,11 @@ class RegexContext _general_context = pcre2_general_context_create(my_malloc, my_free, nullptr); _compile_context = pcre2_compile_context_create(_general_context); _match_context = pcre2_match_context_create(_general_context); - _jit_stack = pcre2_jit_stack_create(4096, 1024 * 1024, nullptr); // 1 page min and 1MB max - pcre2_jit_stack_assign(_match_context, nullptr, _jit_stack); + pcre2_jit_stack_assign(_match_context, jit_stack_for_this_thread, nullptr); } pcre2_general_context *_general_context = nullptr; pcre2_compile_context *_compile_context = nullptr; pcre2_match_context *_match_context = nullptr; - pcre2_jit_stack *_jit_stack = nullptr; }; } // namespace @@ -257,8 +310,20 @@ struct RegexMatchContext::_MatchContext { //---------------------------------------------------------------------------- RegexMatchContext::RegexMatchContext() { - auto ctx = pcre2_match_context_create(nullptr); - debug_assert_message(ctx, "Failed to allocate custom pcre2 match context"); + // Copy the shared context rather than building a blank one. A blank context + // silently drops everything the shared context configures, which is how this + // type came to run with PCRE2's fallback 32KiB JIT stack instead of the 1MiB + // one. Callers override only the fields they mean to. + // + // pcre2_match_context_copy dereferences its argument rather than returning null + // for one, and RegexContext's constructor does not check its allocations, so a + // shared context that failed to allocate would crash here. Fall back to a blank + // context, which is what this constructor built before and which no reader of + // _match_context dereferences unchecked. + auto *shared = RegexContext::get_instance()->get_match_context(); + auto *ctx = shared != nullptr ? pcre2_match_context_copy(shared) : pcre2_match_context_create(nullptr); + + debug_assert_message(ctx, "Failed to obtain a pcre2 match context"); _MatchContext::set(_match_context, ctx); } diff --git a/src/tsutil/unit_tests/test_Regex.cc b/src/tsutil/unit_tests/test_Regex.cc index 6cda5f7a813..167e1fb61a6 100644 --- a/src/tsutil/unit_tests/test_Regex.cc +++ b/src/tsutil/unit_tests/test_Regex.cc @@ -20,7 +20,12 @@ limitations under the License. */ +#include +#include +#include +#include #include +#include #include #define PCRE2_CODE_UNIT_WIDTH 8 @@ -1147,3 +1152,260 @@ TEST_CASE("Regex copies answer the same as their original", "[libts][Regex][copy CHECK(original.exec(ordinary, original_ordinary) == copy.exec(ordinary, copy_ordinary)); } } + +namespace +{ +/** Does PCRE2 have JIT code for this pattern? + * + * The two tests below are about the JIT stack, and PCRE2 consults it only when it + * has JIT code to run. Without it both a blank context and the shared one take the + * interpreter and return the same answer, so the tests would pass whether or not + * the behaviour they describe is present. Ask PCRE2 rather than assume. + * + * This asks about one pattern, deliberately: PCRE2 declines some pattern items and + * leaves PCRE2_INFO_JITSIZE at zero on a build whose JIT is otherwise fine, so the + * library-wide probes (pcre2_config, PCRE2_JIT_TEST_ALLOC) answer a different + * question. Call it from the test thread only; Catch2 assertions are not thread safe. + */ +bool +pattern_has_jit(char const *pattern) +{ + int errnum = 0; + PCRE2_SIZE erroffset = 0; + pcre2_code *code = pcre2_compile(reinterpret_cast(pattern), PCRE2_ZERO_TERMINATED, 0, &errnum, &erroffset, nullptr); + + // A pattern that will not compile is a broken test, not a build without a JIT. + // Reporting it as "no JIT" would turn a typo into a silent skip. + REQUIRE(code != nullptr); + + pcre2_jit_compile(code, PCRE2_JIT_COMPLETE); + size_t jit_size = 0; + + pcre2_pattern_info(code, PCRE2_INFO_JITSIZE, &jit_size); + pcre2_code_free(code); + return jit_size > 0; +} +} // namespace + +// A caller-supplied RegexMatchContext must behave like the shared context that +// Regex::exec uses when none is supplied. A context built from scratch silently +// drops everything the shared one configures, which is how regex_remap came to +// run with PCRE2's fallback 32KiB JIT stack instead of the 1MiB one. +TEST_CASE("RegexMatchContext matches the shared context", "[libts][Regex][RegexMatchContext]") +{ + // Quantified alternation of capture groups: every subject character pushes a + // backtracking frame, so the JIT stack size is what bounds this. + char const *const pattern = R"(^(?:(a)|(b))+$)"; + if (!pattern_has_jit(pattern)) { + SKIP("PCRE2 has no JIT for this pattern, so the JIT stack is never consulted"); + } + + Regex re; + REQUIRE(re.compile(pattern)); + + // 40 bytes of JIT stack per subject character, so PCRE2's 32KiB fallback stops at + // 818 characters and the shared context's 1MiB stack at 26,213. 5,000 sits six + // times above the first and five times below the second, so a blank context and + // the shared one give different answers with margin either way. + std::string const subject(5000, 'a'); + + RegexMatches shared_matches; + RegexMatchContext match_context; + RegexMatches own_matches; + + int const shared_rc = re.exec(subject, shared_matches); + int const own_rc = re.exec(subject, own_matches, 0, &match_context); + CAPTURE(shared_rc, own_rc); + + REQUIRE(shared_rc > 0); + REQUIRE(own_rc == shared_rc); + + // The copy paths must carry the inherited configuration too. Building a blank + // context in either of them is the same defect, and nothing else here notices. + RegexMatchContext const copied{match_context}; + RegexMatchContext assigned; + + assigned = match_context; + + RegexMatches copied_matches; + RegexMatches assigned_matches; + + REQUIRE(re.exec(subject, copied_matches, 0, &copied) == shared_rc); + REQUIRE(re.exec(subject, assigned_matches, 0, &assigned) == shared_rc); +} + +// The guard from #5762: a pattern that backtracks once per character must fail +// cleanly rather than run the thread out of stack. PCRE1 recursed on the machine +// stack and a long enough subject crashed the server; PCRE2 must report an error +// instead. +TEST_CASE("Regex reports resource exhaustion rather than crashing", "[libts][Regex][limits]") +{ + // Only the JIT path has a bound to exhaust here. PCRE2's interpreter keeps its + // backtracking frames on the heap, so it matches this subject rather than running + // out of anything, and there is no resource error to assert. + char const *const pattern = R"(^/alpha/bravo/[?]((?!action=(newsfeed|calendar|contacts|notepad)).)*$)"; + if (!pattern_has_jit(pattern)) { + SKIP("PCRE2 has no JIT for this pattern, so there is no stack bound to exhaust"); + } + + Regex re; + REQUIRE(re.compile(pattern)); + + // This pattern starts failing at roughly 43KiB of subject against a 1MiB JIT + // stack, measured identically on x86_64 and arm64. Smaller JIT frames get more + // subject out of the same stack and so push that threshold up; 256KiB keeps a + // six times margin against that. Do not trim this to just above 43KiB. + std::string subject{"/alpha/bravo/?"}; + subject.append(256 * 1024, 'x'); + + RegexMatches matches; + int const rc = re.exec(subject, matches); + CAPTURE(rc); + + // Reaching this line at all is the crash assertion. + REQUIRE(rc < 0); + REQUIRE(rc != RE_ERROR_NOMATCH); + + // The paired positive control, and the only part of this case that can tell a + // 1MiB stack from PCRE2's 32KiB fallback: 256KiB overruns both bounds, so every + // assertion above also holds with the JIT stack callback deleted. 10,000 is seven + // times over the fallback's bound and four times under the 1MiB stack's. Neither + // bound pins 1MiB itself; the suite only requires a maximum in roughly + // [240KB, 6MB]. + std::string smaller{"/alpha/bravo/?"}; + smaller.append(10000, 'x'); + + RegexMatches small_matches; + int const small_rc = re.exec(smaller, small_matches); + CAPTURE(small_rc); + + REQUIRE(small_rc > 0); +} + +namespace +{ +/** A start gate, so every thread is inside the match loop before any of them gets far + * and the matching actually overlaps. std::latch would say this directly; it is + * hand-rolled here to match notstd::barrier in benchmark_LogObject.cc. + * + * Single-use, like the std::latch it stands in for: once the gate opens it stays open + * and a second round returns immediately. Exactly `expected` threads must call + * arrive_and_wait() or the rest block forever, and the gate must outlive all of them, + * since destroying a condition_variable with waiters on it is undefined. + */ +class ThreadGate +{ +public: + explicit ThreadGate(int expected) : _expected{expected} { REQUIRE(expected > 0); } + + /// Count this thread in, then block until every expected thread has done the same. + void + arrive_and_wait() + { + std::unique_lock lock{_mutex}; + + if (++_arrived == _expected) { + _open = true; + _cv.notify_all(); + return; + } + _cv.wait(lock, [this]() { return _open; }); + } + +private: + std::mutex _mutex; + std::condition_variable _cv; + int const _expected; + int _arrived = 0; + bool _open = false; +}; + +/** A subject the deep pattern below backtracks through once per character. + * + * At 40 bytes of JIT stack per character the longest of these needs about 223KiB: six + * times what PCRE2's 32KiB fallback resolves, and under a quarter of the shared + * context's 1MiB stack. Each thread gets its own length and its own content, so no two + * threads are matching identical bytes. + */ +std::string +stack_hungry_subject(int thread_index) +{ + std::string subject(5000 + thread_index * 100, 'a'); + + subject[11 + thread_index] = 'b'; + return subject; +} +} // namespace + +// The header promises that exec() may be called concurrently on one instance. Every thread +// must reach the same verdict, whether it matches through a caller-supplied context shared +// by several threads or through the thread-global one. +// +// The deep subjects are longer than PCRE2's 32KiB fallback stack can resolve, so on a JIT +// build this also fails if the callback stops handing every thread a 1MiB stack: deleting +// the callback, or returning null from it, turns every deep match here into +// PCRE2_ERROR_JIT_STACKLIMIT. Without a JIT the interpreter resolves them either way, so +// only the concurrency half of this case carries over to such a build. +// +// It does not establish that the stacks are distinct. One stack shared by all eight threads +// corrupts a result only when two matches are deep at the same instant, so it fails +// intermittently rather than reliably and is not an oracle for that. ThreadSanitizer does +// not close the gap either: the racing writes come from sljit-generated code it never sees. +TEST_CASE("Regex matches concurrently on one instance", "[libts][Regex][threads]") +{ + Regex re_captures; + REQUIRE(re_captures.compile(R"(^/([a-z]+)/([0-9]+)/(.*)$)")); + + Regex re_deep; + REQUIRE(re_deep.compile(R"(^(?:(a)|(b))+$)")); + + constexpr int THREADS = 8; + constexpr int ITERATIONS = 2000; + + std::string const hit{"/alpha/42/tail"}; + std::string const miss{"/Alpha/xx/tail"}; + + std::atomic failures{0}; + ThreadGate gate{THREADS}; + + // One caller-supplied context, built here and shared by half the threads. That is the + // production shape: regex_remap builds a context when it loads a rule and every net + // thread then matches through it. + RegexMatchContext shared_caller_context; + + std::vector threads; + threads.reserve(THREADS); + for (int i = 0; i < THREADS; ++i) { + threads.emplace_back([&, i]() { + bool const use_caller_context = (i % 2) == 0; + RegexMatchContext const *const use = use_caller_context ? &shared_caller_context : nullptr; + std::string const deep = stack_hungry_subject(i); + + gate.arrive_and_wait(); + + for (int n = 0; n < ITERATIONS; ++n) { + RegexMatches matches; + if (re_captures.exec(hit, matches, 0, use) != 4 || matches[1] != "alpha" || matches[2] != "42" || matches[3] != "tail") { + ++failures; + } + + RegexMatches no_matches; + if (re_captures.exec(miss, no_matches, 0, use) != RE_ERROR_NOMATCH) { + ++failures; + } + + // Needs the 1MiB stack, and must consume the whole subject to have used it. + RegexMatches deep_matches; + if (re_deep.exec(deep, deep_matches, 0, use) <= 0 || deep_matches[0].size() != deep.size()) { + ++failures; + } + } + }); + } + + for (auto &t : threads) { + t.join(); + } + + CHECK(failures.load() == 0); +} diff --git a/tests/gold_tests/pluginTest/regex_remap/regex_remap.test.py b/tests/gold_tests/pluginTest/regex_remap/regex_remap.test.py index c6c30830127..41415778f61 100644 --- a/tests/gold_tests/pluginTest/regex_remap/regex_remap.test.py +++ b/tests/gold_tests/pluginTest/regex_remap/regex_remap.test.py @@ -88,7 +88,13 @@ 'proxy.config.diags.debug.enabled': 1, 'proxy.config.diags.debug.tags': 'http|regex_remap', 'proxy.config.dns.nameservers': f"127.0.0.1:{nameserver.Variables.Port}", - 'proxy.config.dns.resolv_conf': 'NULL' + 'proxy.config.dns.resolv_conf': 'NULL', + # Run 3b sends a query past the 1 MB JIT stack's ~44 KB bound, which is over the + # 32 KB default. Do not raise request_line_max_size to match: http_parser_parse_req + # asserts `parsed.size() < UINT16_MAX` before it rejects an over-long line, so on + # any assert-enabled build a request line past 65,535 aborts traffic_server instead + # of getting a 414. 65,535 is the real ceiling here, whatever the record says. + 'proxy.config.http.request_header_max_size': 131072 }) # 0 Test - Load cache (miss) (path1) @@ -123,19 +129,60 @@ tr.Processes.Default.Streams.stdout = "gold/regex_remap_simple.gold" tr.StillRunningAfter = ts -# 3 Test - Preserve the original crash guard from #5762. This request must -# survive resource exhaustion without redirecting, regardless of which matching -# resource limit is reached (JIT stack, match work, depth, or heap). -tr = Test.AddTestRun("resource exhaustion does not crash ATS") +# 3 Test - A 3 KB query redirects. This rule backtracks once per subject +# character, so it used to exhaust the 32 KB stack PCRE2 falls back to when a +# match context carries none, and the rule was skipped. The plugin's context now +# inherits the shared 1 MB stack, so the rule matches and the redirect fires. +# +# This run needs no JIT gate. With JIT the 1 MB stack resolves the subject, and +# without JIT the interpreter resolves it on the heap, so the redirect is the +# answer either way. It only demonstrates the fix on a build that has a JIT. +tr = Test.AddTestRun("long query redirects rather than exhausting the JIT stack") creq = replay_txns[1]['client-request'] -tr.MakeCurlCommand(curl_and_args + f"--header 'uuid: {creq['headers']['fields'][1][1]}' '{creq['url']}'", ts=ts) +tr.MakeCurlCommand( + curl_and_args + f"--header 'uuid: {creq['headers']['fields'][1][1]}' '{creq['url']}'" + " | grep -e '^HTTP/' -e '^Location'", + ts=ts) tr.Processes.Default.ReturnCode = 0 -tr.Processes.Default.Streams.stdout = "gold/regex_remap_crash.gold" -ts.Disk.diags_log.Content += Testers.ContainsExpression( - r'ERROR: \[regex_remap\] Bad regular expression result -(?:46|47|53|63).*"\^/alpha/bravo/', - "The crash-guard rule must report resource exhaustion") +tr.Processes.Default.Streams.stdout = "gold/regex_remap_redirect.gold" tr.StillRunningAfter = ts +# 3b Test - Preserve the original crash guard from #5762. This request must +# survive resource exhaustion without redirecting, regardless of which matching +# resource limit is reached (JIT stack, match work, depth, or heap). Against the +# shared 1 MB stack this rule needs a subject past ~44 KB to exhaust it (measured: +# 43,514 matches, 44,021 does not), which is why the header limit is raised above. +# Shortening this query silently turns the run into a plain redirect test. +# +# This run is boxed in, and the box cannot be widened by configuration. The floor is +# that ~44 KB bound; the ceiling is the 65,535 request line the parser asserts on. So +# the usable window is roughly 44,100 to 65,475 bytes of query, about 1.5x wide, and +# 64,000 is near the top of it. A platform whose JIT frames are enough smaller to push +# the floor over the ceiling cannot run this case at any query length: prefer the +# test_Regex.cc unit test, which bounds the stack directly, over stretching this one. +# +# Note this run is a crash guard, not a check on the stack size: its ContainsExpression +# accepts any of -46/-47/-53/-63, and the 32 KB fallback reaches -46 too. Run 3 above is +# what actually fails if the fix is reverted, because its 3 KB subject redirects on the +# 1 MB stack and errors on the fallback. +# +# Only the JIT engine has a stack to exhaust here. PCRE2's interpreter keeps its +# backtracking frames on the heap, so on a build without JIT this subject simply +# matches, the rule redirects, and both assertions below fail for a reason that has +# nothing to do with the behaviour under test. The crash property itself is not lost +# on such a build: run 4 reaches the match limit through the interpreter, and the +# unit test in test_Regex.cc asserts it directly. +if Condition.HasATSFeature('TS_HAS_PCRE2_JIT'): + crash_guard_query = 'x' * 64000 + tr = Test.AddTestRun("resource exhaustion does not crash ATS") + tr.MakeCurlCommand( + curl_and_args + "--header 'uuid: 180' " + f"'http://example.one/alpha/bravo/?action=newsfed;{crash_guard_query}'", ts=ts) + tr.Processes.Default.ReturnCode = 0 + tr.Processes.Default.Streams.stdout = "gold/regex_remap_crash.gold" + ts.Disk.diags_log.Content += Testers.ContainsExpression( + r'ERROR: \[regex_remap\] Bad regular expression result -(?:46|47|53|63).*"\^/alpha/bravo/', + "The crash-guard rule must report resource exhaustion") + tr.StillRunningAfter = ts + # 4 Test - The nested quantifiers must exceed PCRE2's default matching-work limit. tr = Test.AddTestRun("excessive backtracking reaches the match limit") creq = replay_txns[2]['client-request']