From patchwork Mon Jun 29 14:30:58 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Schwab X-Patchwork-Id: 39827 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id DA5C3388E839; Mon, 29 Jun 2020 14:31:02 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id 63E84388C011 for ; Mon, 29 Jun 2020 14:30:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 63E84388C011 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=schwab@suse.de X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 3796BACF0 for ; Mon, 29 Jun 2020 14:30:58 +0000 (UTC) From: Andreas Schwab To: libc-alpha@sourceware.org Subject: [PATCH] Correct locking and cancellation cleanup in syslog functions (bug 26100) X-Yow: Two with FLUFFO, hold th' BEETS..side of SOYETTES! Date: Mon, 29 Jun 2020 16:30:58 +0200 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.91 (gnu/linux) MIME-Version: 1.0 X-Spam-Status: No, score=-8.0 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" Properly serialize the access to the global state shared between the syslog functions, to avoid races in multithreaded processes. Protect a local allocation in the __vsyslog_internal function from leaking during cancellation. Reviewed-by: Adhemerval Zanella --- misc/syslog.c | 44 ++++++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/misc/syslog.c b/misc/syslog.c index fd6537edf6..2cc63ef287 100644 --- a/misc/syslog.c +++ b/misc/syslog.c @@ -91,14 +91,20 @@ struct cleanup_arg static void cancel_handler (void *ptr) { -#ifndef NO_SIGPIPE /* Restore the old signal handler. */ struct cleanup_arg *clarg = (struct cleanup_arg *) ptr; - if (clarg != NULL && clarg->oldaction != NULL) - __sigaction (SIGPIPE, clarg->oldaction, NULL); + if (clarg != NULL) + { +#ifndef NO_SIGPIPE + if (clarg->oldaction != NULL) + __sigaction (SIGPIPE, clarg->oldaction, NULL); #endif + /* Free the memstream buffer, */ + free (clarg->buf); + } + /* Free the lock. */ __libc_lock_unlock (syslog_lock); } @@ -169,9 +175,17 @@ __vsyslog_internal(int pri, const char *fmt, va_list ap, pri &= LOG_PRIMASK|LOG_FACMASK; } + /* Prepare for multiple users. We have to take care: most + syscalls we are using are cancellation points. */ + struct cleanup_arg clarg; + clarg.buf = NULL; + clarg.oldaction = NULL; + __libc_cleanup_push (cancel_handler, &clarg); + __libc_lock_lock (syslog_lock); + /* Check priority against setlogmask values. */ if ((LOG_MASK (LOG_PRI (pri)) & LogMask) == 0) - return; + goto out; /* Set default facility if none specified. */ if ((pri & LOG_FACMASK) == 0) @@ -235,6 +249,9 @@ __vsyslog_internal(int pri, const char *fmt, va_list ap, /* Close the memory stream; this will finalize the data into a malloc'd buffer in BUF. */ fclose (f); + + /* Tell the cancellation handler to free this buffer. */ + clarg.buf = buf; } /* Output to stderr if requested. */ @@ -252,22 +269,10 @@ __vsyslog_internal(int pri, const char *fmt, va_list ap, v->iov_len = 1; } - __libc_cleanup_push (free, buf == failbuf ? NULL : buf); - /* writev is a cancellation point. */ (void)__writev(STDERR_FILENO, iov, v - iov + 1); - - __libc_cleanup_pop (0); } - /* Prepare for multiple users. We have to take care: open and - write are cancellation points. */ - struct cleanup_arg clarg; - clarg.buf = buf; - clarg.oldaction = NULL; - __libc_cleanup_push (cancel_handler, &clarg); - __libc_lock_lock (syslog_lock); - #ifndef NO_SIGPIPE /* Prepare for a broken connection. */ memset (&action, 0, sizeof (action)); @@ -320,6 +325,7 @@ __vsyslog_internal(int pri, const char *fmt, va_list ap, __sigaction (SIGPIPE, &oldaction, (struct sigaction *) NULL); #endif + out: /* End of critical section. */ __libc_cleanup_pop (0); __libc_lock_unlock (syslog_lock); @@ -430,8 +436,14 @@ setlogmask (int pmask) { int omask; + /* Protect against multiple users. */ + __libc_lock_lock (syslog_lock); + omask = LogMask; if (pmask != 0) LogMask = pmask; + + __libc_lock_unlock (syslog_lock); + return (omask); }