[Bug libc/2074] New: _IO_new_file_xsputn() in fileops.c not checking for EOF

[Bug libc/2074] New: _IO_new_file_xsputn() in fileops.c not checking for EOF

[jfardo at laurelnetworks dot com]

In glibc 2.3.4, the function new_do_write() in libio/fileops.c returns an 
unsigned
value (an _IO_size_t). If the underlying write system call in new_do_write()
returns EOF (-1), new_do_write() will return 0xffffffff rather than -1.

The function IO_new_file_xsputn() in libio/fileops.c appears to have
2 problems if the call to new_do_write() returns 0xffffffff. Here's
the specific snippet of code from IO_new_file_xsputn():
      if (do_write)
        {
          count = new_do_write (f, s, do_write);
          to_do -= count;
          if (count < do_write)
            return n - to_do;
        }
      /* Now write out the remainder.  Normally, this will fit in the
	 buffer, but it's somewhat messier for line-buffered files,
	 so we let _IO_default_xsputn handle the general case. */
      if (to_do)
	to_do -= INTUSE(_IO_default_xsputn) (f, s+do_write, to_do);
    }
  return n - to_do;


If new_do_write() returns 0xffffffff, 'to_do' will actually
be incremented by 1. Also, since 'count' is an unsigned quantity,
the if statement 'if (count < do_write)' will evaluate to false,
and the code will incorrectly fall through to the call to
IO_default_xsputn().

-- 
           Summary: _IO_new_file_xsputn() in fileops.c not checking for EOF
           Product: glibc
           Version: 2.3.4
            Status: NEW
          Severity: normal
          Priority: P2
         Component: libc
        AssignedTo: drepper at redhat dot com
        ReportedBy: jfardo at laurelnetworks dot com
                CC: glibc-bugs at sources dot redhat dot com
 GCC build triplet: i686-pc-linux-gnu
  GCC host triplet: i686-pc-linux-gnu
GCC target triplet: i686-pc-linux-gnu


http://sourceware.org/bugzilla/show_bug.cgi?id=2074

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/2074] _IO_new_file_xsputn() in fileops.c not checking for EOF

[drepper at redhat dot com]

------- Additional Comments From drepper at redhat dot com  2006-04-25 18:42 -------
Provide an example of a user of libc (not the internal functions) when this is a
problem.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |WAITING


http://sourceware.org/bugzilla/show_bug.cgi?id=2074

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/2074] _IO_new_file_xsputn() in fileops.c not checking for EOF

[jfardo at laurelnetworks dot com]

------- Additional Comments From jfardo at laurelnetworks dot com  2006-04-25 21:40 -------
I don't have a specific test case any more. This would intermittently cause
problems when running 'rpm' to install a package on a file system 
that was full. If the package being installed was just the right 
size, rpm would seg fault.

The following change in fileops.c fixes the bug. 

glibc/libio/fileops.c:

***************
*** 1371,1376 ****
--- 1371,1379 ----
        if (do_write)
          {
            count = new_do_write (f, s, do_write);
+           if (count == EOF) {
+               count = 0;
+           }
            to_do -= count;
            if (count < do_write)
              return n - to_do;


-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=2074

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/2074] _IO_new_file_xsputn() in fileops.c not checking for EOF

[drepper at redhat dot com]

------- Additional Comments From drepper at redhat dot com  2006-08-03 18:25 -------
In which path does new_do_write return EOF?  It shouldn't.  Therefore the patch
isn't needed.  I'll close the bug unless you can show some data.

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=2074

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/2074] _IO_new_file_xsputn() in fileops.c not checking for EOF

[jfardo at laurelnetworks dot com]

------- Additional Comments From jfardo at laurelnetworks dot com  2006-08-03 18:54 -------
(In reply to comment #0)
> In glibc 2.3.4, the function new_do_write() in libio/fileops.c returns an 
> unsigned
> value (an _IO_size_t). If the underlying write system call in new_do_write()
> returns EOF (-1), new_do_write() will return 0xffffffff rather than -1.
> The function IO_new_file_xsputn() in libio/fileops.c appears to have
> 2 problems if the call to new_do_write() returns 0xffffffff. Here's
> the specific snippet of code from IO_new_file_xsputn():
>       if (do_write)
>         {
>           count = new_do_write (f, s, do_write);
>           to_do -= count;
>           if (count < do_write)
>             return n - to_do;
>         }
>       /* Now write out the remainder.  Normally, this will fit in the
> 	 buffer, but it's somewhat messier for line-buffered files,
> 	 so we let _IO_default_xsputn handle the general case. */
>       if (to_do)
> 	to_do -= INTUSE(_IO_default_xsputn) (f, s+do_write, to_do);
>     }
>   return n - to_do;
> If new_do_write() returns 0xffffffff, 'to_do' will actually
> be incremented by 1. Also, since 'count' is an unsigned quantity,
> the if statement 'if (count < do_write)' will evaluate to false,
> and the code will incorrectly fall through to the call to
> IO_default_xsputn().

(In reply to comment #3)

_IO_new_file_xsputn() calls new_do_write() which in turn calls _IO_SYSWRITE(). 
The call to _IO_SYSWRITE() may return -1 (EOF). When this happens, new_do_write
() returns this value as the 'count'.

I've highlighted this in the following piece of code in fileops.c.

static
int
new_do_write (fp, data, to_do)
     _IO_FILE *fp;
     const char *data;
     _IO_size_t to_do;
{
  _IO_size_t count;
  if (fp->_flags & _IO_IS_APPENDING)
    /* On a system without a proper O_APPEND implementation,
       you would need to sys_seek(0, SEEK_END) here, but is
       is not needed nor desirable for Unix- or Posix-like systems.
       Instead, just indicate that offset (before and after) is
       unpredictable. */
    fp->_offset = _IO_pos_BAD;
  else if (fp->_IO_read_end != fp->_IO_write_base)
    {
      _IO_off64_t new_pos
	= _IO_SYSSEEK (fp, fp->_IO_write_base - fp->_IO_read_end, 1);
      if (new_pos == _IO_pos_BAD)
	return 0;
      fp->_offset = new_pos;
    }

********** THIS CALL MAY RETURN -1 ***************
  count = _IO_SYSWRITE (fp, data, to_do);
*************************************************

  if (fp->_cur_column && count)
    fp->_cur_column = INTUSE(_IO_adjust_column) (fp->_cur_column - 1, data,
						 count) + 1;
  _IO_setg (fp, fp->_IO_buf_base, fp->_IO_buf_base, fp->_IO_buf_base);
  fp->_IO_write_base = fp->_IO_write_ptr = fp->_IO_buf_base;
  fp->_IO_write_end = (fp->_mode <= 0
		       && (fp->_flags & (_IO_LINE_BUF+_IO_UNBUFFERED))
		       ? fp->_IO_buf_base : fp->_IO_buf_end);
  return count;
}



-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=2074

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/2074] _IO_new_file_xsputn() in fileops.c not checking for EOF

[drepper at redhat dot com]

------- Additional Comments From drepper at redhat dot com  2006-08-28 15:24 -------
Which function called though _IO_SYSWRITE returns -1?  None should, that's the
whole point.

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=2074

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/2074] _IO_new_file_xsputn() in fileops.c not checking for EOF

[John dot Fardo at ecitele dot com]

------- Additional Comments From John dot Fardo at ecitele dot com  2006-08-29 15:03 -------
Subject: RE:  _IO_new_file_xsputn() in fileops.c not checking for EOF

It's not very clear from the glibc code, but, isn't _IO_SYSWRITE
a wrapper for the system call to write?

Are you implying that the write system call should never fail? 
What do you think will happen when the file system is full?

- John Fardo

> -----Original Message-----
> From: drepper at redhat dot com 
> [mailto:sourceware-bugzilla@sourceware.org] 
> Sent: Monday, August 28, 2006 11:25 AM
> To: John Fardo
> Subject: [Bug libc/2074] _IO_new_file_xsputn() in fileops.c 
> not checking for EOF
> 
> 
> ------- Additional Comments From drepper at redhat dot com  
> 2006-08-28 15:24 ------- Which function called though 
> _IO_SYSWRITE returns -1?  None should, that's the whole point.
> 
> -- 
> 
> 
> http://sourceware.org/bugzilla/show_bug.cgi?id=2074
> 
> ------- You are receiving this mail because: ------- You 
> reported the bug, or are watching the reporter.
> 
> 


-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=2074

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/2074] _IO_new_file_xsputn() in fileops.c not checking for EOF

[jakub at redhat dot com]

------- Additional Comments From jakub at redhat dot com  2006-08-29 15:34 -------
So you are just guessing what is it?  Can't you instead grep the code?
find libc -name \*.[ch] | xargs grep 'JUMP_INIT[[:blank:]]*(write'
shows the functions and none of them return EOF.

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=2074

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/2074] _IO_new_file_xsputn() in fileops.c not checking for EOF

[John dot Fardo at ecitele dot com]

------- Additional Comments From John dot Fardo at ecitele dot com  2006-08-29 17:43 -------
Subject: RE:  _IO_new_file_xsputn() in fileops.c not checking for EOF

_IO_cookie_write() in iofopncook.c looks like it can return -1.

Also, this problem was observed when running rpm -
rpm uses fopencookie().

Code from iofopncook.c:
static _IO_ssize_t
_IO_cookie_write (fp, buf, size)
     _IO_FILE *fp;
     const void *buf;
     _IO_ssize_t size;
{
  struct _IO_cookie_file *cfile = (struct _IO_cookie_file *) fp;

  if (cfile->__io_functions.write == NULL)
    return -1;

  return cfile->__io_functions.write (cfile->__cookie, buf, size);
}

- John Fardo 

