From: Peter Krempa <[email protected]>

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 <[email protected]>
---
 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

Reply via email to