git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mathias Krause <minipli@grsecurity.net>
To: git@vger.kernel.org
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Mathias Krause" <minipli@grsecurity.net>,
	"Carlo Marcelo Arenas Belón" <carenas@gmail.com>
Subject: [PATCH v2] grep: fall back to interpreter if JIT memory allocation fails
Date: Fri, 27 Jan 2023 16:49:52 +0100	[thread overview]
Message-ID: <20230127154952.485913-1-minipli@grsecurity.net> (raw)
In-Reply-To: <20221216121557.30714-1-minipli@grsecurity.net>

Under Linux systems with SELinux's 'deny_execmem' or PaX's MPROTECT
enabled, the allocation of PCRE2's JIT rwx memory may be prohibited,
making pcre2_jit_compile() fail with PCRE2_ERROR_NOMEMORY (-48):

  [user@fedora git]$ git grep -c PCRE2_JIT
  grep.c:1

  [user@fedora git]$ # Enable SELinux's W^X policy
  [user@fedora git]$ sudo semanage boolean -m -1 deny_execmem

  [user@fedora git]$ # JIT memory allocation fails, breaking 'git grep'
  [user@fedora git]$ git grep -c PCRE2_JIT
  fatal: Couldn't JIT the PCRE2 pattern 'PCRE2_JIT', got '-48'

Instead of failing hard in this case and making 'git grep' unusable on
such systems, simply fall back to interpreter mode, leading to a much
better user experience.

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.

Cc: Carlo Marcelo Arenas Belón <carenas@gmail.com>
Signed-off-by: Mathias Krause <minipli@grsecurity.net>
---

This patch is based on a previous attempt proposed by Carlo already 4
years ago[1]. However, it wasn't applied as there were still ongoing
discussions about how to handle and possibly avoid the automatic
fallback.

A follow-up RFC had been posted half a year later[2], adding a config
option and, after some more discussion, even command line switches[3].
But, after all, it was agreed on that this is far too much and Junio
suggested to simply revert back to the initial RFC and implement the
automatic fallback[4], basically merging it with a proper changelog[5].
As that never happened, I took up the work and tried to do just that.

This, again, lead to some discussion but, fortunately, less about the
config knobs, but more about the implications of such a change. I tried
to address Ævar's concerns about always falling back to the interpreter
and limited it to the problematic case I want to get solved.

1. https://lore.kernel.org/r/20181209230024.43444-3-carenas@gmail.com
2. https://lore.kernel.org/git/20190728235427.41425-1-carenas@gmail.com/
3. https://lore.kernel.org/git/20190729105955.44390-1-carenas@gmail.com/
4. https://lore.kernel.org/git/xmqqh874vikk.fsf@gitster-ct.c.googlers.com/
5. https://lore.kernel.org/git/xmqqef1zmkp5.fsf@gitster-ct.c.googlers.com/

Changes in v2:

The current version takes a conservative approach by only implementing
the fallback to interpreter mode when the runtime test for basic JIT
support fails as well, indicating the inability of PCRE2's memory
allocator to acquire a W|X mappings for runtime code generation.

pcre2_jit_functional() very much intentional calls pcre2_compile()
without making use of the context set up in compile_pcre2_pattern() to
exclude any errors related to that -- a wrong combination of options
making pcre2_jit_compile() fail for these reasons instead of the memory
mapping error we try to detect. This ensures we're doing a dumb simple
JIT compile test to probe its general runtime availability and not
wrongly fall back to interpreter mode because the option combination
we're trying to make use of isn't supported by the JIT.

I also changed the author to myself as the current state has little in
common with what Carlo once proposed.
---
 grep.c | 42 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/grep.c b/grep.c
index 06eed694936c..59afc3f07fc9 100644
--- a/grep.c
+++ b/grep.c
@@ -262,6 +262,31 @@ static void pcre2_free(void *pointer, MAYBE_UNUSED void *memory_data)
 	free(pointer);
 }
 
+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);
+
+	return jit_working;
+}
+
 static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt)
 {
 	int error;
@@ -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;
+		} else if (jitret) {
 			die("Couldn't JIT the PCRE2 pattern '%s', got '%d'\n", p->pattern, jitret);
+		}
 
 		/*
 		 * The pcre2_config(PCRE2_CONFIG_JIT, ...) call just
-- 
2.39.0


  parent reply	other threads:[~2023-01-27 15:50 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-16 12:15 [PATCH] grep: fall back to interpreter mode if JIT fails Mathias Krause
2022-12-16 16:12 ` Ævar Arnfjörð Bjarmason
2022-12-16 19:26   ` Mathias Krause
2022-12-16 23:09     ` Junio C Hamano
2022-12-17  2:50       ` Carlo Arenas
2022-12-19  9:00         ` Ævar Arnfjörð Bjarmason
2022-12-20 19:29           ` Mathias Krause
2022-12-20 21:11             ` Ævar Arnfjörð Bjarmason
2023-01-18 14:22               ` Mathias Krause
2023-01-18 15:44                 ` Ævar Arnfjörð Bjarmason
2023-01-19  9:19                   ` Mathias Krause
2022-12-16 22:52 ` Junio C Hamano
2022-12-20 20:40   ` Mathias Krause
2023-01-27 15:49 ` Mathias Krause [this message]
2023-01-27 16:34   ` [PATCH v2] grep: fall back to interpreter if JIT memory allocation fails Junio C Hamano
2023-01-27 17:39     ` Junio C Hamano
2023-01-27 18:46       ` Junio C Hamano
2023-01-29 13:37         ` Mathias Krause
2023-01-29 13:36       ` Mathias Krause
2023-01-29 17:15         ` Junio C Hamano
2023-01-30 10:56           ` Ævar Arnfjörð Bjarmason
2023-01-30 18:49             ` Junio C Hamano
2023-01-31  8:34               ` Ævar Arnfjörð Bjarmason
2023-01-30 11:08           ` Mathias Krause
2023-01-30 18:54             ` Junio C Hamano
2023-01-30 20:08               ` Junio C Hamano
2023-01-30 21:21                 ` Junio C Hamano
2023-01-30 22:30                   ` Ramsay Jones
2023-01-30 23:27                     ` Junio C Hamano
2023-01-31  7:48                   ` Mathias Krause
2023-01-31 16:41                     ` Junio C Hamano
2023-01-31 18:34                       ` Mathias Krause
2023-01-31  7:30                 ` Mathias Krause
2023-01-29 12:28     ` Mathias Krause
2023-01-31 18:56   ` [PATCH v3] " Mathias Krause
2023-01-31 21:05     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230127154952.485913-1-minipli@grsecurity.net \
    --to=minipli@grsecurity.net \
    --cc=avarab@gmail.com \
    --cc=carenas@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).