View Bug Activity | Format For Printing
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().
Provide an example of a user of libc (not the internal functions) when this is a problem.
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;
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.
(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; }
Which function called though _IO_SYSWRITE returns -1? None should, that's the whole point.
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. > >
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.
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. > >
> _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.
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.
Created an attachment (id=4777) proposed patch Ok to commit?