From patchwork Mon Jul 18 11:45:50 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Schwab X-Patchwork-Id: 1657401 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=KFhfgdCn; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=libc-alpha-bounces+incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4LmgCQ6QNzz9sFr for ; Mon, 18 Jul 2022 21:46:10 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 3EFEC3852770 for ; Mon, 18 Jul 2022 11:46:07 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 3EFEC3852770 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1658144767; bh=JWO9WuymitjNHc7gMWhdpU2UXO7LoDtkFmULLKFzqIs=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=KFhfgdCncvLDZebTguUJqocmfPtvy8r2n3hPIvJkh8I/HmHZHPLhMM9zsQ2WOoxG6 R6LQcskzmc0VBEszOrK56z8JLq3GVnv7Spt215/qymfkeXpfixwPDiHuq+ic7SMeEU y6PbSkxKLncvecOKrUIwCyTnqZGs6C98DVETUKEo= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2001:67c:2178:6::1d]) by sourceware.org (Postfix) with ESMTPS id A719F385276D for ; Mon, 18 Jul 2022 11:45:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A719F385276D Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id A745420AA4 for ; Mon, 18 Jul 2022 11:45:51 +0000 (UTC) Received: from hawking.suse.de (unknown [10.168.4.11]) by relay2.suse.de (Postfix) with ESMTP id A1F0B2C141 for ; Mon, 18 Jul 2022 11:45:51 +0000 (UTC) Received: by hawking.suse.de (Postfix, from userid 17005) id 05861444B2F; Mon, 18 Jul 2022 13:45:50 +0200 (CEST) To: libc-alpha@sourceware.org Subject: [PATCH] Always do locking when accessing streams (bug 15142, bug 14697) X-Yow: Gibble, Gobble, we ACCEPT YOU --- Date: Mon, 18 Jul 2022 13:45:50 +0200 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.1.90 (gnu/linux) MIME-Version: 1.0 X-Spam-Status: No, score=-8.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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: , X-Patchwork-Original-From: Andreas Schwab via Libc-alpha From: Andreas Schwab Reply-To: Andreas Schwab Errors-To: libc-alpha-bounces+incoming=patchwork.ozlabs.org@sourceware.org Sender: "Libc-alpha" Now that abort no longer calls fflush there is no reason to avoid locking the stdio streams anywhere. This fixes a conformance issue and potential heap corruption during exit. The test nptl/tst-stdio1 is removed as that was expecting the problematic behaviour. --- libio/genops.c | 43 +++++++--------------------- libio/libioP.h | 1 - sysdeps/pthread/Makefile | 2 +- sysdeps/pthread/tst-stdio1.c | 55 ------------------------------------ 4 files changed, 11 insertions(+), 90 deletions(-) delete mode 100644 sysdeps/pthread/tst-stdio1.c diff --git a/libio/genops.c b/libio/genops.c index 1b629eb695..21fb118eae 100644 --- a/libio/genops.c +++ b/libio/genops.c @@ -682,7 +682,7 @@ _IO_adjust_column (unsigned start, const char *line, int count) libc_hidden_def (_IO_adjust_column) int -_IO_flush_all_lockp (int do_lock) +_IO_flush_all (void) { int result = 0; FILE *fp; @@ -695,8 +695,7 @@ _IO_flush_all_lockp (int do_lock) for (fp = (FILE *) _IO_list_all; fp != NULL; fp = fp->_chain) { run_fp = fp; - if (do_lock) - _IO_flockfile (fp); + _IO_flockfile (fp); if (((fp->_mode <= 0 && fp->_IO_write_ptr > fp->_IO_write_base) || (_IO_vtable_offset (fp) == 0 @@ -706,8 +705,7 @@ _IO_flush_all_lockp (int do_lock) && _IO_OVERFLOW (fp, EOF) == EOF) result = EOF; - if (do_lock) - _IO_funlockfile (fp); + _IO_funlockfile (fp); run_fp = NULL; } @@ -718,14 +716,6 @@ _IO_flush_all_lockp (int do_lock) return result; } - - -int -_IO_flush_all (void) -{ - /* We want locking. */ - return _IO_flush_all_lockp (1); -} libc_hidden_def (_IO_flush_all) void @@ -791,6 +781,9 @@ _IO_unbuffer_all (void) { int legacy = 0; + run_fp = fp; + _IO_flockfile (fp); + #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1) if (__glibc_unlikely (_IO_vtable_offset (fp) != 0)) legacy = 1; @@ -800,18 +793,6 @@ _IO_unbuffer_all (void) /* Iff stream is un-orientated, it wasn't used. */ && (legacy || fp->_mode != 0)) { -#ifdef _IO_MTSAFE_IO - int cnt; -#define MAXTRIES 2 - for (cnt = 0; cnt < MAXTRIES; ++cnt) - if (fp->_lock == NULL || _IO_lock_trylock (*fp->_lock) == 0) - break; - else - /* Give the other thread time to finish up its use of the - stream. */ - __sched_yield (); -#endif - if (! legacy && ! dealloc_buffers && !(fp->_flags & _IO_USER_BUF)) { fp->_flags |= _IO_USER_BUF; @@ -825,17 +806,15 @@ _IO_unbuffer_all (void) if (! legacy && fp->_mode > 0) _IO_wsetb (fp, NULL, NULL, 0); - -#ifdef _IO_MTSAFE_IO - if (cnt < MAXTRIES && fp->_lock != NULL) - _IO_lock_unlock (*fp->_lock); -#endif } /* Make sure that never again the wide char functions can be used. */ if (! legacy) fp->_mode = -1; + + _IO_funlockfile (fp); + run_fp = NULL; } #ifdef _IO_MTSAFE_IO @@ -861,9 +840,7 @@ libc_freeres_fn (buffer_free) int _IO_cleanup (void) { - /* We do *not* want locking. Some threads might use streams but - that is their problem, we flush them underneath them. */ - int result = _IO_flush_all_lockp (0); + int result = _IO_flush_all (); /* We currently don't have a reliable mechanism for making sure that C++ static destructors are executed in the correct order. diff --git a/libio/libioP.h b/libio/libioP.h index ba4fdbd200..88342b1bfa 100644 --- a/libio/libioP.h +++ b/libio/libioP.h @@ -487,7 +487,6 @@ extern int _IO_new_do_write (FILE *, const char *, size_t); extern int _IO_old_do_write (FILE *, const char *, size_t); extern int _IO_wdo_write (FILE *, const wchar_t *, size_t); libc_hidden_proto (_IO_wdo_write) -extern int _IO_flush_all_lockp (int); extern int _IO_flush_all (void); libc_hidden_proto (_IO_flush_all) extern int _IO_cleanup (void); diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile index 7d1670da87..b88914b755 100644 --- a/sysdeps/pthread/Makefile +++ b/sysdeps/pthread/Makefile @@ -111,7 +111,7 @@ tests += tst-cnd-basic tst-mtx-trylock tst-cnd-broadcast \ tst-signal4 tst-signal5 tst-signal6 tst-signal8 \ tst-spin1 tst-spin2 tst-spin3 tst-spin4 \ tst-stack1 \ - tst-stdio1 tst-stdio2 \ + tst-stdio2 \ tst-pt-sysconf \ tst-pt-tls1 tst-pt-tls2 \ tst-tsd1 tst-tsd2 tst-tsd5 tst-tsd6 \ diff --git a/sysdeps/pthread/tst-stdio1.c b/sysdeps/pthread/tst-stdio1.c deleted file mode 100644 index 568e516a30..0000000000 --- a/sysdeps/pthread/tst-stdio1.c +++ /dev/null @@ -1,55 +0,0 @@ -/* Copyright (C) 2002-2022 Free Software Foundation, Inc. - This file is part of the GNU C Library. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, see - . */ - -#include -#include -#include -#include - -static int do_test (void); - -#define TEST_FUNCTION do_test () -#include "../test-skeleton.c" - -static void *tf (void *a) -{ - flockfile (stdout); - /* This call should never return. */ - return a; -} - - -int -do_test (void) -{ - pthread_t th; - - flockfile (stdout); - - if (pthread_create (&th, NULL, tf, NULL) != 0) - { - write_message ("create failed\n"); - _exit (1); - } - - delayed_exit (1); - xpthread_join (th); - - puts ("join returned"); - - return 1; -}