The syslogd daemon seems to be leaking few bytes of memory when modifications are done to the /etc/syslog.conf when it has entries to forward the syslog messages to a remote-host, and making syslogd read the new conf by signalling it with a SIGHUP. Consider the following entries in /etc/syslog.conf # $FreeBSD: releng/10.1/etc/syslog.conf 260519 2014-01-10 17:56:23Z asomers $ # # Spaces ARE valid field separators in this file. However, # other *nix-like systems still insist on using tabs as field # separators. If you are sharing this file between systems, you # may want to use only tabs as field separators here. # Consult the syslog.conf(5) manpage. *.err;kern.warning;auth.notice;mail.crit /dev/console *.notice;authpriv.none;kern.debug;lpr.info;mail.crit;news.err /var/log/messages security.* /var/log/security auth.info;authpriv.info /var/log/auth.log mail.info /var/log/maillog lpr.info /var/log/lpd-errs ftp.info /var/log/xferlog cron.* /var/log/cron !-devd *.=debug /var/log/debug.log *.emerg * # uncomment this to log all writes to /dev/console to /var/log/console.log # touch /var/log/console.log and chmod it to mode 600 before it will work #console.info /var/log/console.log # uncomment this to enable logging of all log messages to /var/log/all.log # touch /var/log/all.log and chmod it to mode 600 before it will work #*.* /var/log/all.log # uncomment this to enable logging to a remote loghost named loghost #*.* @loghost *.info @192.168.1.40 *.info @192.168.1.41 *.info @192.168.1.42 *.info @192.168.1.43 Now, the last three lines above are replaced with a line with different host, So, the file (at those lines) looks like this: *.info @192.168.1.40 *.info @192.168.1.51 << Two lines removed Now, I issue the 'kill -HUP <syslogd-pid>' This would re-read this conf file, by first flushing out the current entries. During this situation, the syslogd seems to be leaking few bytes of memory. This is because, whenever syslogd reads the conf-file, and finds an entry for remote-host, it issues a 'getaddrinfo' to get the name resolution for that entry. getaddrinfo() will internally allocated memory for linked-list of addresses related to that host, and will return the same. The caller is supposed to free that memory by issuing freeaddrinfo() once his task is done. The portion of code where getaddrinfo() is called in syslogd.c file of FreeBSD-10.1 is: 1776 1777 /* 1778 * Crack a configuration file line 1779 */ 1780 static void 1781 cfline(const char *line, struct filed *f, const char *prog, const char *host) 1782 { 1783 struct addrinfo hints, *res; 1784 int error, i, pri, syncfile; 1785 const char *p, *q; 1786 char *bp; ... ... 1939 1940 switch (*p) { 1941 case '@': 1942 { 1943 char *tp; 1944 char endkey = ':'; ... ... 1974 memset(&hints, 0, sizeof(hints)); 1975 hints.ai_family = family; 1976 hints.ai_socktype = SOCK_DGRAM; 1977 error = getaddrinfo(f->f_un.f_forw.f_hname, 1978 p ? p : "syslog", &hints, &res); 1979 if (error) { 1980 logerror(gai_strerror(error)); 1981 break; 1982 } 1983 f->f_un.f_forw.f_addr = res; 1984 f->f_type = F_FORW; 1985 break; The address is collected and set ready for remote-logging the syslog messages. Now, when changes are done to syslog.conf file, and SIGHUP issued, the init() function gets called, and in this function, arrangements to free up this memory returned by getaddrinfo() is not made. The init() function looks like this: 1533 1534 /* 1535 * INIT -- Initialize syslogd from configuration table 1536 */ 1537 static void 1538 init(int signo) 1539 { 1540 int i; 1541 FILE *cf; 1542 struct filed *f, *next, **nextp; 1543 char *p; 1544 char cline[LINE_MAX]; ... ... 1577 switch (f->f_type) { 1578 case F_FILE: 1579 case F_FORW: <<<<<<< No arrangements to call freeaddrinfo() 1588 case F_CONSOLE: 1581 case F_TTY: 1582 (void)close(f->f_file); 1583 break; 1584 case F_PIPE: 1585 if (f->f_un.f_pipe.f_pid > 0) { 1586 (void)close(f->f_file); 1587 deadq_enter(f->f_un.f_pipe.f_pid, 1588 f->f_un.f_pipe.f_pname); 1589 } 1590 f->f_un.f_pipe.f_pid = 0; 1591 break; 1592 } This results in a memory leak. Although this could be of few bytes, it would be nice to fix this, as the fix is simple. I have made the changes to syslogd.c (in my FreeBSD box) to fix this issue. The changes look like this: 1577 switch (f->f_type) { 1578 case F_FILE: 1579 case F_FORW: 1580 if (f->f_un.f_forw.f_addr) { <<< 1581 freeaddrinfo(f->f_un.f_forw.f_addr); <<< Call freeaddrinfo 1583 } <<< 1584 /* fall through */ <<< 1585 case F_CONSOLE: 1586 case F_TTY: 1587 (void)close(f->f_file); 1588 break; 1589 case F_PIPE: 1590 if (f->f_un.f_pipe.f_pid > 0) { 1591 (void)close(f->f_file); 1592 deadq_enter(f->f_un.f_pipe.f_pid, 1593 f->f_un.f_pipe.f_pname); 1594 } 1595 f->f_un.f_pipe.f_pid = 0; 1596 break; 1597 } Request: I would like to take up this task of fixing this issue. I have never contributed to FreeBSD-code before this, and this is a great opportunity for me to fix this and contribute. But, I request you to kindly suggest me the steps followed by other developers in FreeBSD for fixing BUGs. Please inform. Regards, Sreeram
Sreeram, please let me know if you are still interested in pursuing a fix to this bug. I can help you with that if so. One change I would suggest in your proposed patch is to exchange the order of the F_FILE and F_FORW cases so that the clean up applicable to F_FORW is not also done for the F_FILE case. Granted, the "if (f->f_un.f_forw.f_addr)" test would (should) fail in the F_FILE case, but it would still be cleaner not to attempt it at all. Also, it would be very helpful if you could rebase your patch on the current HEAD branch, which is where it would first be integrated into the FreeBSD code base. If you do not want to pursue this fix, I will do so myself, based on the information you have provided so far. Thank you.
A commit references this bug: Author: dab Date: Mon Feb 26 19:27:59 UTC 2018 New revision: 330034 URL: https://svnweb.freebsd.org/changeset/base/330034 Log: Fix two memory leaks in syslogd A memory leak in syslogd for processing of forward actions was reported. This modification adapts the patch submitted with that bug to fix the leak. While testing the modification, another leak was also found and fixed. PR: 198385 Submitted by: Sreeram <sreeramabs@yahoo.com> Reported by: Sreeram <sreeramabs@yahoo.com> Reviewed by: hrs MFC after: 1 week Sponsored by: Dell EMC Differential Revision: https://reviews.freebsd.org/D14510 Changes: head/usr.sbin/syslogd/syslogd.c
A commit references this bug: Author: dab Date: Thu Mar 8 16:26:49 UTC 2018 New revision: 330661 URL: https://svnweb.freebsd.org/changeset/base/330661 Log: MFC r330034 Fix a memory leak in syslogd A memory leak in syslogd for processing of forward actions was reported. This modification adapts the patch submitted with that bug to fix the leak. PR: 198385 Submitted by: Sreeram <sreeramabs@yahoo.com> Reported by: Sreeram <sreeramabs@yahoo.com> Sponsored by: Dell EMC Changes: _U stable/11/ stable/11/usr.sbin/syslogd/syslogd.c
A commit references this bug: Author: dab Date: Thu Mar 8 17:14:16 UTC 2018 New revision: 330664 URL: https://svnweb.freebsd.org/changeset/base/330664 Log: MFC r330034 Fix a memory leak in syslogd A memory leak in syslogd for processing of forward actions was reported. This modification adapts the patch submitted with that bug to fix the leak. PR: 198385 Submitted by: Sreeram <sreeramabs@yahoo.com> Reported by: Sreeram <sreeramabs@yahoo.com> Sponsored by: Dell EMC Changes: _U stable/10/ stable/10/usr.sbin/syslogd/syslogd.c