Re: Syslog bug

[Alex Kost]

Ludovic Courtès (2015-03-31 00:20 +0300) wrote:

> Alex Kost <address@hidden> skribis:
>
>> Nope, syslog.conf in store is fine.  The problem can be definitely
>> solved for me by removing leading spaces.  Actually never mind, as I'm
>> going to use my config for syslog-service, but anyway here is what
>> happens when I start syslogd with the conf-file of the same contents as
>> provided by 'syslog-service':
>>
>> # /run/current-system/profile/libexec/syslogd --debug --rcfile 
>> /tmp/syslog-with-leading-spaces.conf
>> init
>> cfline(*.aauth.noth.notice;authpriv.none       /dev/console)
>> syslogd: unknown priority name "aauth.noth.notice"
>> (logmsg): syslog.err (43), flags 4, from localhost, msg syslogd: unknown 
>> priority name "aauth.noth.notice"
>> Logging to CONSOLE /dev/console
>> cfline(*.iail.none.none;authpriv.none          /var/log/messages)
>> syslogd: unknown priority name "iail.none.none"
>
> Ooh, you found a genuine bug, as evidenced by the corrupt strings above.
>
> Confirmed with Valgrind:
>
>
> $ valgrind ./src/syslogd --debug --rcfile 
> /gnu/store/cz9n7s884mlr5y4x2bk8kq9hq44nnmaz-syslog.conf
> ==29582== Memcheck, a memory error detector
> ==29582== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
> ==29582== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
> ==29582== Command: ./src/syslogd --debug --rcfile 
> /gnu/store/cz9n7s884mlr5y4x2bk8kq9hq44nnmaz-syslog.conf
> ==29582== 
> init
> ==29582== Source and destination overlap in strcpy(0x55ebc00, 0x55ebc05)
> ==29582==    at 0x4C29C02: strcpy (in 
> /gnu/store/13dzn85z8yhh6i977lwsii4wd7zjzyka-valgrind-3.10.1/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==29582==    by 0x405D1B: load_conffile.constprop.5 (syslogd.c:1974)
> ==29582==    by 0x4060CF: init (syslogd.c:2109)
> ==29582==    by 0x402BAC: main (syslogd.c:601)
>
> This patch placates Valgrind and seems to do the job:
>
>
> diff --git a/src/syslogd.c b/src/syslogd.c
> index 7af10f3..1db4455 100644
> --- a/src/syslogd.c
> +++ b/src/syslogd.c
> @@ -1971,7 +1971,7 @@ load_conffile (const char *filename, struct filed 
> **nextp)
>        if (*p == '\0' || *p == '#')
>       continue;
>  
> -      strcpy (cline, p);
> +      bcopy (p, cline, strlen (p));
>  
>        /* Cut the trailing spaces.  */
>        for (p = strchr (cline, '\0'); isspace (*--p);)
>
> Could you confirm that it solves the problem for you?  Just add it
> locally to the inetutils recipe.

Wow, I admire how deep you dig!  Your patch does some funny thing, it
"moves" corruption to another place.  Here is the output:

--8<---------------cut here---------------start------------->8---
# ./syslogd --debug --rcfile /tmp/syslog-with-leading-spaces.conf
init
cfline(*.alert;auth.notice;authpriv.none       /dev/console
sole)
cfline(*.info;mail.none;authpriv.none          /var/log/messages
ages)
cfline(*.info;mail.none;authpriv.none          /dev/tty12
ty12)
cfline(authpriv.*                              /var/log/secure
cure)
cfline(mail.*                                  /var/log/maillog
llog)
cannot open 
/gnu/store/2k3qc6lff23y0kq51rw5b0kwpq4x7q54-inetutils-1.9.2/etc/syslog.d
 X  X ff  X  X  X  X  X  X  X  X  X  X  X  X  X  X  X  X  X  X  X  X  X  X 
FILE: /var/log/maillog
llog
 X  X  X  X  X  X  X  X  X  X ff  X  X  X  X  X  X  X  X  X  X  X  X  X  X 
FILE: /var/log/secure
cure
7f 7f  X 7f 7f 7f 7f 7f 7f 7f  X 7f 7f 7f 7f 7f 7f 7f 7f 7f 7f 7f 7f 7f 7f 
FILE: /dev/tty12
ty12
7f 7f  X 7f 7f 7f 7f 7f 7f 7f  X 7f 7f 7f 7f 7f 7f 7f 7f 7f 7f 7f 7f 7f 7f 
FILE: /var/log/messages
ages
 3  3  3  3 3f  3  3  3  3  3  X  3  3  3  3  3  3  3  3  3  3  3  3  3  3 
FILE: /dev/console
sole
(logmsg): syslog.info (46), flags 4, from localhost, msg syslogd (GNU inetutils 
1.9.2): restart
Logging to FILE /dev/tty12
ty12
Logging to FILE /var/log/messages
ages
syslogd: restarted
Klog open /proc/kmsg
Opened UNIX socket `/dev/log'.
off & running....
Debugging is disabled. Send SIGUSR1 to PID=21422 to turn on debugging.
--8<---------------cut here---------------end--------------->8---

Look *close* at the file names :-)  This version of syslogd created the
following files on my system:

- "/var/log/maillog
llog",

- "/var/log/messages
ages", etc.

With a line feed character inside (i.e., "/var/log/messages^Jages").

The following modified patch fixes the issue for me.  However my C fu is
almost absent, so I'm not sure about side effects and if there is a
better way (perhaps, it would be better just to write a null character
after moving 'p' to 'cline').

--- a/src/syslogd.c
+++ b/src/syslogd.c
@@ -1971,7 +1971,7 @@ load_conffile (const char *filename, struct filed **nextp)
       if (*p == '\0' || *p == '#')
        continue;
 
-      strcpy (cline, p);
+      strncpy (cline, p, strlen (cline));
 
       /* Cut the trailing spaces.  */
       for (p = strchr (cline, '\0'); isspace (*--p);)

A side note: compilation of inetutils failed for me complaining about
missing "help2man".  It finished successfully after I had added
"help2man" to native-inputs.

-- 
Alex