From patchwork Tue Jan 14 02:03:05 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joseph Myers X-Patchwork-Id: 104726 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 8614B3857709 for ; Tue, 14 Jan 2025 02:07:37 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8614B3857709 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=PMshv1hG X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id 14567385842A for ; Tue, 14 Jan 2025 02:03:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 14567385842A Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 14567385842A Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1736820192; cv=none; b=RrJhB6fQw2se4+TAf3UUZeaZ8Yy5l3jQSZYR0pUd4u0FPxaIRuFP060/3D1h8BE+qw1fbAPxyF/8BfQJkMQ8o078y/rYhmubmLKt4ZVaFtp0E34Edu0VULxILr0MIeWQxA8UsaJYjWNBjY97dICmxOkWE3P1w5W7kEqiG1mJu2Y= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1736820192; c=relaxed/simple; bh=eKBtwM6Zpm1iy2jnz8ibNxPqG0olxw6z+au8TeD2D0g=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=uKX1l9k4ifJPReApREj8uG2RY71niR4GBMXykA1Sk8tsqJqQwaWzTJh21KkO+JfX0XICJnSffm1u57wWZTxW1dHBfD8iHo0YtAIr1sQ6+6wuZ9omWcpiP76Diyun9Q+IW5TnfA41C9elfvhdbX7JfVwzplLIiAEcaJ+0t9IP5uA= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 14567385842A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1736820191; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=t3/GOv57K+MyFbq42b4mYJpKhCTtZmxQwSeQa1fTzF8=; b=PMshv1hGZhxgUV/tilVIKcpne6pd83Hi2k4waLpIoLaB8Cxeh3FZVGH/aR8Zd354h3T96s ox72B8WvyfYZuLInu1OUtjl+GHuyyh/a3R1VHTm0LKnwwU0O0DsCDWoamqhUu6eXSfFnPR +qWJBwMC7OWclLIomCVDSi6u2P80nY4= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-427-IpTPNadlM3GtpDF7cECIZA-1; Mon, 13 Jan 2025 21:03:10 -0500 X-MC-Unique: IpTPNadlM3GtpDF7cECIZA-1 X-Mimecast-MFC-AGG-ID: IpTPNadlM3GtpDF7cECIZA Received: by mail-wm1-f71.google.com with SMTP id 5b1f17b1804b1-43582d49dacso33940165e9.2 for ; Mon, 13 Jan 2025 18:03:10 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736820189; x=1737424989; h=mime-version:references:message-id:in-reply-to:subject:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=t3/GOv57K+MyFbq42b4mYJpKhCTtZmxQwSeQa1fTzF8=; b=rzd2UpUtkwdJ1nwmFBgRhzAthKVmyMehmDFk/uBcFLpBPBU7VFoDTr1WUdKJ7ueRH1 TIZR16UsL2R+7q3AOg+gDBDlBtPcrldI2m87eZRuJcW+GslkwrrjDBM8v3NCGkdiAa2g +1xzZCFxam0bsntMMWWbhCddzMHmHwTqHClytkpskh6OK1P5VYamy7gIJYcaxebqxNRX 6Q2x3EyNv4BbbpeenaOqbkYvorAeqyTcSiSKNjiMXmepm67nK4p2l2yINPY7GL1N375L 5UcllKkawBrAs9fd/Eo//fZfB/8Zf2nfaZAeJgtnXa7pjJQkE11mj8X/CTd304z7t5vG 3etg== X-Gm-Message-State: AOJu0YwAY4+eN1C4iuyUlQZ1kBur4r7VO7txevYOk6NOP7KH6QVreEfF FPO2vsWGVVw4F6a8SQOpGXuWy3mG4Q8l5P8KeGDM8diCnqiC7GDcH//dvvhyHCUGH/qEts1+nVJ 3Mx/ZgJGsdELdzyB3x8siyzgw5o2iIt4I/Y4MgnSCnuWFfS4kNznjnSQ9vNfIt+i4M1/vS7L3p8 88TKRjtKsU92YjRwhulzsxOCKuUIHHtxloVlWtdmuEkg== X-Gm-Gg: ASbGncvXmOJeMOGYb8zW/pkqyYHtsofPy7EnQ7qi34qyRkIG1hL3deNk9xC9OsuK36W /08xe3KZ/XpNK9ckrEdAkW4bXr54C1fM9GggzFEdVlzsCFNfD+IyXsAN3OWS4xAcYli/nV9ZUPU AmCBoqJ2T1RCoDbc+ZZUuGiDOB+/zp1+RqFfrUrU9xN4iIcSnT81bvxzgTPKocs+Qey1tBYAXmf BWVVpjtoaQBidWg34dsokNezzsTIxxBeWyIgjQO32iUGgH005Sfa0NtdtDzJwql0AL/d784pYae UYAs9+n8E5Zd X-Received: by 2002:a5d:47c3:0:b0:386:3329:6a04 with SMTP id ffacd0b85a97d-38a8732c076mr22658199f8f.39.1736820188825; Mon, 13 Jan 2025 18:03:08 -0800 (PST) X-Google-Smtp-Source: AGHT+IFiO915BZQLGPAJFC+Z9J+g3GGXj3W3Kwbezz5Qr8L4cazTeDfZpKcxGI2tB8jPBbY1v849ZA== X-Received: by 2002:a5d:47c3:0:b0:386:3329:6a04 with SMTP id ffacd0b85a97d-38a8732c076mr22658165f8f.39.1736820187867; Mon, 13 Jan 2025 18:03:07 -0800 (PST) Received: from digraph.polyomino.org.uk (digraph.polyomino.org.uk. [2001:8b0:bf73:93f7::51bb:e332]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-38a8e37e36asm13225180f8f.5.2025.01.13.18.03.06 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 13 Jan 2025 18:03:07 -0800 (PST) Received: from jsm28 (helo=localhost) by digraph.polyomino.org.uk with local-esmtp (Exim 4.97) (envelope-from ) id 1tXWGn-0000000CvkI-1EbI for libc-alpha@sourceware.org; Tue, 14 Jan 2025 02:03:05 +0000 Date: Tue, 14 Jan 2025 02:03:05 +0000 (UTC) From: Joseph Myers To: libc-alpha@sourceware.org Subject: [PATCH 2/6] Make fclose seek input file to right offset (bug 12724) In-Reply-To: Message-ID: <217b4e2c-4c71-678c-2827-41866c4012a8@redhat.com> References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: 70qcg_IySMJyb5bU3aTTMSA-r7qwtWbYIykn_D1liOY_1736820189 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-9.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, 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~patchwork=sourceware.org@sourceware.org As discussed in bug 12724 and required by POSIX, before an input file (based on an underlying seekable file descriptor) is closed, fclose is sometimes required to seek that file descriptor to the correct offset, so that any other file descriptors sharing the underlying open file description are left at that offset (as a motivating example, a script could call a sequence of commands each of which processes some data from (seekable) stdin using stdio; fclose needs to do this so that each successive command can read exactly the data not handled by previous commands), but glibc fails to do this. The precise POSIX wording has changed a few times; in the 2024 edition it's "If the file is not already at EOF, and the file is one capable of seeking, the file offset of the underlying open file description shall be set to the file position of the stream if the stream is the active handle to the underlying file description.". Add appropriate logic to _IO_new_file_close_it to handle this case. I haven't made any attempt to test or change things in this area for the "old" functions. Note that there was a previous attempt to fix bug 12724, reverted in commit eb6cbd249f4465b01f428057bf6ab61f5f0c07e3. The fix version here addresses the original test in that bug report without breaking the one given in a subsequent comment in that bug report (which works with glibc before the patch, but maybe was broken by the original fix that was reverted). The logic here tries to take care not to seek the file, even to its newly computed current offset, if at EOF / possibly not the active handle; even seeking to the current offset would be problematic because of a potential race (fclose computes the current offset, another thread or process with the active handle does its own seek, fclose does a seek (not permitted by POSIX in this case) that loses the effect of the seek on the active handle in another thread or process). There are tests included for various cases of being or not being the active handle, though there aren't tests for the potential race condition. Tested for x86_64. Reviewed-by: DJ Delorie --- libio/fileops.c | 43 +++++- stdio-common/Makefile | 1 + stdio-common/tst-fclose-offset.c | 225 +++++++++++++++++++++++++++++++ 3 files changed, 264 insertions(+), 5 deletions(-) create mode 100644 stdio-common/tst-fclose-offset.c diff --git a/libio/fileops.c b/libio/fileops.c index 27e1977eb6..358378431f 100644 --- a/libio/fileops.c +++ b/libio/fileops.c @@ -127,15 +127,48 @@ _IO_new_file_init (struct _IO_FILE_plus *fp) int _IO_new_file_close_it (FILE *fp) { - int write_status; + int flush_status = 0; if (!_IO_file_is_open (fp)) return EOF; if ((fp->_flags & _IO_NO_WRITES) == 0 && (fp->_flags & _IO_CURRENTLY_PUTTING) != 0) - write_status = _IO_do_flush (fp); - else - write_status = 0; + flush_status = _IO_do_flush (fp); + else if (fp->_fileno >= 0 + /* If this is the active handle, we must seek the + underlying open file description (possibly shared with + other file descriptors that remain open) to the correct + offset. But if this stream is in a state such that some + other handle might have become the active handle, then + (a) at the time it entered that state, the underlying + open file description had the correct offset, and (b) + seeking the underlying open file description, even to + its newly determined current offset, is not safe because + it can race with operations on a different active + handle. So check here for cases where it is necessary + to seek, while avoiding seeking in cases where it is + unsafe to do so. */ + && (_IO_in_backup (fp) + || (fp->_mode <= 0 && fp->_IO_read_ptr < fp->_IO_read_end) + || (_IO_vtable_offset (fp) == 0 + && fp->_mode > 0 && (fp->_wide_data->_IO_read_ptr + < fp->_wide_data->_IO_read_end)))) + { + off64_t o = _IO_SEEKOFF (fp, 0, _IO_seek_cur, 0); + if (o == EOF) + { + if (errno != ESPIPE) + flush_status = EOF; + } + else + { + if (_IO_in_backup (fp)) + o -= fp->_IO_save_end - fp->_IO_save_base; + flush_status = (_IO_SYSSEEK (fp, o, SEEK_SET) < 0 && errno != ESPIPE + ? EOF + : 0); + } + } _IO_unsave_markers (fp); @@ -160,7 +193,7 @@ _IO_new_file_close_it (FILE *fp) fp->_fileno = -1; fp->_offset = _IO_pos_BAD; - return close_status ? close_status : write_status; + return close_status ? close_status : flush_status; } libc_hidden_ver (_IO_new_file_close_it, _IO_file_close_it) diff --git a/stdio-common/Makefile b/stdio-common/Makefile index 589b786f45..b5f78c365b 100644 --- a/stdio-common/Makefile +++ b/stdio-common/Makefile @@ -234,6 +234,7 @@ tests := \ tst-bz11319-fortify2 \ tst-cookie \ tst-dprintf-length \ + tst-fclose-offset \ tst-fdopen \ tst-fdopen2 \ tst-ferror \ diff --git a/stdio-common/tst-fclose-offset.c b/stdio-common/tst-fclose-offset.c new file mode 100644 index 0000000000..a31de1117c --- /dev/null +++ b/stdio-common/tst-fclose-offset.c @@ -0,0 +1,225 @@ +/* Test offset of input file descriptor after close (bug 12724). + Copyright (C) 2025 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 + +int +do_test (void) +{ + char *filename = NULL; + int fd = create_temp_file ("tst-fclose-offset", &filename); + TEST_VERIFY_EXIT (fd != -1); + + /* Test offset of open file description for output and input streams + after fclose, case from bug 12724. */ + + const char buf[] = "hello world"; + xwrite (fd, buf, sizeof buf); + TEST_COMPARE (lseek (fd, 1, SEEK_SET), 1); + int fd2 = xdup (fd); + FILE *f = fdopen (fd2, "w"); + TEST_VERIFY_EXIT (f != NULL); + TEST_COMPARE (fputc (buf[1], f), buf[1]); + xfclose (f); + errno = 0; + TEST_COMPARE (lseek (fd2, 0, SEEK_CUR), -1); + TEST_COMPARE (errno, EBADF); + TEST_COMPARE (lseek (fd, 0, SEEK_CUR), 2); + + /* Likewise for an input stream. */ + fd2 = xdup (fd); + f = fdopen (fd2, "r"); + TEST_VERIFY_EXIT (f != NULL); + TEST_COMPARE (fgetc (f), buf[2]); + xfclose (f); + errno = 0; + TEST_COMPARE (lseek (fd2, 0, SEEK_CUR), -1); + TEST_COMPARE (errno, EBADF); + TEST_COMPARE (lseek (fd, 0, SEEK_CUR), 3); + + /* Test offset of open file description for output and input streams + after fclose, case from comment on bug 12724 (failed after first + attempt at fixing that bug). This verifies that the offset is + not reset when there has been no input or output on the FILE* (in + that case, the FILE* might not be the active handle). */ + + TEST_COMPARE (lseek (fd, 0, SEEK_SET), 0); + xwrite (fd, buf, sizeof buf); + TEST_COMPARE (lseek (fd, 1, SEEK_SET), 1); + fd2 = xdup (fd); + f = fdopen (fd2, "w"); + TEST_VERIFY_EXIT (f != NULL); + TEST_COMPARE (lseek (fd, 4, SEEK_SET), 4); + xfclose (f); + errno = 0; + TEST_COMPARE (lseek (fd2, 0, SEEK_CUR), -1); + TEST_COMPARE (errno, EBADF); + TEST_COMPARE (lseek (fd, 0, SEEK_CUR), 4); + + /* Likewise for an input stream. */ + fd2 = xdup (fd); + f = fdopen (fd2, "r"); + TEST_VERIFY_EXIT (f != NULL); + TEST_COMPARE (lseek (fd, 4, SEEK_SET), 4); + xfclose (f); + errno = 0; + TEST_COMPARE (lseek (fd2, 0, SEEK_CUR), -1); + TEST_COMPARE (errno, EBADF); + TEST_COMPARE (lseek (fd, 0, SEEK_CUR), 4); + + /* Further cases without specific tests in bug 12724, to verify + proper operation of the rules about the offset only being set + when the stream is the active handle. */ + + /* Test offset set by fclose after fseek and fgetc. */ + TEST_COMPARE (lseek (fd, 0, SEEK_SET), 0); + fd2 = xdup (fd); + f = fdopen (fd2, "r"); + TEST_VERIFY_EXIT (f != NULL); + TEST_COMPARE (fseek (f, 1, SEEK_SET), 0); + TEST_COMPARE (fgetc (f), buf[1]); + xfclose (f); + errno = 0; + TEST_COMPARE (lseek (fd2, 0, SEEK_CUR), -1); + TEST_COMPARE (errno, EBADF); + TEST_COMPARE (lseek (fd, 0, SEEK_CUR), 2); + + /* Test offset not set by fclose after fseek and fgetc, if that + fgetc is at EOF (in which case the active handle might have + changed). */ + TEST_COMPARE (lseek (fd, 0, SEEK_SET), 0); + fd2 = xdup (fd); + f = fdopen (fd2, "r"); + TEST_VERIFY_EXIT (f != NULL); + TEST_COMPARE (fseek (f, sizeof buf, SEEK_SET), 0); + TEST_COMPARE (fgetc (f), EOF); + TEST_COMPARE (lseek (fd, 4, SEEK_SET), 4); + xfclose (f); + errno = 0; + TEST_COMPARE (lseek (fd2, 0, SEEK_CUR), -1); + TEST_COMPARE (errno, EBADF); + TEST_COMPARE (lseek (fd, 0, SEEK_CUR), 4); + + /* Test offset not set by fclose after fseek and fgetc and fflush + (active handle might have changed after fflush). */ + TEST_COMPARE (lseek (fd, 0, SEEK_SET), 0); + fd2 = xdup (fd); + f = fdopen (fd2, "r"); + TEST_VERIFY_EXIT (f != NULL); + TEST_COMPARE (fseek (f, 1, SEEK_SET), 0); + TEST_COMPARE (fgetc (f), buf[1]); + TEST_COMPARE (fflush (f), 0); + TEST_COMPARE (lseek (fd, 4, SEEK_SET), 4); + xfclose (f); + errno = 0; + TEST_COMPARE (lseek (fd2, 0, SEEK_CUR), -1); + TEST_COMPARE (errno, EBADF); + TEST_COMPARE (lseek (fd, 0, SEEK_CUR), 4); + + /* Test offset not set by fclose after fseek and fgetc, if the + stream is unbuffered (active handle might change at any + time). */ + TEST_COMPARE (lseek (fd, 0, SEEK_SET), 0); + fd2 = xdup (fd); + f = fdopen (fd2, "r"); + TEST_VERIFY_EXIT (f != NULL); + setbuf (f, NULL); + TEST_COMPARE (fseek (f, 1, SEEK_SET), 0); + TEST_COMPARE (fgetc (f), buf[1]); + TEST_COMPARE (lseek (fd, 4, SEEK_SET), 4); + xfclose (f); + errno = 0; + TEST_COMPARE (lseek (fd2, 0, SEEK_CUR), -1); + TEST_COMPARE (errno, EBADF); + TEST_COMPARE (lseek (fd, 0, SEEK_CUR), 4); + + /* Also test such cases with the stream in wide mode. */ + + /* Test offset set by fclose after fseek and fgetwc. */ + TEST_COMPARE (lseek (fd, 0, SEEK_SET), 0); + fd2 = xdup (fd); + f = fdopen (fd2, "r"); + TEST_VERIFY_EXIT (f != NULL); + TEST_COMPARE (fseek (f, 1, SEEK_SET), 0); + TEST_COMPARE (fgetwc (f), (wint_t) buf[1]); + xfclose (f); + errno = 0; + TEST_COMPARE (lseek (fd2, 0, SEEK_CUR), -1); + TEST_COMPARE (errno, EBADF); + TEST_COMPARE (lseek (fd, 0, SEEK_CUR), 2); + + /* Test offset not set by fclose after fseek and fgetwc, if that + fgetwc is at EOF (in which case the active handle might have + changed). */ + TEST_COMPARE (lseek (fd, 0, SEEK_SET), 0); + fd2 = xdup (fd); + f = fdopen (fd2, "r"); + TEST_VERIFY_EXIT (f != NULL); + TEST_COMPARE (fseek (f, sizeof buf, SEEK_SET), 0); + TEST_COMPARE (fgetwc (f), WEOF); + TEST_COMPARE (lseek (fd, 4, SEEK_SET), 4); + xfclose (f); + errno = 0; + TEST_COMPARE (lseek (fd2, 0, SEEK_CUR), -1); + TEST_COMPARE (errno, EBADF); + TEST_COMPARE (lseek (fd, 0, SEEK_CUR), 4); + + /* Test offset not set by fclose after fseek and fgetwc and fflush + (active handle might have changed after fflush). */ + TEST_COMPARE (lseek (fd, 0, SEEK_SET), 0); + fd2 = xdup (fd); + f = fdopen (fd2, "r"); + TEST_VERIFY_EXIT (f != NULL); + TEST_COMPARE (fseek (f, 1, SEEK_SET), 0); + TEST_COMPARE (fgetwc (f), (wint_t) buf[1]); + TEST_COMPARE (fflush (f), 0); + TEST_COMPARE (lseek (fd, 4, SEEK_SET), 4); + xfclose (f); + errno = 0; + TEST_COMPARE (lseek (fd2, 0, SEEK_CUR), -1); + TEST_COMPARE (errno, EBADF); + TEST_COMPARE (lseek (fd, 0, SEEK_CUR), 4); + + /* Test offset not set by fclose after fseek and fgetwc, if the + stream is unbuffered (active handle might change at any + time). */ + TEST_COMPARE (lseek (fd, 0, SEEK_SET), 0); + fd2 = xdup (fd); + f = fdopen (fd2, "r"); + TEST_VERIFY_EXIT (f != NULL); + setbuf (f, NULL); + TEST_COMPARE (fseek (f, 1, SEEK_SET), 0); + TEST_COMPARE (fgetwc (f), (wint_t) buf[1]); + TEST_COMPARE (lseek (fd, 4, SEEK_SET), 4); + xfclose (f); + errno = 0; + TEST_COMPARE (lseek (fd2, 0, SEEK_CUR), -1); + TEST_COMPARE (errno, EBADF); + TEST_COMPARE (lseek (fd, 0, SEEK_CUR), 4); + + return 0; +} + +#include