Axel Beckert wrote:
> A local test on my /var/log/syslog immediately ran into a segfault,
> though, so I guess, that's one of the plugins you couldn't test.

I tested it but I guess results depend on the contents of the log;
apparently my log didn't trigger this code.

> Culprit is this line, actually the first line in my current
> /var/log/syslog:
> 
>   Dec  3 06:38:28 c6 syslog-ng[26651]: Configuration reload request
>   received, reloading configuration;

Many thanks for the reproducer.  Attached is patch that fixes it for
me.  I suspect there are other issues like these; in fact my pathethic
patch introduces a few memory leaks (I should have told you that in
advance).  The problem is that ccze code manipulates strings obtained
with pcre_get_substring, and assumes it can always use "free" to free
them.  That assumption is fine in the case with the old pcre, because
the definition of pcre_free_substring is just a wrapper around free
(src:pcre3: pcre_get.c:655):

void
pcre_free_substring(const char *pointer)
{
(PUBL(free))((void *)pointer);
}

So ccze code always uses "free".  However, in pcre2, the equivalent
function is different (src:pcre2: src/pcre2_substring.c:240):

void
pcre2_substring_free(PCRE2_UCHAR *string)
{
if (string != NULL)
  {
  pcre2_memctl *memctl = (pcre2_memctl *)((char *)string - 
sizeof(pcre2_memctl));
  memctl->free(memctl, memctl->memory_data);
  }
}

Attempting to use "free" to free a string obtained with
pcre2_subtring_get_by* results in SIGABRT with invalid pointer, while
using pcre2_substring_free on a string obtained with strdup or some
other standard (glibc) string manipulation functions results in
SIGSEGV.  Attached are two equivalent minimalistic programs that
demonstrate this: foo.c uses pcre2 and if you exchange the free
functions at the end for date and dup, you will observe exactly what I
wrote above.  The other program, bar.c, uses the old pcre and it
doesn't make any difference if you swap them or use only "free" as
ccze does.

I guess I need to revisit my patch and find some way to fix this.
It's up to you whether to upload the memleaky patch now or wait for me
to find a solution.  Sorry about this.
diff --git a/debian/patches/pcre2.patch b/debian/patches/pcre2.patch
index 6b60dc8..2723ff8 100644
--- a/debian/patches/pcre2.patch
+++ b/debian/patches/pcre2.patch
@@ -2455,26 +2455,32 @@ Last-Update: 2023-12-03
      }
        
    if (process)
-@@ -87,12 +87,13 @@
+@@ -60,7 +60,7 @@
+ 
+         pid = strndup (&t[1], (size_t)(t2 - t - 1));
+         tmp = strndup (process, (size_t)(t - process));
+-        free (process);
++        pcre2_substring_free (process);
+         process = tmp;
+       }
+     }
+@@ -87,11 +87,11 @@
    else
      toret = strdup (send);
  
 -  free (date);
 -  free (host);
 -  free (send);
--  free (process);
--  free (msg);
 +  pcre2_substring_free (date);
 +  pcre2_substring_free (host);
 +  pcre2_substring_free (send);
-+  pcre2_substring_free (process);
 +  pcre2_substring_free (msg);
+   free (process);
+-  free (msg);
    free (pid);
-+  free (tmp);
  
    return toret;
- }
-@@ -100,33 +101,34 @@
+@@ -100,33 +100,34 @@
  static void
  ccze_syslog_setup (void)
  {
#define PCRE2_CODE_UNIT_WIDTH 8
#include <pcre2.h>
#include <stdio.h>
#include <string.h>

int
main (void)
{
  pcre2_code *pcre;
  pcre2_match_data *md;
  int error;
  size_t errptr, l;
  char *date = NULL, *dup;
  const char *str = "Dec  3 06:38:28 c6 syslog-ng[26651]: Configuration "
    "reload request received, reloading configuration;";

  pcre = pcre2_compile ("^(\\S*\\s{1,2}\\d{1,2}\\s\\d\\d:\\d\\d:\\d\\d)"
                        "\\s(\\S+)\\s+((\\S+:?)\\s(.*))$",
                        PCRE2_ZERO_TERMINATED, 0, &error, &errptr, NULL);

  md = pcre2_match_data_create (99, NULL);

  pcre2_match (pcre, str, PCRE2_ZERO_TERMINATED, 0, 0, md, NULL);
  pcre2_substring_get_bynumber (md, 1, (unsigned char **)&date, &l);

  if (date)
    {
      printf ("%s\n", date);
      dup = strdup (date);
    }

  pcre2_substring_free (date);
  free (dup);

  return 0;
}
#include <pcre.h>
#include <stdio.h>
#include <string.h>

int
main (void)
{
  pcre *pcre;
  int md[99];
  const char *error;
  int errptr, match;
  char *date = NULL, *dup;
  const char *str = "Dec  3 06:38:28 c6 syslog-ng[26651]: Configuration "
    "reload request received, reloading configuration;";

  pcre = pcre_compile ("^(\\S*\\s{1,2}\\d{1,2}\\s\\d\\d:\\d\\d:\\d\\d)"
                       "\\s(\\S+)\\s+((\\S+:?)\\s(.*))$",
                       0, &error, &errptr, NULL);

  match = pcre_exec (pcre, NULL, str, strlen (str), 0, 0, md, 99);
  pcre_get_substring (str, md, match, 1, (const char **)&date);

  if (date)
    {
      printf ("%s\n", date);
      dup = strdup (date);
    }

  pcre_free_substring (date);
  free (dup);

  return 0;
}

Reply via email to