Runtime JIT support detection #157

minipli-oss · 2022-11-08T13:09:57Z

This PR adds runtime detection for the system's support of JIT memory allocations and allows users of pcre2 to fall back to the interpreter mode if rwx mappings are prohibited.

The most prominent user would be git grep currently failing hard on such systems. With this change in place, it simply uses the interpreter mode and "just works."

The first two commits are also send as a PR to the sljit repository. So you can either wait until these are merged and integrate the changes through the upstream project or you take all three of them directly through this PR.

Thanks!

zherczeg · 2022-11-09T08:08:35Z

zherczeg/sljit#136

In create_tempfile() we look for a suitable place to put the temporary file into and, among others, look at $TMPDIR. If the value of this environment variable exceeds the bounds of the local tmp_name[] buffer, we ignore it. However, we still change the value of 'tmp_name_len' which leads to follow-up errors. On debug builds this can lead to hitting the assertion as can be seen below: $ TMPDIR=$(perl -e 'print "A"x1024') ./bin/array_access Assertion failed at sljit_src/sljitProtExecAllocator.c:147 Aborted For non-debug builds, however, this can lead to a memory corruption, by abusing the fact that we change a trailing '/' to '\0' later on. With a sufficiently high enough value for 'tmp_name_len' this can corrupt stack frames up in the call chain. Fix this by setting 'tmp_name_len' only if value it is based on is found to be valid -- just like it was prior to commit 98323bd82218. Fixes: 98323bd82218 ("protexec: refactor create_tempfile() (PCRE2Project#37)") Signed-off-by: Mathias Krause <[email protected]>

SELinux or PaX/grsecurity based kernels may deny creating writable and executable mappings, leading to errors when trying to allocate JIT memory, even though JIT support is generally available. Provide a function to probe for the runtime availability of rwx maps to support users like libpcre2 which can use it to announce the lack of JIT and fall back to the interpreter instead. This function is only needed for Linux and only if we're using the default JIT memory allocator, as all others implement workarounds via double mappings. Signed-off-by: Mathias Krause <[email protected]>

SELinux or PaX/grsecurity based kernels may deny creating writable and executable mappings, leading to errors when trying to allocate JIT memory, even though JIT support is generally available. Instead of failing hard in this case, allow to use the interpreter fallback by probing and announcing the availability of JIT mode at runtime through the PCRE2_CONFIG_JIT hook. This only happens for configurations using only the default JIT memory allocator, i.e. not the SELinux aware one. However, we still mark the latter as experimental and distributions like Debian don't enable it. The current behaviour leads to nasty user visible errors on such systems, e.g. when running 'git grep': $ git grep peach fatal: Couldn't JIT the PCRE2 pattern 'peach', got '-48' With this change in place, it'll fall back to the interpreter and "just work", providing a much more pleasant user experience. Signed-off-by: Mathias Krause <[email protected]>

zherczeg · 2022-12-16T16:37:27Z

How can you tell the difference between "jit is not compiled" and "jit needs extra permissions"? If it is not possible, this direction is a feature loss.

minipli-oss · 2022-12-16T19:15:09Z

How can you tell the difference between "jit is not compiled" and "jit needs extra permissions"?

Easy:

#ifdef SUPPORT_JIT
  printf("JIT support compiled in %s\n", !!PRIV(jit_supported)() ? "and runtime supported" : "but runtime disabled");
#else
  printf("JIT support not compiled in\n");
#endif

If it is not possible, this direction is a feature loss.

What kind of feature loss? Users of PCRE2 just need to know if JIT will be working or not. Everything else doesn't matter. It's like as if one's complaining that glibc has an AVX2 implementation of memcpy() but that can't be used on an Atom box.

zherczeg · 2022-12-17T06:11:06Z

This is pcre2 code, SUPPORT_JIT is unknown for a library user. Is there another way to do it?

minipli-oss · 2022-12-20T11:05:03Z

We could change the API and make pcre2_config(PCRE2_CONFIG_JIT, &jit) require an int * instead of and uint32_t * with the following meaning of its value:

0: JIT support not compiled in
1: JIT support compiled in and supported at runtime
-1: JIT support compiled in but not supported at runtime

However, that's an ABI-breaking and backwards incompatible change, as users such as git that simply test for zero / non-zero would get a wrong result for case -1.

But what's the intended use case anyway? Why would a user of PCRE2 need to know why JIT isn't working?

zherczeg · 2022-12-20T19:17:23Z

