From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.1 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 Received: from second.openwall.net (second.openwall.net [193.110.157.125]) by inbox.vuxu.org (Postfix) with SMTP id 112D6220DA for ; Thu, 13 Jun 2024 15:45:58 +0200 (CEST) Received: (qmail 30207 invoked by uid 550); 13 Jun 2024 13:45:54 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 30167 invoked from network); 13 Jun 2024 13:45:54 -0000 Date: Thu, 13 Jun 2024 09:46:07 -0400 From: Rich Felker To: erny hombre Cc: musl@lists.openwall.com Message-ID: <20240613134607.GW10433@brightrain.aerifal.cx> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] 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 > > 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