[PATCH 0/2] Deal with G_REGEX_OPTIMIZE pitfalls
G_REGEX_OPTIMIZE invokes PCRE's JIT optimizer for regexes which requires writable executable memory. This clashes with the 'execmem' SELinux permission which we don't really want to grant to any of the virt daemons. Since all uses of G_REGEX_OPTIMIZE are on code paths unlikely to hit the ~3x performance penalty of using interpeted evaluation of the regex rather than the JIT'd one we can simply remove the optimization from the rest of the calls. (Only 3 of 10 used it). Patch 2 is optional; selinux can simply ban 'execmem' and mark it as 'dontaudit'. glib will fall back to the interpreter. The patches are based on a discussion which I've summarized in: https://redhat.atlassian.net/browse/RHEL-44901 Peter Krempa (2): syntax-check: Add warning about implications of G_REGEX_OPTIMIZE Avoid use of glib's G_REGEX_OPTIMIZE flag build-aux/syntax-check.mk | 13 +++++++++++++ src/conf/domain_event.c | 2 +- src/util/vircommand.c | 2 +- src/util/virlog.c | 2 +- 4 files changed, 16 insertions(+), 3 deletions(-) -- 2.54.0
From: Peter Krempa <pkrempa@redhat.com> glib's G_REGEX_OPTIMIZE flag for regex operations implies the use of pcre's JIT optimizer for regexes which requires writable executable memory for the optimized code. Security frameworks such as SELinux can restrict program's access to writable executable memory to harden the code. glib's implementation of the regex functions handles downgrade to interpreted evaluation of regexes gracefully if writable executable memory is unavailable, but that is non-obvious and has a performance penalty. Add a syntax check which requires all uses of G_REGEX_OPTIMIZE to be annotated so that the reader of the code can be hinted to why G_REGEX_OPTIMIZE should perhaps be avoided. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- build-aux/syntax-check.mk | 12 ++++++++++++ src/conf/domain_event.c | 2 +- src/util/vircommand.c | 2 +- src/util/virlog.c | 2 +- 4 files changed, 15 insertions(+), 3 deletions(-) diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index 6e22212944..4ad2a53779 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -1387,6 +1387,15 @@ sc_prohibit_inline_functions: halt='avoid inline functions in .c files' \ $(_sc_search_regexp) +# glib's G_REGEX_OPTIMIZE instructs the PCRE library to use JIT optimization +# of the regexes, which requires executable memory to work. Security mechanisms +# such as SELinux may disallow it. glib gracefully falls back to un-optimized +# code. This check serves as a warning to not rely on G_REGEX_OPTIMIZE: +sc_G_REGEX_OPTIMIZE: + @prohibit='G_REGEX_OPTIMIZE' \ + exclude='see sc_G_REGEX_OPTIMIZE' \ + halt='Optimizations may not work everywhere. Read the warning in build-aux/syntax-check.mk' \ + $(_sc_search_regexp) ## ---------- ## ## Exceptions ## @@ -1550,6 +1559,9 @@ exclude_file_name_regexp--sc_spacing-check = \ exclude_file_name_regexp--sc_prohibit_inline_functions = \ ^src/storage_file/storage_source.*.c$$ +exclude_file_name_regexp--sc_G_REGEX_OPTIMIZE = \ + ^build-aux/syntax-check\.mk$$ + ## -------------- ## ## Implementation ## ## -------------- ## diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index a6bf9735aa..434401a5ef 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -2643,7 +2643,7 @@ virDomainQemuMonitorEventStateRegisterID(virConnectPtr conn, data->flags = flags; if (event && flags != -1) { if (flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX) { - int cflags = G_REGEX_OPTIMIZE; + int cflags = G_REGEX_OPTIMIZE; /* see sc_G_REGEX_OPTIMIZE */ g_autoptr(GError) err = NULL; if (flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_NOCASE) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 1a9875fc87..84d4d9c218 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -3270,7 +3270,7 @@ virCommandRunRegex(virCommand *cmd, for (i = 0; i < nregex; i++) { g_autoptr(GError) err = NULL; - reg[i] = g_regex_new(regex[i], G_REGEX_OPTIMIZE, 0, &err); + reg[i] = g_regex_new(regex[i], G_REGEX_OPTIMIZE, 0, &err); /* see sc_G_REGEX_OPTIMIZE */ if (!reg[i]) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to compile regex %1$s"), err->message); diff --git a/src/util/virlog.c b/src/util/virlog.c index 02b8dd1385..8a6cb939bc 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -253,7 +253,7 @@ virLogOnceInit(void) virLogLock(); virLogDefaultPriority = VIR_LOG_DEFAULT; - virLogRegex = g_regex_new(VIR_LOG_REGEX, G_REGEX_OPTIMIZE, 0, NULL); + virLogRegex = g_regex_new(VIR_LOG_REGEX, G_REGEX_OPTIMIZE, 0, NULL); /* see sc_G_REGEX_OPTIMIZE */ /* GLib caches the hostname using a one time thread initializer. * We want to prime this cache early though, because at later time -- 2.54.0
From: Peter Krempa <pkrempa@redhat.com> glib's G_REGEX_OPTIMIZE invokes pcre's JIT optimizer which requires writable executable memory. On hosts with SELinux this by default caused AVC denials to be logged. The solution in Fedora's SELinux policy was to allow execmem for e.g. virtqemud, but e.g. virtlogd or libvirt_leaseshelper on the other hand got a 'dontaudit' rule. Now the optimizer itself does have a substantial effect; I've measured around 3x speedup when matching VIR_LOG_REGEX against a sample of a qemu VM log file. On the other hand, only 3 out of 10 uses of 'g_regex_new' used the flag and also none of the regexes which used 'G_REGEX_OPTIMIZE' are on a hot path: - virDomainQemuMonitorEventStateRegisterID Used only when custom qemu monitor event callback is registered, which resides in libvirt_qemu.so so noone will actually use this in production. - virCommandRunRegex Used in: - virStorageBackendFileSystemNetFindNFSPoolSources Invoked only via API on output of 'showmount'. Unlikely to ever see lots of data. - virStorageBackendLogicalFindLVs - virStorageBackendLogicalGetPoolSources - virStorageBackendLogicalRefreshPool Invoked on output of lvs/pvs/vgs. Neither of them are likely to ever see lots of output to match. - virISCSIGetSession - virISCSIScanTargetsInternal Invoked on output of iscsiadm, unlikely to see lots of data. - virLogProbablyLogMessage - virLXCProcessIgnorableLogLine - domainLogContextReadFiltered Both of the above are used on code paths processing log output when failure to startup a VM process or one of the helper processes for devices for a qemu domain. Thus they are not invoked on success and the processed buffer is capped to 1k of data. Since none of the above are on anything resembling a hot path and thus likely to substantially benefit from the optimizer, and we do have plenty of other uses without optimization, drop the optimizations everywhere and add a note to sc_G_REGEX_OPTIMIZE that we're currently avoiding the use of this flag. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- build-aux/syntax-check.mk | 3 ++- src/conf/domain_event.c | 2 +- src/util/vircommand.c | 2 +- src/util/virlog.c | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index 4ad2a53779..a049c47ea7 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -1390,7 +1390,8 @@ sc_prohibit_inline_functions: # glib's G_REGEX_OPTIMIZE instructs the PCRE library to use JIT optimization # of the regexes, which requires executable memory to work. Security mechanisms # such as SELinux may disallow it. glib gracefully falls back to un-optimized -# code. This check serves as a warning to not rely on G_REGEX_OPTIMIZE: +# code. This check serves as a warning to not rely on G_REGEX_OPTIMIZE. +# Currently we avoid use of G_REGEX_OPTIMIZE anywhere. sc_G_REGEX_OPTIMIZE: @prohibit='G_REGEX_OPTIMIZE' \ exclude='see sc_G_REGEX_OPTIMIZE' \ diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 434401a5ef..1d9796a1aa 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -2643,7 +2643,7 @@ virDomainQemuMonitorEventStateRegisterID(virConnectPtr conn, data->flags = flags; if (event && flags != -1) { if (flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX) { - int cflags = G_REGEX_OPTIMIZE; /* see sc_G_REGEX_OPTIMIZE */ + int cflags = 0; g_autoptr(GError) err = NULL; if (flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_NOCASE) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 84d4d9c218..984481c613 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -3270,7 +3270,7 @@ virCommandRunRegex(virCommand *cmd, for (i = 0; i < nregex; i++) { g_autoptr(GError) err = NULL; - reg[i] = g_regex_new(regex[i], G_REGEX_OPTIMIZE, 0, &err); /* see sc_G_REGEX_OPTIMIZE */ + reg[i] = g_regex_new(regex[i], 0, 0, &err); if (!reg[i]) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to compile regex %1$s"), err->message); diff --git a/src/util/virlog.c b/src/util/virlog.c index 8a6cb939bc..7e767e8424 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -253,7 +253,7 @@ virLogOnceInit(void) virLogLock(); virLogDefaultPriority = VIR_LOG_DEFAULT; - virLogRegex = g_regex_new(VIR_LOG_REGEX, G_REGEX_OPTIMIZE, 0, NULL); /* see sc_G_REGEX_OPTIMIZE */ + virLogRegex = g_regex_new(VIR_LOG_REGEX, 0, 0, NULL); /* GLib caches the hostname using a one time thread initializer. * We want to prime this cache early though, because at later time -- 2.54.0
On a Monday in 2026, Peter Krempa via Devel wrote:
G_REGEX_OPTIMIZE invokes PCRE's JIT optimizer for regexes which requires writable executable memory. This clashes with the 'execmem' SELinux permission which we don't really want to grant to any of the virt daemons.
Since all uses of G_REGEX_OPTIMIZE are on code paths unlikely to hit the ~3x performance penalty of using interpeted evaluation of the regex rather than the JIT'd one we can simply remove the optimization from the rest of the calls. (Only 3 of 10 used it).
Patch 2 is optional; selinux can simply ban 'execmem' and mark it as 'dontaudit'. glib will fall back to the interpreter.
The patches are based on a discussion which I've summarized in: https://redhat.atlassian.net/browse/RHEL-44901
Peter Krempa (2): syntax-check: Add warning about implications of G_REGEX_OPTIMIZE Avoid use of glib's G_REGEX_OPTIMIZE flag
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa