Bug 12799

Summary:	fflush violates POSIX on seekable input streams
Product:	glibc	Reporter:	Eric Blake <eblake>
Component:	stdio	Assignee:	Not yet assigned to anyone <unassigned>
Status:	NEW ---
Severity:	normal	CC:	chimpatheist, dancol, fweimer, hannes, rjones, siddhesh
Priority:	P2	Flags:	fweimer: security-
Version:	2.13
Target Milestone:	---
Host:		Target:
Build:		Last reconfirmed:
Bug Depends on:
Bug Blocks:	12724

Description Eric Blake 2011-05-23 17:14:06 UTC

Per http://austingroupbugs.net/view.php?id=87, fflush is required to discard any ungetc() bytes and set the underlying file position to the point as though the pushed-back bytes had never been read.  That is, in the sequence:

{ app1; app2; } < seekable

if app1 reads one byte too many, then does ungetc() to push it back, then app2 should start reading at the byte that app1 did not want, rather than at the point that app1 reached before using ungetc().  More concretely, this program should exit with status 0, but right now it is exiting with status 7 (glibc is reading 'w' instead of ' ' after the fflush):


#define _POSIX_C_SOURCE 200112L
#include <stdio.h>
int main (void)
{
  FILE *f;
  char buffer[10];
  int fd;
  int c;

  f = fopen ("file", "w+");
  if (f == NULL)
    return 1;
  if (fputs ("hello world", f) == EOF)
    return 2;
  rewind (f);

  fd = fileno (f);
  if (fd < 0 || fread (buffer, 1, 5, f) != 5)
    return 3;
  c = fgetc (f);
  if (c != ' ')
    return 4;
  if (ungetc ('@', f) != '@')
    return 5;
  if (fflush (f) == EOF)
    return 6;
  if (fgetc (f) != c)
    return 7;
  return 0;
}

This program returns 0 on Solaris and Cygwin.

Comment 1 Eric Blake 2018-03-05 19:20:41 UTC

The removal of libio.h is making this bug all the more important to fix, so that gnulib can quit mucking around in glibc internals:
https://lists.gnu.org/archive/html/bug-gnulib/2018-03/msg00002.html

Is there a chance that it can be fixed for 2.28?

Comment 2 Eric Blake 2020-04-14 00:24:27 UTC

(In reply to Eric Blake from comment #1)
> The removal of libio.h is making this bug all the more important to fix, so
> that gnulib can quit mucking around in glibc internals:
> https://lists.gnu.org/archive/html/bug-gnulib/2018-03/msg00002.html
> 
> Is there a chance that it can be fixed for 2.28?

Still broken in 2020.  Similarly, fflush(NULL) should have this effect on all seekable input streams.

Comment 3 chimpatheist 2020-08-02 23:05:45 UTC

Maybe I'm wrong, but seems that this bug goes out if fflush() make a call to _IO_switch_to_main_get_area() just before the call to _IO_new_file_sync().

Comment 4 chimpatheist 2020-08-03 18:34:29 UTC

(In reply to chimpatheist from comment #3)
> Maybe I'm wrong, but seems that this bug goes out if fflush() make a call to
> _IO_switch_to_main_get_area() just before the call to _IO_new_file_sync().

Never mind.

Actually, the bug occurs when fflush() is called and _IO_IN_BACKUP is on, but just returning to main_get_area doesn't work.

Before switching, we need calculate the difference between fp->_IO_read_end and fp->_IO_read_ptr and adjust fp->_IO_read_ptr after the switch.

something like this :

fflush(FILE *fp)
{
     :
     :    
  if(fp->_flags & _IO_IN_BACKUP)
  {
     long delta = fp->_IO_read_end - fp->_IO_read_ptr;
     _IO_switch_to_main_get_area(fp);
     fp->_IO_read_base -= delta;
     fp->_IO_read_ptr = fp->_IO_read_base;
  }
     :
     :
}

Comment 5 chimpatheist 2020-08-03 20:03:46 UTC

More exactly...

    if(fp->_flags & _IO_IN_BACKUP)
    {
        // how many ungetc() ?
        long delta1 = fp->_IO_read_end - fp->_IO_read_ptr;

        // how many fgetc() before going to backup area ?
        long delta2 = fp->_IO_save_base - fp->_IO_buf_base;

        // choose the better of the two values
        if(delta1 < delta2) delta2 = delta1;

        // discard all pushed chars
        fp->_IO_read_ptr = fp->_IO_read_end;

        // and reset save_base
        fp->_IO_save_base -= delta2;

        _IO_switch_to_main_get_area(fp);
    }