Bug 17063 - fclose() may fail to flush data
Summary: fclose() may fail to flush data
Status: RESOLVED INVALID
Alias: None
Product: glibc
Classification: Unclassified
Component: stdio (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
: 18659 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-06-17 17:35 UTC by Paul Pluzhnikov
Modified: 2015-07-11 08:25 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Pluzhnikov 2014-06-17 17:35:39 UTC
This is a followup to PR16532.

A program that does

  fopen(..., "w+");
  fwrite(...);
  fread(...);
  fclose();

may leave empty file on disk (fail to flush when it should have), when the fread requests more than a page's worth of data.

At least glibc-2.19 through current trunk (754c5a08aacb44895d1ab97c553ce424eb43f761) are affected.

./t
testing     3
testing     3 OK
testing     6
testing     6 OK
testing     9
testing     9 OK
testing  4095
testing  4095 OK
testing  4096
t: t.c:28: do_test: Assertion `pos == 6' failed.
Aborted

/// --- cut ---
#include <assert.h>
#include <stdio.h>

void do_test (int n)
{
  FILE *fp;
  const char *const fname = "/tmp/output";
  int nwritten, nread, pos;
  char line[8192];

  printf ("testing %5d\n", n);
  fp = fopen (fname, "w+");
  nwritten = fwrite ("abcabc", 1, 6, fp);
  assert (nwritten == 6);

  pos = ftello (fp);
  assert (pos == 6);

  nread = fread (line, 1, n, fp);
  assert (nread == 0);

  fclose (fp);

  fp = fopen (fname, "r");
  fseeko (fp, 0, SEEK_END);
  pos = ftello (fp);

  assert (pos == 6);
  printf ("testing %5d OK\n", n);
}

int main(int argc, char *argv[])
{
  int j, nreads[] = { 3, 6, 9, 4095, 4096, 4097, 8191, 8192 };

  for (j = 0; j < sizeof (nreads) / sizeof (nreads[0]); ++j)
    do_test (nreads[j]);

  return 0;
}
Comment 1 Paul Pluzhnikov 2014-06-17 18:50:17 UTC
Google ref: b/15017950
Comment 2 Paul Pluzhnikov 2014-06-23 14:55:53 UTC
From: Siddhesh Poyarekar <siddhesh@redhat.com>
---
That's not a bug because you're invoking a behaviour explicitly
forbidden by the standard:

    When a file is opened with update mode ( '+' as the second or
    third character in the mode argument), both input and output may
    be performed on the associated stream. However, the application
    shall ensure that output is not directly followed by input without
    an intervening call to fflush() or to a file positioning function
    ( fseek(), fsetpos(), or rewind()), and input is not directly
    followed by output without an intervening call to a file
    positioning function, unless the input operation encounters
    end-of-file.

We forbid it in the manual too:

    As you can see, ‘+’ requests a stream that can do both input and
    output. When using such a stream, you must call fflush (see Stream
    Buffering) or a file positioning function such as fseek (see File
    Positioning) when switching from reading to writing or vice
    versa. Otherwise, internal buffers might not be emptied properly.

If you add an fseek call before ftello, you'll see the correct
behaviour because fseek acts as the necessary barrier between fread
and fwrite.  ftell is not good enough for that because it cannot be
relied on to change fp state.
---

Thanks for analysis.

This is quite end-user unfriendly behavior:
- it's new ("broke" somewhere between glibc-2.15 and 2.19),
- it's "broken" only for some, but not all, calls to output followed by input without intervening flush.
Comment 3 Siddhesh Poyarekar 2014-06-23 15:15:16 UTC
(In reply to Paul Pluzhnikov from comment #2)
> This is quite end-user unfriendly behavior:
> - it's new ("broke" somewhere between glibc-2.15 and 2.19),
> - it's "broken" only for some, but not all, calls to output followed by
> input without intervening flush.

Yes, it broke when I wrote a new implementation of ftell.  The earlier implementation was essentially an alias for fseek (fp, 0, SEEK_CUR), which is why it worked as an intermediate function to switch between reads and writes.  I didn't bother versioning the change because we explicitly forbid the behaviour this bug relies on.
Comment 4 Andreas Schwab 2015-07-11 08:25:13 UTC
*** Bug 18659 has been marked as a duplicate of this bug. ***