From patchwork Tue Oct 8 19:02:03 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tulio Magno Quites Machado Filho X-Patchwork-Id: 1994395 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=ascii.art.br header.i=@ascii.art.br header.a=rsa-sha256 header.s=dreamhost header.b=Oyon1oX8; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=server2.sourceware.org; envelope-from=libc-alpha-bounces~incoming=patchwork.ozlabs.org@sourceware.org; receiver=patchwork.ozlabs.org) Received: from server2.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 ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4XNQQ60vDrz1xsv for ; Wed, 9 Oct 2024 06:02:53 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id DEC1A384F01B for ; Tue, 8 Oct 2024 19:02:51 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from zebra.cherry.relay.mailchannels.net (zebra.cherry.relay.mailchannels.net [23.83.223.195]) by sourceware.org (Postfix) with ESMTPS id 30C2D385C6CC for ; Tue, 8 Oct 2024 19:02:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 30C2D385C6CC Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=ascii.art.br Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=ascii.art.br ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 30C2D385C6CC Authentication-Results: server2.sourceware.org; arc=pass smtp.remote-ip=23.83.223.195 ARC-Seal: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1728414150; cv=pass; b=GbSw6VrQYTybRl4EGeJD1Mo8r3tJqIeqYMBDKZtBaTsMwN8DflJPsMA8ygR3LeubVoDHvVKK2z1aFTw2c6IMmwktf4dZaKjkz5zQVRjFAx+RNHHc+kpstTBYwwvVs+bkpRUP2BzhcUcbYKuZvieQt4e5EUhWzGbvNN/BRSTqre8= ARC-Message-Signature: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1728414150; c=relaxed/simple; bh=KcL9qaKYQbZL6BaQ1x/D5073vfEXrPejktxaSHx6PqI=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=Ul6ZkDk9lmnonBjTrYC74yuSoUdS+of6LAu90d++NSDjN66pgw7zlAiB7VSnBQzrbFhQTnHziFSYRzYix6R8EXtkr20z9qnqMEi3D7MESL+s0eUA8m0lCbXoYqY79UQWY7QvOEWMOXLlwgf6w7eL59NZub4stjOcE1dULF2Oos4= ARC-Authentication-Results: i=2; server2.sourceware.org X-Sender-Id: dreamhost|x-authsender|tuliom@ascii.art.br Received: from relay.mailchannels.net (localhost [127.0.0.1]) by relay.mailchannels.net (Postfix) with ESMTP id 12E07906B17 for ; Tue, 8 Oct 2024 19:02:26 +0000 (UTC) Received: from pdx1-sub0-mail-a288.dreamhost.com (trex-12.trex.outbound.svc.cluster.local [100.101.192.86]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id CC2E3906804 for ; Tue, 8 Oct 2024 19:02:21 +0000 (UTC) ARC-Seal: i=1; s=arc-2022; d=mailchannels.net; t=1728414141; a=rsa-sha256; cv=none; b=HQve69w29oI5v6wmLMqe5SIeC3nQgi8NB7h5YIa9igt4PnTf5HWU+VE1vLXEzZtIL8hlx0 qpGdDsXo7eSSTxeMv5hRLwrxzrxZnu/si0dVo7j7esKsafL4skT2jIFz0fOQSXPL2n/Xsz 3nquB5WWnSxNzT3CFfSxtlHvs8KOUPi6B3tkz39I3T16S/dED1y2YV16R3PrcufHLvD+xd AVv4ilMqhLqa5ksPI7+OWn1ZIeistFHAjTpus4SN5qdU7T8qDleJpWPgBOQNpJ8n9DhtaA 4WhgHwkKnH/NnhmcRhglPHmt28hGiEsPO5Yqqw04mrDfOnYagutm8vWcFP+gcg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=mailchannels.net; s=arc-2022; t=1728414141; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding:dkim-signature; bh=ET8YhG4FtPFrDFdCfSoncZI6w2u0xAbZJ1YiIqvzr9I=; b=6x0lxZ0NgpfBHGwdYxqAD+6hZrqfrb7yGfjaH1tFlVGd+teWJYFrBCOiYv/P61EyICgod7 7RMJmzDSpuE7hBT/K0xOqXy5PwMEbjGW7mJAnFQUMLynKkB0zT/2enyyW9gzjDcMWJa7KZ xJGZuRpHr47upGUl68ThZhSQEGYO6m9UferS7KOR1gzeZmRuvTqGuzJeh/had5w2aRLiWy g+4VHqmNQAnDJxddhqC+CaKq275Z+ByXXuYkQ+0iJwJhF9xpFk+UWuwzY0Z/csctV4bCc6 GB4O700pvAwWfYjR+0ZyQJxy60gSUn1YdyzlVeDoJj535g0Ft2w8KcbMhQFstQ== ARC-Authentication-Results: i=1; rspamd-54f99b5bc4-kmmxb; auth=pass smtp.auth=dreamhost smtp.mailfrom=tuliom@ascii.art.br X-Sender-Id: dreamhost|x-authsender|tuliom@ascii.art.br X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|tuliom@ascii.art.br X-MailChannels-Auth-Id: dreamhost X-Society-Reaction: 667f05a87948789f_1728414145994_2674333048 X-MC-Loop-Signature: 1728414145994:1276183034 X-MC-Ingress-Time: 1728414145994 Received: from pdx1-sub0-mail-a288.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384) by 100.101.192.86 (trex/7.0.2); Tue, 08 Oct 2024 19:02:25 +0000 Received: from ascii.art.br (ip-191-5-84-47.isp.valenet.com.br [191.5.84.47]) (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) (Authenticated sender: tuliom@ascii.art.br) by pdx1-sub0-mail-a288.dreamhost.com (Postfix) with ESMTPSA id 4XNQPT2bKfzGQ for ; Tue, 8 Oct 2024 12:02:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ascii.art.br; s=dreamhost; t=1728414141; bh=ET8YhG4FtPFrDFdCfSoncZI6w2u0xAbZJ1YiIqvzr9I=; h=From:To:Subject:Date:Content-Transfer-Encoding; b=Oyon1oX8mvGOROg7OnR1XCnnhp5+l+ZfWjVXxslstpBCHHpxMa3Z5YEfQav/X43c/ Br7NpAQOJfsyyIbBbmnEQIjClfyHUZNinqyP1uuDaoT3xpKRdlxiN/gqcfKlg5dhWh qLznfv1W+f2rMkhGydksZrADMSGXZmf3D443UlQNJ059HtzWBaXFZtRB7U680Rm1Bi WfxF0BN8nQP0DSgNmoa9dxdTeOVR0C6rGXA9RY7BQIH3kRKOnB9wRSeaJkVdQtHMv1 64l8CHkBCrZv3gb+2VAJhNrQSvo4iRKeHdpLXhCsxmlqjl8DmUeJTevOV8CNZd8orj 150GAqckpJ58Q== From: Tulio Magno Quites Machado Filho To: libc-alpha@sourceware.org Subject: [PATCH] libio: Start to return errors when flushing fwrite's buffer Date: Tue, 8 Oct 2024 16:02:03 -0300 Message-ID: <20241008190203.2856619-1-tuliom@ascii.art.br> X-Mailer: git-send-email 2.46.2 MIME-Version: 1.0 X-Spam-Status: No, score=-8.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, 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.30 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libc-alpha-bounces~incoming=patchwork.ozlabs.org@sourceware.org From: Tulio Magno Quites Machado Filho When an error happens, fwrite is expected to return a value that is less than nmemb. If this error happens while flushing its internal buffer, fwrite is in a complex scenario: all the data might have been written to the buffer, indicating a successful copy, but the buffer is expected to be flushed and it was not. POSIX.1-2024 states the following about errors on fwrite: If an error occurs, the resulting value of the file-position indicator for the stream is unspecified. The fwrite() function shall return the number of elements successfully written, which may be less than nitems if a write error is encountered. With that in mind, this commit modifies fwrite's behavior in case of an error while flushing the buffer in order to return 0 in case an irrecoverable error is found. Contents copied to the buffer are kept there despite the return value because the error is irrecoverable. In case of recoverable errors, fwrite continues to return success (aka. nmemb), keeping the old behavior and allowing the caller to try again. Add 2 tests: 1. tst-fwrite-bz29459: This test is based on the reproducer attached to bug 29459. In order to work, it requires to pipe stdout to another process making it hard to reuse test-driver.c. This code is more specific to the issue reported. 2. tst-fwrite-pipe: Recreates the issue by creating a pipe that is shared with a child process. Reuses test-driver.c. Evaluates a more generic scenario. --- libio/iofwrite.c | 23 +++-- stdio-common/Makefile | 7 ++ stdio-common/tst-fwrite-bz29459.c | 89 ++++++++++++++++++++ stdio-common/tst-fwrite-bz29459.sh | 34 ++++++++ stdio-common/tst-fwrite-pipe.c | 130 +++++++++++++++++++++++++++++ 5 files changed, 278 insertions(+), 5 deletions(-) create mode 100644 stdio-common/tst-fwrite-bz29459.c create mode 100755 stdio-common/tst-fwrite-bz29459.sh create mode 100644 stdio-common/tst-fwrite-pipe.c diff --git a/libio/iofwrite.c b/libio/iofwrite.c index af2e2070aff..8c0323947ae 100644 --- a/libio/iofwrite.c +++ b/libio/iofwrite.c @@ -38,12 +38,25 @@ _IO_fwrite (const void *buf, size_t size, size_t count, FILE *fp) if (_IO_vtable_offset (fp) != 0 || _IO_fwide (fp, -1) == -1) written = _IO_sputn (fp, (const char *) buf, request); _IO_release_lock (fp); - /* We have written all of the input in case the return value indicates - this or EOF is returned. The latter is a special case where we - simply did not manage to flush the buffer. But the data is in the - buffer and therefore written as far as fwrite is concerned. */ - if (written == request || written == EOF) + if (written == request) + /* We have written all of the input successfully. */ return count; + else if (written == EOF) + { + /* sputn() has the same semantics as fputs(), returning EOF on error. + It also means we did not manage to flush the buffer, but the data is + in the buffer and therefore written, which is a conflicting + scenario. + + Confirm that an irrecoverable error happened and return an error, + i.e. return less than count. + Otherwise, return success (aka. count) and let the caller try + again, which is the behavior that fwrite had for years. */ + if ((fp->_flags & _IO_ERR_SEEN) && errno != 0 && errno != EAGAIN) + return 0; + else + return count; + } else return written / size; } diff --git a/stdio-common/Makefile b/stdio-common/Makefile index 88105b3c1b3..bb5a8b0462d 100644 --- a/stdio-common/Makefile +++ b/stdio-common/Makefile @@ -234,6 +234,7 @@ tests := \ tst-fwrite \ tst-fwrite-memstrm \ tst-fwrite-overflow \ + tst-fwrite-pipe \ tst-fwrite-ro \ tst-getline \ tst-getline-enomem \ @@ -316,6 +317,7 @@ tests-internal = \ # tests-internal test-srcs = \ + tst-fwrite-bz29459 \ tst-printf \ tst-printfsz-islongdouble \ tst-unbputc \ @@ -323,6 +325,7 @@ test-srcs = \ ifeq ($(run-built-tests),yes) tests-special += \ + $(objpfx)tst-fwrite-bz29459.out \ $(objpfx)tst-printf.out \ $(objpfx)tst-printfsz-islongdouble.out \ $(objpfx)tst-setvbuf1-cmp.out \ @@ -517,6 +520,10 @@ tst-freopen64-6-ENV = \ MALLOC_TRACE=$(objpfx)tst-freopen64-6.mtrace \ LD_PRELOAD=$(common-objpfx)malloc/libc_malloc_debug.so +$(objpfx)tst-fwrite-bz29459.out: tst-fwrite-bz29459.sh $(objpfx)tst-fwrite-bz29459 + $(SHELL) $< $(common-objpfx) '$(test-program-prefix)'; \ + $(evaluate-test) + $(objpfx)tst-unbputc.out: tst-unbputc.sh $(objpfx)tst-unbputc $(SHELL) $< $(common-objpfx) '$(test-program-prefix)'; \ $(evaluate-test) diff --git a/stdio-common/tst-fwrite-bz29459.c b/stdio-common/tst-fwrite-bz29459.c new file mode 100644 index 00000000000..7ced6d419ca --- /dev/null +++ b/stdio-common/tst-fwrite-bz29459.c @@ -0,0 +1,89 @@ +/* Test fwrite against bug 29459. + Copyright (C) 2024 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 + . */ + +/* This test is based on the code attached to bug 29459. + It depends on stdout being redirected to a specific process via a script + with the same name. Because of this, we cannot use the features from + test_driver.c. */ + +#include +#include +#include +#include +#include +#include +#include + +/* Usually this test reproduces in a few iterations. However, keep a high + number of iterations in order to avoid return false-positives due to an + overwhelmed/slow system. */ +#define ITERATIONS 5000 + +/* The goal of this test is to use fwrite () on a redirected and closed + stdout. A script will guarantee that stdout is redirected to another + process that closes it during the execution. The process reading from + the pipe must read at least the first line in order to guarantee that + flag _IO_CURRENTLY_PUTTING is set in the write end of the pipe, triggering + important parts of the code that flushes lines from fwrite's internal + buffer. The underlying write () returns EPIPE, which fwrite () must + propagate. */ + +int +main (void) +{ + int i; + size_t rc; + /* Ensure the string we send has a new line because we're dealing + with a lined-buffered stream. */ + const char *s = "hello\n"; + const size_t len = strlen(s); + + /* Ensure that fwrite buffers the output before writing to stdout. */ + setlinebuf(stdout); + /* Ignore SIGPIPE in order to catch the EPIPE returned by the + underlying call to write(). */ + xsignal(SIGPIPE, SIG_IGN); + + for (i = 1; i <= ITERATIONS; i++) + { + /* Keep writing to stdout. Success means that fwrite () returns an + error. */ + if ((rc = fwrite(s, 1, len, stdout)) < len) + { + /* An error happened. Check if ferror () does return an error + and that it is indeed EPIPE. */ + TEST_COMPARE (ferror (stdout), 1); + TEST_COMPARE (errno, EPIPE); + fprintf(stderr, "Success: i=%d. fwrite returned %zu < %zu \ +and errno=EPIPE\n", + i, rc, len); + /* The test succeeded! */ + return 0; + } + else + { + /* fwrite () was able to write all the contents. Check if no errors + have been reported and try again. */ + TEST_COMPARE (ferror (stdout), 0); + TEST_COMPARE (errno, 0); + } + } + + fprintf(stderr, "Error: fwrite did not return an error\n"); + return 1; +} diff --git a/stdio-common/tst-fwrite-bz29459.sh b/stdio-common/tst-fwrite-bz29459.sh new file mode 100755 index 00000000000..c63e94bd6bf --- /dev/null +++ b/stdio-common/tst-fwrite-bz29459.sh @@ -0,0 +1,34 @@ +#!/bin/sh +# Test fwrite for bug 29459. +# Copyright (C) 2024 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 +# . + +set -e + +common_objpfx=$1; shift +test_program_prefix=$1; shift + +status=0 + +${test_program_prefix} \ + ${common_objpfx}stdio-common/tst-fwrite-bz29459 \ + 2> ${common_objpfx}stdio-common/tst-fwrite-bz29459.out \ + | head -n1 > /dev/null + +grep -q Success ${common_objpfx}stdio-common/tst-fwrite-bz29459.out || status=1 + +exit $status diff --git a/stdio-common/tst-fwrite-pipe.c b/stdio-common/tst-fwrite-pipe.c new file mode 100644 index 00000000000..7f88e962655 --- /dev/null +++ b/stdio-common/tst-fwrite-pipe.c @@ -0,0 +1,130 @@ +/* Test if fwrite returns EPIPE. + Copyright (C) 2024 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 +#include +#include +#include + +/* Usually this test reproduces in a few iterations. However, keep a high + number of iterations in order to avoid return false-positives due to an + overwhelmed/slow system. */ +#define ITERATIONS 5000 + +#define BUFFERSIZE 20 + +/* When the underlying write () fails with EPIPE, fwrite () is expected to + return an error by returning < size*nmemb and keeping errno=EPIPE. */ + +static int +do_test (void) +{ + int fd[2]; + pid_t p; + FILE *f; + size_t written; + int ret = 1; /* Return failure by default. */ + + /* Try to create a pipe. */ + xpipe (fd); + + p = xfork (); + if (p == 0) + { + char b[BUFFERSIZE]; + size_t bytes; + + /* Read at least the first line from the pipe before closing it. + This is important because it guarantees the file stream will have + flag _IO_CURRENTLY_PUTTING set, which triggers important parts of + the code that flushes lines from fwrite's internal buffer. */ + do { + bytes = read (fd[0], b, BUFFERSIZE); + } while(memrchr (b, '\n', bytes) == NULL); + + /* Child closes both ends of the pipe in order to trigger an EPIPE + error on the parent. */ + xclose (fd[0]); + xclose (fd[1]); + + return 0; + } + else + { + /* Ensure the string we send has a new line because we're dealing + with a lined-buffered stream. */ + const char *s = "hello\n"; + size_t len = strlen (s); + int i; + + /* Parent only writes to pipe. + Close the unused read end of the pipe. */ + xclose (fd[0]); + + /* Ignore SIGPIPE in order to catch the EPIPE returned by the + underlying call to write(). */ + xsignal(SIGPIPE, SIG_IGN); + + /* Create a file stream associated with the write end of the pipe. */ + f = fdopen (fd[1], "w"); + TEST_VERIFY_EXIT (f != NULL); + /* Ensure that fwrite buffers the output before writing to the pipe. */ + setlinebuf (f); + + /* Ensure errno is not set before starting. */ + TEST_VERIFY_EXIT (errno == 0); + for (i = 1; i <= ITERATIONS; i++) + { + /* Try to write to the pipe. The first calls are expected to + suceeded until the child process closes the read end. + After that, fwrite () is expected to fail and errno should be + set to EPIPE. */ + written = fwrite (s, 1, len, f); + + if (written == len) + { + TEST_VERIFY_EXIT (ferror (f) == 0); + TEST_VERIFY_EXIT (errno == 0); + } + else + { + /* An error happened. Check if ferror () does return an error + and that it is indeed EPIPE. */ + TEST_COMPARE (ferror (f), 1); + TEST_COMPARE (errno, EPIPE); + /* The test succeeded! Clear the error from the file stream and + return success. */ + clearerr (f); + ret = 0; + break; + } + } + + xfclose (f); + } + + if (ret) + FAIL_RET ("fwrite should have returned an error, but it didn't.\n"); + + return ret; +} + +#include