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 46C8729847 for ; Thu, 13 Jun 2024 17:37:20 +0200 (CEST) Received: (qmail 13574 invoked by uid 550); 13 Jun 2024 15:37:16 -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 13542 invoked from network); 13 Jun 2024 15:37:16 -0000 Date: Thu, 13 Jun 2024 11:37:31 -0400 From: Rich Felker To: Thorsten Glaser Cc: musl@lists.openwall.com, erny hombre Message-ID: <20240613153729.GY10433@brightrain.aerifal.cx> References: <20240613134607.GW10433@brightrain.aerifal.cx> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit 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 03:20:11PM +0000, Thorsten Glaser wrote: > Rich Felker dixit: > > >If I follow correctly, the bug here is that the nonstandard > >functionality of passing a facility to syslog() > > At least on BSD, it is expected to work: > > | The facility parameter encodes a default facility to be assigned to all > | messages that do not have an explicit facility encoded: > > Although it also has… > > | #define LOG_MAKEPRI(fac, pri) (((fac) << 3) | (pri)) > > … the uses are… > > | bin/date/date.c: syslog(LOG_AUTH | LOG_NOTICE, "date set by %s", pc); > > … and… > > | #define LOG_USER (1<<3) /* random user-level messages */ > > … so I guess it suffers from the same bug. > > $ ./a.out > exampleprog[11414]: LOG_DAEMON | LOG_NOTICE > exampleprog[11414]: LOG_LOCAL7 | LOG_NOTICE > exampleprog[11414]: LOG_MAKEPRI(LOG_DAEMON, LOG_NOTICE) > exampleprog[11414]: syslog: unknown facility/priority: 5c5 > exampleprog[11414]: LOG_MAKEPRI(LOG_LOCAL7, LOG_NOTICE) > > The only users of LOG_MAKEPRI in the entire tree are Sys::Syslog::Syslog > (Perl XS) though and… > > sys/sys/syslog.h:#define INTERNAL_MARK LOG_MAKEPRI(LOG_NFACILITIES, 0) > sys/sys/syslog.h: { "mark", INTERNAL_MARK }, /* INTERNAL */ > > … so this is probably fixed more easily than when having > to keep ABI. LOG_FAC has one more user than Perl, but… > > /* > * Don't allow users to log kernel messages. > * NOTE: since LOG_KERN == 0 this will also match > * messages with no facility specified. > */ > if (LOG_FAC(pri) == LOG_KERN) > pri = LOG_USER | LOG_PRI(pri); > > … this is also very nicely written to not hit this bug. > > LOG_FACMASK must of course match the next power-of-two-minus-one > for (LOG_NFACILITIES << 3)… so, yes, 0xF8 is correct for BSD as well. > > What do you think about… > > #define LOG_MAKEPRI(fac,pri) (((fac) & LOG_FACMASK) | ((pri) & LOG_PRIMASK)) > > … however? That seems okay. However I also kinda wonder if LOG_MAKEPRI shouldn't just be removed. This would highlight anything that was previously broken, and there seems to be utterly no reason to use it when the standard already specifies that you just or (I missed that before). > With the three changes applied, the test program is happy: > > $ ./a.out > exampleprog[6837]: LOG_DAEMON | LOG_NOTICE > exampleprog[6837]: LOG_LOCAL7 | LOG_NOTICE > exampleprog[6837]: LOG_MAKEPRI(LOG_DAEMON, LOG_NOTICE) > exampleprog[6837]: LOG_MAKEPRI(LOG_LOCAL7, LOG_NOTICE) > > >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. > > This would not bring the MAKEPRI and the “or” method into > congruence, and, assuming you actually test that the three > bits are 0 and at least one higher isn’t, it would still mean > LOG_UUCP/LOG_USER, LOG_LOCAL0/LOG_MAIL, LOG_NFACILITIES/LOG_DAEMON > (strictly speaking) confusion. Indeed, I was wrong about this and missed the standard requirement (which TBF is kinda buried; the text in XBD for syslog.h says the facilities are for openlog not for openlog and syslog). Rich