Some (future) OS may allow giving exec rights on demand, so the application sees it cannot create executable memory, but it can requests the OS to do so (e.g a modal form requests admin password), and then retry the compilation. Furthermore the application can give different suggestions how to enable jit based on the return value of the config. Offline (pic) code generation might also be possible at some point similar to saving compiled patterns.

Honestly the whole who has which rights are out-of-scope from the perspective of jit. It would be good to have a separate library which handles / manages these things. Maybe there are some libraries for that.

minipli-oss · 2022-12-21T07:58:24Z

I agree, but this has nothing to do with this PR. That future OS may never materialize and if it does, it better handles the modal request dialog during the syscall in question. Everything else (application retrying syscalls (how often?)) makes no sense.

Anything else that blocks this bug from getting fixed?

zherczeg · 2022-12-21T08:06:05Z

This is an api breaking feature request, not a bug. The current code is working as intended, and this case can be handled as well.

minipli-oss · 2022-12-21T08:28:40Z

Oh, sorry, I must be missing something. I tried very hard to not break the API (nor ABI). Where does the change do that? (I guess you don't mean f852788 which fixes a memory corruption bug that might be considered "API.")

And I have to disagree with you, again, the current code is not working as intended, as outlined multiple times already: c2dfa1e, zherczeg/sljit#136 (comment) (and various other comments of mine in that issue).

zherczeg · 2022-12-21T09:31:24Z

I don't mind if you disagree with me. The patch collection changes the behaviour of PCRE2_CONFIG_JIT, so it is an api breaking change. It is like changing the fread to work like fwrite. The fix part can be landed.

You could work on a library which doing things you like. You could create a function there which checks PCRE2_CONFIG_JIT and adds your extra checks for your platform. It can also update the system config automatically when needed, filter the messages you don't like, etc.

minipli-oss · 2022-12-21T11:03:19Z

The patch changes the behavior of PCRE2_CONFIG_JIT indeed, but, from my understanding, to match what the API wants to provide.

pcre2_config(3) states:

         PCRE2_CONFIG_JIT             Availability of just-in-time compiler
                                       support (1=yes 0=no)

My understanding of this wording is that a call to pcre2_config(PCRE2_CONFIG_JIT, ...) should tell the caller if the JIT compiler is available, available to be used, i.e. if using pcre2_jit_*() functions will possibly succeed.

I mean, after all it's a function call. If one would merely be interested if JIT support is compiled in (in contrast of being supported), a C preprocessor define would be sufficient. But it's a call, so looking at the current runtime constraints is very much in line with answering the question "Does libpcre2 support JIT compilation (for this very process)?".

So IMO it's an API fix, not an API break.

PhilipHazel · 2022-12-21T11:53:33Z

The pcre2_config() function is intended to tell a client what configuration options were used when PCRE2 was compiled - hence its name. So it returns 1 or 0 according as SUPPORT_JIT was defined or not at PCRE2 compile time. A client cannot use a C preprocessor test because SUPPORT_JIT is not included in pcre2.h. If pcre2_config() returns 0, you know that calls to JIT functions will never succeed. If it returns 1, they might - the only way to find out is to try.

zherczeg · 2022-12-21T12:00:01Z

Even if the description is unclear, the code worked this way since beginning, and existing software rely on this.

minipli-oss · 2022-12-22T07:07:35Z

What's the use case of pcre2_config(PCRE2_CONFIG_JIT) returning 1 but every JIT function failing, i.e. having a non-functional JIT compiler?

Isn't it much more sensible to make pcre2_config(PCRE2_CONFIG_JIT) return 0 in this case (when we know for sure that the JIT compiler cannot work)?

zherczeg · 2022-12-22T09:48:05Z

Why x86 32 bit has only 8 registers, when it is totally inefficient? It is designed that way and compatibility must be maintained. Your use case is valid, but it seems it works well as a separate project. You create a library, which checks user rights, and this library could provide an extended version of pcre2_config, and lot of other things. By experience I know the next thing people want is to auto-update system configuration. They will use the same arguments as yours. And things get out of hand quickly.

PhilipHazel · 2022-12-22T16:38:59Z

The pcre2_jit_config() function has always just reflected the state of SUPPORT_JIT at compile time. I have made a note to add more explanation to the documentation at some point. Changing this is not something I would want to do - PCRE2 is very widely used and I know from past experience that changes of this sort, however minor, can cause issues. // Note that there have been times in the past when pcre2_jit_compile() worked for some patterns but not for others - when new features were added to the interpreter before they were supported in JIT. Therefore, the only way to tell if JIT will work for this pattern is to try it.

minipli-oss · 2022-12-23T11:29:32Z

I'm not arguing against handling errors of pcre2_jit_*() functions, not at all! I'm just saying, it makes no sense to announce JIT support when we know for sure that all patterns will fail (because the JIT cannot allocate memory for its code generation).

Or, to get back to @zherczeg's x86 analogy, what sense does it make trying to execute AES-NI instructions on a i486 and force the caller to handle invalid opcode exceptions -- just because some crypto library ships an AES-NI optimized implementation along with a generic one?

zherczeg · 2022-12-23T11:54:13Z

I cannot say anything about the "makes sense" part. For some people, something makes sense, and for the others the opposite is true. As for pcre, API compatibility is one of the most important aspect. When we couldn't extend pcre anymore, we have created pcre2. So the next time we do breaking changes will be pcre3 (no plans for that at the moment). Maybe we should discuss this topic then.

minipli-oss · 2023-02-15T09:53:56Z

FWIW, git will soon have some kind of a workaround for this API deficiency in PCRE2 via git/git@50b6ad5, because I simply want a functional 'git grep'. The workaround tries to detect the case where JIT support is announced to be available, but found to be non-functional at runtime. This detection has to be a heuristic, unfortunately. In case the user provided pattern from the commandline fails to JIT compile with PCRE2_ERROR_NOMEMORY, it tries to JIT the fixed pattern ".". If this fails with PCRE2_ERROR_NOMEMORY as well, git decides, it must be the memory allocator failing and deems JIT support to be non-functional. To still be able to provide a service to the user, git falls back to interpreter mode.

It's a gross hack, but the best git can do by making use of PCRE2's API only. In particular, it cannot know the details of the JIT memory allocator, so cannot do a similar probing as zherczeg/sljit@8e46492 would be able to.

Another observation, while developing the workaround, it became clear that the PCRE2 API simply doesn't allow to tell the following two cases apart:

JIT compiling a pattern failed because...

JIT memory cannot be allocated because system policies prevent it.
JIT ran into its size limitations.

Both cases will simply fail with PCRE2_ERROR_NOMEMORY which is unfortunate, as now it's impossible to detect the second case, which was a nice way to reject too complex patterns fast.

I still think this detection logic belongs to PCRE2/SLJIT, as that's where the JIT allocator lives and where all required details are known. Users of PCRE2 should have no need to guess or poke around any of this. It's an internal implementation detail of PCRE2/SLJIT after all. But I got it, you think differently, so we have to live with what we have. However, could you please tell how to distinguish error cases 1 and 2 above without making use of heuristics to maybe detect case 1?

zherczeg · 2023-02-18T03:35:48Z

I think tools use a very simple pattern, e.g. an empty string pattern to check initial allocation. But it is possible to an return error code when compiling is failed because of an unsupported pattern. There are various reasons for that, but maybe 1-2 return values should be enough to cover all cases. I will think about it.

minipli-oss · 2023-02-21T07:04:21Z

Can you provide examples of such tools so I've something to compare with and also confirm that this is considered PCRE2 API, i.e. will be a sufficient test that can detect JIT runtime support for current, future and past versions of PCRE2?

Regarding the error codes, I guess changing them is no option for PCRE2, as that would break the API and, from what I got, that's out of scope.

zherczeg · 2023-02-21T13:40:48Z

Yes, we cannot change existing error codes, only add new ones. Existing applications will not understand the new code, they will simply accept something is wrong. I will think about the best option.

zherczeg · 2023-03-01T09:32:49Z

#213

minipli-oss · 2023-03-01T10:19:34Z

Even adding new ones will break existing applications, as pointed out in #213 (review).

minipli-oss · 2024-07-22T19:25:47Z

Trying to resurrect the discussion...

Even though my urgent needs where solved by implementing the git hack (git/git@50b6ad5), I don't like it, I don't like it at all!

First of all, the runtime probing hack is needed just because pcre2_config(PCRE2_CONFIG_JIT,...) returned 1, i.e. made git take the branch that tries to make use of the JIT instead of directly falling back to the interpreter.

The probing code itself is, well, relying on a specific error code which isn't even uniquely identifying the case (and further may break in the future, e.g. via changes like #213, causing breakage in other applications checking the error code as well) and assuming JIT compiling the pattern "." will succeed if the JIT compiler actually can work when signalling being supported.

But the biggest issue I have with the hack, it needs to be repeated for each and every project making use of PCRE2 (or SLJIT) and that's quite cumbersome. Having it implemented right at the core, within SLJIT, actually, feels much more sensible than having to redo the same thing for each user.

I did a Debian Code search -- which is by no means a complete view but should provide a data point for what Debian and derived distribution are making use of -- and here are examples that still need patches and are currently broken if pcre2_config(PCRE2_CONFIG_JIT) returns 1 but pcre2_jit_compile(...) fails, e.g. for Linux kernels with PaX MPROTECT or SELinux's deny_execmem enabled:

zsh:

    int jit = 0;
    if (!pcre2_config(PCRE2_CONFIG_JIT, &jit) && jit) {
	if (pcre2_jit_compile(pcre_pattern, PCRE2_JIT_COMPLETE) < 0) {
	    zwarnnam(nam, "error while studying regex");
	    return 1;
	}
    }

    return 0;

Debian's ngrep:

+        pcre2_config(PCRE2_CONFIG_JIT, &pcre2_jit_on);
+        if (pcre2_jit_on) {
+            int rc;
+            size_t jitsz;
+
+            if (pcre2_jit_compile(re, PCRE2_JIT_COMPLETE) != 0) {
+                fprintf(stderr, "unable to JIT-compile pcre2 regular expression\n");
+                return 1;
+            }

GDAL and sqlite-pcre2:

    pcre2_config(PCRE2_CONFIG_JIT, &has_jit);
    if (has_jit)
    {
        errorcode = pcre2_jit_compile(pat, 0);
        if (errorcode)
        {
            pcre2_get_error_message(errorcode, err_buff, sizeof(err_buff));
            char *e2 = sqlite3_mprintf("%s: %s", re, err_buff);
            sqlite3_result_error(ctx, e2, -1);
            sqlite3_free(e2);
            pcre2_code_free(pat);
            return nullptr;
        }
    }

The following examples do handle the error and fallback to the interpreter, however, do so in a suboptimal way:

PHP, generating a one time warning prior falling back to the interpreter:

	rc = pcre2_jit_compile(re, PCRE2_JIT_COMPLETE);
	if (EXPECTED(rc >= 0)) {
		size_t jit_size = 0;
		if (!pcre2_pattern_info(re, PCRE2_INFO_JITSIZE, &jit_size) && jit_size > 0) {
			poptions |= PREG_JIT;
		}
	} else if (rc == PCRE2_ERROR_NOMEMORY) {
		php_error_docref(NULL, E_WARNING,
			"Allocation of JIT memory failed, PCRE JIT will be disabled. "
			"This is likely caused by security restrictions. "
			"Either grant PHP permission to allocate executable memory, or set pcre.jit=0");
		PCRE_G(jit) = 0;
	} else {
		pcre2_get_error_message(rc, error, sizeof(error));
		php_error_docref(NULL, E_WARNING, "JIT compilation failed: %s", error);
		pcre_handle_exec_error(PCRE2_ERROR_INTERNAL);
	}

NGINX, generating a log message for each failed JIT compile if pcre_jit on was specified in the config and pcre2_config(PCRE2_CONFIG_JIT) returned 1 initially:

        n = pcre2_jit_compile(elts[i].regex, PCRE2_JIT_COMPLETE);

        if (n != 0) {
            ngx_log_error(NGX_LOG_INFO, cycle->log, 0,
                          "pcre2_jit_compile() failed: %d in \"%s\", "
                          "ignored",
                          n, elts[i].name);
        }

R, generating a warning once per compiled pattern (R_PCRE_use_JIT gets initialized to pcre2_config(PCRE2_CONFIG_JIT)):

    if (R_PCRE_use_JIT) {
	int rc = pcre2_jit_compile(*re, 0);
	if (rc && rc != PCRE2_ERROR_JIT_BADOPTION) {
	    /* PCRE2_ERROR_JIT_BADOPTION is returned when JIT support is not
	       compiled in PCRE2 library */
	    char buf[256];
	    pcre2_get_error_message(rc, (PCRE2_UCHAR *)buf, sizeof(buf));
	    warning(_("PCRE JIT compilation error\n\t'%s'"), buf);
	}
	if (!rc)
	    setup_jit(*mcontext);
    }

If, instead, PCRE2 would be changed as proposed, to do a test at runtime for PCRE2_CONFIG_JIT instead of simply mirroring the compile-time value, all of the currently failing projects would work out-of-the-box and no downstream patches would be needed.

The error handling examples would also be better off by avoiding unnecessary error message or even log flood spam, as is the case for NGINX or R.

That's why I'm asking you again to reconsider the (purely theoretical?) concerns about changing the semantics of pcre2_config(PCRE2_CONFIG_JIT) which wouldn't even be breaking the user contract, as the documentation doesn't specify it as a compile-time constant but as "Availability of just-in-time compiler" -- nowhere is specified that this would be a compile-time constant and, for the principle of least surprise, should have been implemented as a runtime check from the very beginning, IMHO.

From above examples it's also clear that the assumption of a lot of projects is that pcre2_config(PCRE2_CONFIG_JIT) returning 1 is, in fact, seen as a runtime availability check. Making it return 0 when the JIT code generation is guaranteed to always fail will "fix" these projects automatically instead of making them error out or generate volumes of log spam.

zherczeg · 2024-07-23T07:23:47Z

#433 is a good fix for this

minipli-oss · 2024-07-23T08:05:25Z

#433 is a good fix for this

No, it's not. It's adding a new option for pcre2_jit_compile() to explicitly support the "test if WX memory can be allocated" case but still leaves all current code (all of the examples I listed above) broken and requiring patches which my attempt tries to avoid -- simply for scalability reasons.

zherczeg · 2024-07-23T08:12:39Z

You can call it after pcre2_config(PCRE2_CONFIG_JIT) to do the extra check.

minipli-oss · 2024-07-23T12:05:11Z

Adding a call is completely missing the point of not having to modify the usage side.

minipli-oss force-pushed the prot_exec branch 2 times, most recently from 9dee7e8 to 6f5c4da Compare November 11, 2022 13:00

minipli-oss mentioned this pull request Nov 29, 2022

Prot exec fixes zherczeg/sljit#136

Open

minipli-oss added 3 commits December 16, 2022 09:38

minipli-oss force-pushed the prot_exec branch from 6f5c4da to c2dfa1e Compare December 16, 2022 09:10

SolitaryGrass mentioned this pull request May 31, 2023

internal_dfa_match, a stack overflow occurred due to recursive calls. #258

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Runtime JIT support detection #157

Runtime JIT support detection #157

minipli-oss commented Nov 8, 2022

zherczeg commented Nov 9, 2022

zherczeg commented Dec 16, 2022

minipli-oss commented Dec 16, 2022

zherczeg commented Dec 17, 2022

minipli-oss commented Dec 20, 2022

zherczeg commented Dec 20, 2022

minipli-oss commented Dec 21, 2022

zherczeg commented Dec 21, 2022

minipli-oss commented Dec 21, 2022

zherczeg commented Dec 21, 2022

minipli-oss commented Dec 21, 2022

PhilipHazel commented Dec 21, 2022

zherczeg commented Dec 21, 2022

minipli-oss commented Dec 22, 2022

zherczeg commented Dec 22, 2022

PhilipHazel commented Dec 22, 2022

minipli-oss commented Dec 23, 2022

zherczeg commented Dec 23, 2022

minipli-oss commented Feb 15, 2023

zherczeg commented Feb 18, 2023

minipli-oss commented Feb 21, 2023

zherczeg commented Feb 21, 2023

zherczeg commented Mar 1, 2023

minipli-oss commented Mar 1, 2023

minipli-oss commented Jul 22, 2024

zherczeg commented Jul 23, 2024

minipli-oss commented Jul 23, 2024

zherczeg commented Jul 23, 2024

minipli-oss commented Jul 23, 2024

Runtime JIT support detection #157

Are you sure you want to change the base?

Runtime JIT support detection #157

Conversation

minipli-oss commented Nov 8, 2022

zherczeg commented Nov 9, 2022

zherczeg commented Dec 16, 2022

minipli-oss commented Dec 16, 2022

zherczeg commented Dec 17, 2022

minipli-oss commented Dec 20, 2022

zherczeg commented Dec 20, 2022

minipli-oss commented Dec 21, 2022

zherczeg commented Dec 21, 2022

minipli-oss commented Dec 21, 2022

zherczeg commented Dec 21, 2022

minipli-oss commented Dec 21, 2022

PhilipHazel commented Dec 21, 2022

zherczeg commented Dec 21, 2022

minipli-oss commented Dec 22, 2022

zherczeg commented Dec 22, 2022

PhilipHazel commented Dec 22, 2022

minipli-oss commented Dec 23, 2022

zherczeg commented Dec 23, 2022

minipli-oss commented Feb 15, 2023

zherczeg commented Feb 18, 2023

minipli-oss commented Feb 21, 2023

zherczeg commented Feb 21, 2023

zherczeg commented Mar 1, 2023

minipli-oss commented Mar 1, 2023

minipli-oss commented Jul 22, 2024

zherczeg commented Jul 23, 2024

minipli-oss commented Jul 23, 2024

zherczeg commented Jul 23, 2024

minipli-oss commented Jul 23, 2024