![]() |
|
Message-ID: <20240613134607.GW10433@brightrain.aerifal.cx> Date: Thu, 13 Jun 2024 09:46:07 -0400 From: Rich Felker <dalias@...c.org> To: erny hombre <hombre67@....at> Cc: musl@...ts.openwall.com Subject: Re: possible bug in syslog On Thu, Jun 13, 2024 at 01:44:40PM +0200, erny hombre wrote: > Hello, > > I think there is a bug in syslog: > > This assert fails (in gcc the test is ok): > #if (LOG_MAKEPRI(LOG_DAEMON, LOG_WARNING) != (LOG_DAEMON|LOG_WARNING)) > #error "LOG_MAKEPRI" > #endif This assertion itself is not valid; the whole reason a LOG_MAKEPRI macro exists is that it's not guaranteed that the packing be simple or. But the functionality it goes with is intended to work. > This code should produce a log message, but actually it does not: > setlogmask(LOG_UPTO(LOG_NOTICE)); > syslog(LOG_MAKEPRI(LOG_LOCAL7, LOG_NOTICE), "LOG_MAKEPRI(LOG_LOCAL7, LOG_NOTICE)"); > This works: > syslog(LOG_MAKEPRI(LOG_DAEMON, LOG_NOTICE), "LOG_MAKEPRI(LOG_DAEMON, LOG_NOTICE)"); > > --- > > Maybe these macros in syslog.h are wrong: > #define LOG_MAKEPRI(f, p) (((f)<<3)|(p)) > #define LOG_FACMASK 0x3f8 > #define LOG_FAC(p) (((p)&LOG_FACMASK)>>3) > > correct version: > #define LOG_MAKEPRI(f, p) (((f))|(p)) > #define LOG_FACMASK 0xf8 > #define LOG_FAC(p) (((p)&LOG_FACMASK)) > > Also a line in syslog.c, __vsyslog() should be changed > from: > if (!(log_mask & LOG_MASK(priority&7)) || (priority&~0x3ff)) return; > to: > if (!(log_mask & LOG_MASK(priority&7)) || (priority&~0xff)) return; > > ---- > This is my testprogram to reproduce the error: > > #include <syslog.h> > > int main(void) > { > #if 1 // test ok > #if (LOG_FAC(LOG_MAKEPRI(LOG_DAEMON, LOG_WARNING)) != LOG_DAEMON) > #error "LOG_FAC" > #endif > > #if 0 // test fails > #if (LOG_MAKEPRI(LOG_DAEMON, LOG_WARNING) != (LOG_DAEMON|LOG_WARNING)) > #error "LOG_MAKEPRI" > #endif > #endif > > setlogmask(LOG_UPTO(LOG_NOTICE)); > > openlog("exampleprog", LOG_CONS | LOG_PID | LOG_NDELAY | LOG_PERROR, LOG_LOCAL1); > > syslog(LOG_DAEMON | LOG_NOTICE, "LOG_DAEMON | LOG_NOTICE"); > syslog(LOG_LOCAL7 | LOG_NOTICE, "LOG_LOCAL7 | LOG_NOTICE"); > syslog(LOG_MAKEPRI(LOG_DAEMON, LOG_NOTICE), "LOG_MAKEPRI(LOG_DAEMON, LOG_NOTICE)"); > // the following call does not write a log message: > syslog(LOG_MAKEPRI(LOG_LOCAL7, LOG_NOTICE), "LOG_MAKEPRI(LOG_LOCAL7, LOG_NOTICE)"); > > closelog(); > > return 0; > } > ---- If I follow correctly, the bug here is that the nonstandard functionality of passing a facility to syslog() is not working because the facility bits are shifted into a mismatching position from where they're checked when packing them with priority. As for how to fix it, I assume your patch works, but it doesn't do anything for existing binaries built with the existing LOG_MAKEPRI macro, which will silently use the wrong facility. Another option would be to leave the macro definitions alone (i.e. preserve existing ABI) and make syslog accept the packing we inadvertently defined with the 3 empty bits between priority and facility. I'm not sure what we should do since this functionality has never worked right, and since it's fairly obscure and nonstandard. Let's wait a bit and see if anyone else has opinions on which way to fix it. Thanks for reporting. Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.