> -----Original Message-----
> From: jakub at redhat dot com 
> [mailto:sourceware-bugzilla@sourceware.org] 
> Sent: Tuesday, August 29, 2006 11:34 AM
> To: John Fardo
> Subject: [Bug libc/2074] _IO_new_file_xsputn() in fileops.c 
> not checking for EOF
> 
> 
> ------- Additional Comments From jakub at redhat dot com  
> 2006-08-29 15:34 ------- So you are just guessing what is it? 
>  Can't you instead grep the code?
> find libc -name \*.[ch] | xargs grep 'JUMP_INIT[[:blank:]]*(write'
> shows the functions and none of them return EOF.
> 
> -- 
> 
> 
> http://sourceware.org/bugzilla/show_bug.cgi?id=2074
> 
> ------- You are receiving this mail because: ------- You 
> reported the bug, or are watching the reporter.
> 
> 


-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=2074

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug libc/2074] _IO_new_file_xsputn() in fileops.c not checking for EOF

[drepper at redhat dot com]

------- Additional Comments From drepper at redhat dot com  2006-09-09 17:33 -------
> _IO_cookie_write() in iofopncook.c looks like it can return -1.

It returns what the user provided function returns.  If the latter returns -1 it
is buggy and must be fixed.  This is not glibc's problem.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |RESOLVED
         Resolution|                            |INVALID


http://sourceware.org/bugzilla/show_bug.cgi?id=2074

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug manual/2074] _IO_new_file_xsputn() in fileops.c not checking for EOF

[pasky at suse dot cz]

------- Additional Comments From pasky at suse dot cz  2010-05-10 23:46 -------
Then this is a documentation bug - 12.21.3.2 Custom Stream Hook Functions says:

   You should define the function to write data to the cookie as:

     ssize_t WRITER (void *COOKIE, const char *BUFFER, size_t SIZE)

   This is very similar to the `write' function; see *note I/O
Primitives::.  Your function should transfer up to SIZE bytes from the
buffer, and return the number of bytes written.  You can return a value
of `-1' to indicate an error.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
          Component|libc                        |manual
         Resolution|INVALID                     |


http://sourceware.org/bugzilla/show_bug.cgi?id=2074

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug manual/2074] _IO_new_file_xsputn() in fileops.c not checking for EOF

[pasky at suse dot cz]

------- Additional Comments From pasky at suse dot cz  2010-05-11 00:51 -------
Created an attachment (id=4777)
 --> (http://sourceware.org/bugzilla/attachment.cgi?id=4777&action=view)
proposed patch

Ok to commit?

-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=2074

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

[Bug manual/2074] _IO_new_file_xsputn() in fileops.c not checking for EOF

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=2074

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |security-

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug manual/2074] _IO_new_file_xsputn() in fileops.c not checking for EOF

[schwab at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=2074

Andreas Schwab <schwab at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|jackie.rosen at hushmail dot com   |

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug manual/2074] _IO_new_file_xsputn() in fileops.c not checking for EOF

[jackie.rosen at hushmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=2074

Jackie Rosen <jackie.rosen at hushmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jackie.rosen at hushmail dot com

--- Comment #15 from Jackie Rosen <jackie.rosen at hushmail dot com> ---
*** Bug 260998 has been marked as a duplicate of this bug. ***
Seen from the domain http://volichat.com
Page where seen: http://volichat.com/adult-chat-rooms
Marked for reference. Resolved as fixed @bugzilla.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug manual/2074] _IO_new_file_xsputn() in fileops.c not checking for EOF

[mtk.manpages at gmail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=2074

Michael Kerrisk <mtk.manpages at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mtk.manpages at gmail dot
                   |                            |com

--- Comment #14 from Michael Kerrisk <mtk.manpages at gmail dot com> 2012-04-30 01:11:15 UTC ---
(In reply to comment #11)
> Created attachment 4777 [details]
> proposed patch
> 
> Ok to commit?

I've committed a corresponding change to the fopencookie(3) man page.

@@ -158,7 +158,8 @@ As its function result, the
 .I write
 function should return the number of bytes copied from
 .IR buf ,
-or \-1 on error.
+or 0 on error.
+(The function must not return a negative value.)
 The
 .I write
 function should update the stream offset appropriately.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug manual/2074] _IO_new_file_xsputn() in fileops.c not checking for EOF

[aj at suse dot de]

http://sourceware.org/bugzilla/show_bug.cgi?id=2074

--- Comment #13 from Andreas Jaeger <aj at suse dot de> 2012-04-12 19:09:46 UTC ---
I also filed Bug 13975 - fopencookie interface surprising.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug manual/2074] _IO_new_file_xsputn() in fileops.c not checking for EOF

[aj at suse dot de]

http://sourceware.org/bugzilla/show_bug.cgi?id=2074

Andreas Jaeger <aj at suse dot de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
                 CC|                            |aj at suse dot de
         Resolution|                            |FIXED

--- Comment #12 from Andreas Jaeger <aj at suse dot de> 2012-04-12 19:05:39 UTC ---
The patch to the manual and also a corresponding change to libio.h has been put
into git now.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.