Bug 12724

Summary: fclose violates POSIX 2008 on seekable input streams
Product: glibc Reporter: Eric Blake <eblake>
Component: stdioAssignee: Not yet assigned to anyone <unassigned>
Status: UNCONFIRMED ---    
Severity: normal CC: carlos, hannes, matz, toolchain
Priority: P1 Flags: fweimer: security-
Version: 2.13   
Target Milestone: ---   
Host: Target:
Build: Last reconfirmed:

Description Eric Blake 2011-05-02 20:20:41 UTC
POSIX 2008 states:

"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 adjusted so that the next operation on the open file description deals with the byte after the last one read from or written to the stream being closed."

[XSH fclose, http://pubs.opengroup.org/onlinepubs/9699919799/functions/fclose.html]

However, this sample program proves that for seekable input streams, glibc is violating this requirement.  The same program runs to completion on Solaris 10.


#include <stdio.h>
#include <errno.h>
#include <fcntl.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <assert.h>

#define NAME "test-fclose.t"

int
main (void)
{
  const char buf[] = "hello world";
  int fd;
  int fd2;
  FILE *f;

  /* Prepare a seekable file.  */
  fd = open (NAME, O_RDWR | O_CREAT | O_TRUNC, 0600);
  assert (0 <= fd);
  assert (write (fd, buf, sizeof buf) == sizeof buf);
  assert (lseek (fd, 1, SEEK_SET) == 1);

  /* Create an output stream visiting the file; when it is closed, all
     other file descriptors visiting the file must see the new file
     position.  */
  fd2 = dup (fd);
  assert (0 <= fd2);
  f = fdopen (fd2, "w");
  assert (f);
  assert (fputc (buf[1], f) == buf[1]);
  assert (fclose (f) == 0);
  errno = 0;
  assert (lseek (fd2, 0, SEEK_CUR) == -1);
  assert (errno == EBADF);
  assert (lseek (fd, 0, SEEK_CUR) == 2);

  /* Likewise for an input stream.  */
  fd2 = dup (fd);
  assert (0 <= fd2);
  f = fdopen (fd2, "r");
  assert (f);
  assert (fgetc (f) == buf[2]);
  assert (fclose (f) == 0);
  errno = 0;
  assert (lseek (fd2, 0, SEEK_CUR) == -1);
  assert (errno == EBADF);
  assert (lseek (fd, 0, SEEK_CUR) == 3);

  /* Clean up.  */
  assert (remove (NAME) == 0);

  return 0;
}
Comment 1 Ulrich Drepper 2011-05-14 01:11:00 UTC
Fixed in git.
Comment 2 Eric Blake 2011-07-27 17:34:22 UTC
glibc 2.14 still has issues.  Per the latest wording:

http://austingroupbugs.net/view.php?id=87#c838

At page 805 line 26801 section fclose, change:

     the file offset of the underlying open file description shall be
     adjusted so that the next operation on the open file description
     deals with the byte after the last one read from or written to the
     stream being closed.

to:

     the file offset of the underlying open file description shall be
     set to the file position of the stream.

fclose() _must_ reposition the underlying fd, even if there was no last byte read.  Therefore, this program, which passes on Solaris, shows that glibc is still buggy:

#include <stdio.h>
#include <errno.h>
#include <fcntl.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <assert.h>

#define NAME "test-fclose.t"

int
main (void)
{
  const char buf[] = "hello world";
  int fd;
  int fd2;
  FILE *f;

  /* Prepare a seekable file.  */
  fd = open (NAME, O_RDWR | O_CREAT | O_TRUNC, 0600);
  assert (0 <= fd);
  assert (write (fd, buf, sizeof buf) == sizeof buf);
  assert (lseek (fd, 1, SEEK_SET) == 1);

  /* Create an output stream visiting the file; when it is closed, all
     other file descriptors visiting the file must see the new file
     position.  */
  fd2 = dup (fd);
  assert (0 <= fd2);
  f = fdopen (fd2, "w");
  assert (f);
  assert(lseek(fd, 4, SEEK_SET) == 4);
  assert (fclose (f) == 0);
  errno = 0;
  assert (lseek (fd2, 0, SEEK_CUR) == -1);
  assert (errno == EBADF);
  assert (lseek (fd, 0, SEEK_CUR) == 4);

  /* Likewise for an input stream.  */
  fd2 = dup (fd);
  assert (0 <= fd2);
  f = fdopen (fd2, "r");
  assert (f);
  assert(lseek(fd, 4, SEEK_SET) == 4);
  assert (fclose (f) == 0);
  errno = 0;
  assert (lseek (fd2, 0, SEEK_CUR) == -1);
  assert (errno == EBADF);
  assert (lseek (fd, 0, SEEK_CUR) == 4);

  /* Clean up.  */
  assert (remove (NAME) == 0);

  return 0;
}
Comment 3 Michael Matz 2011-08-12 15:31:21 UTC
Over at SuSE we ran into issues with this change.  See
  https://bugzilla.novell.com/show_bug.cgi?id=711829
.  Ruby does a dup/fdopen, but then doesn't do any reading/writing with 
the new stream, and when it gets fclosed the file position is reset also
for the other stream associated with the same file.  That one does
reads/writes.

I believe that's exactly the interaction between active/inactive streams
that http://austingroupbugs.net/view.php?id=87#c895 is talking about.

Now, that may or may not be useless code in ruby, but there we are.
The current half-POSIX-compliant way is worse than either the non-compliant
or the compliant way.
Comment 4 Carlos O'Donell 2012-11-29 15:19:04 UTC
We need someone to look into this on master and see if they can come up with a patch to fix this.
Comment 5 Carlos O'Donell 2012-12-01 19:56:12 UTC
We are reverting this patch for 2.17 and will be fixing this correctly for 2.18 and ensuring that existing applications are not broken.
Comment 6 Eric Blake 2014-01-17 22:06:33 UTC
Yet another example of how glibc violates POSIX:
http://austingroupbugs.net/view.php?id=816

and now cygwin has chosen to copy glibc's buggy behavior:
http://cygwin.com/ml/cygwin/2014-01/threads.html#00110

Any word on when this will be fixed?
Comment 7 Eric Blake 2014-01-17 22:10:30 UTC
Intuitively, glibc is broken as long as:

$ seq 3 > file
$ { sed -n 1q; sed -n 1q; cat; } < file

has incorrect output.  On Solaris, where fclose (and fflush(NULL) and exit()) correctly sync the seekable position of the FILE* back to the underyling fd, the above test has the POSIX-mandated behavior of acting like 'tail -n+3 file'.  [It is possible to use gnulib's 'close-stdin' module to make 'sed' work around glibc's bug, and in fact, coreutils already does so for all of its programs which use stdin without reading to EOF; but we shouldn't have to work around the bug in every single program that reads partial input from stdin when we can instead fix it completely by fixing glibc]
Comment 8 joseph@codesourcery.com 2014-01-17 23:11:30 UTC
On Fri, 17 Jan 2014, eblake at redhat dot com wrote:

> Any word on when this will be fixed?

When I started going through open "math" bugs a couple of years ago, 
fixing them and identifying underlying issues (with the implementation, 
test coverage, etc.) and other bugs in the course of fixing them, I was 
hoping for other people similarly to pick up the roles of glibc experts in 
particular areas, go through open bugs, fix them, file bugs for issues 
found, improve test coverage, understand the strengths and weaknesses of 
glibc in those areas and use that understanding to seek out and fix 
classes of bugs that seem likely to be present (without waiting for users 
to find and report them).

I still hope for people to pick up such roles.  One area needing an expert 
to take on leadership in resolving known issues is stdio.  Another is 
POSIX conformance, including going through clarifications / changes 
resulting from issues reported against various POSIX versions to ensure 
they are properly reflected in glibc, and bugs filed if not (and then, in 
future, to ensure bugs get filed for all future POSIX clarifications / 
changes indicating that a glibc change is needed).  Related to that is 
tracking new feature additions in POSIX (both between past revisions and 
in future ones) and ensuring we understand what features are missing from 
glibc.  Each component in Bugzilla could do with at least one expert.  
"libc" - miscellaneous bugs - needs a generalist who will, inter alia, 
seek consensus on questions of whether bugs that are really feature 
requests reflect features we do or do not want to have.

So: I'd expect this bug to be fixed after someone takes the lead on fixing 
it, possibly as part of taking the lead more generally on sorting out 
stdio or POSIX conformance issues, and including analysis of the 
compatibility issues and getting consensus on what might need doing for 
compatibility with existing binaries.