Re: [musl] possible bug in syslog

[Rich Felker <dalias@libc.org>]

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