Re: [PATCH v2] grep: fall back to interpreter if JIT memory allocation fails

> Mathias Krause <minipli@grsecurity.net> writes:
> 
>> As having a functional PCRE2 JIT compiler is a legitimate use case for
>> performance reasons, we'll only do the fallback if the supposedly
>> available JIT is found to be non-functional by attempting to JIT compile
>> a very simple pattern. If this fails, JIT is deemed to be non-functional
>> and we do the interpreter fallback. For all other cases, i.e. the simple
>> pattern can be compiled but the user provided cannot, we fail hard as we
>> do now as the reason for the failure must be the pattern itself.
> 
> I do not know if it is a good idea to rely on the "very simple
> pattern".  The implementation of JIT could devise various ways to
> succeed for such simple patterns without having writable-executable
> piece of memory.

> 
>> +static int pcre2_jit_functional(void)
>> +{
>> +	static int jit_working = -1;
>> +	pcre2_code *code;
>> +	size_t off;
>> +	int err;
>> +
>> +	if (jit_working != -1)
>> +		return jit_working;
>> +
>> +	/*
>> +	 * Try to JIT compile a simple pattern to probe if the JIT is
>> +	 * working in general. It might fail for systems where creating
>> +	 * memory mappings for runtime code generation is restricted.
>> +	 */
>> +	code = pcre2_compile((PCRE2_SPTR)".", 1, 0, &err, &off, NULL);
>> +	if (!code)
>> +		return 0;
>> +
>> +	jit_working = pcre2_jit_compile(code, PCRE2_JIT_COMPLETE) == 0;
>> +	pcre2_code_free(code);
> 
> I'd prefer not having to worry about: Or it might not fail for such
> systems, as the pattern is too simple and future versions of
> pcre2_compile() could have special case code.

> 
>> @@ -317,8 +342,23 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt
>>  	pcre2_config(PCRE2_CONFIG_JIT, &p->pcre2_jit_on);
>>  	if (p->pcre2_jit_on) {
>>  		jitret = pcre2_jit_compile(p->pcre2_pattern, PCRE2_JIT_COMPLETE);
>> -		if (jitret)
>> +		if (jitret == PCRE2_ERROR_NOMEMORY && !pcre2_jit_functional()) {
>> +			/*
>> +			 * Even though pcre2_config(PCRE2_CONFIG_JIT, ...)
>> +			 * indicated JIT support, the library might still
>> +			 * fail to generate JIT code for various reasons,
>> +			 * e.g. when SELinux's 'deny_execmem' or PaX's
>> +			 * MPROTECT prevent creating W|X memory mappings.
>> +			 *
>> +			 * Instead of faling hard, fall back to interpreter
>> +			 * mode, just as if the pattern was prefixed with
>> +			 * '(*NO_JIT)'.
>> +			 */
>> +			p->pcre2_jit_on = 0;
>> +			return;
> 
> Yes, the "instead of failing hard, fall back" makes sense.  Just
> that I do not see why the runtime test is a good thing to have.

>                                                                  In
> short, we are not in the business of catching bugs in pcre2_jit
> implementations, so if they say they cannot compile the pattern (I
> would even say I doubt the point of checking the return code to
> ensure it is NOMEMORY), it would be fine to let the interpreter
> codepath to inspect the pattern and diagnose problems with it, or
> take the slow match without JIT.