Bug 2337

Created attachment 864 [details]
code snippet

Bug 2337 - libio in wide mode deallocates user supplied buffer, 
same behaviour also for post 2.3.91 CVS snapshot

Attached code snippet compiled with -DUSER_BUF still segfaults, 
can also be used for testsuite.

Petr

libio deallocates user supplied buffer - same behaviour also for  glibc-20060501
CVS snapshot

I've identified two problems with the glibc src code:

1.) The first fwprintf() invocation automatically reorients the FILE stream as
'wide' using _IO_fwide().  The user provided buffer (_IO_FILE->_IO_buf_base) is
NOT USED as the wide character buffer(_IO_FILE->_wide_data->_IO_buf_base).  This
causes vfprintf to detect an empty buffer and __woverflow allocates an internal
wide character buffer the size of the file system blk_size (i.e. 1024) to use
for wide character vfprintf.  This is not directly related to the spurious
deallocation of the user supplied buffer.

2.) When fclose is called _IO_new_fclose() invokes INT_USE(_IO_file_close_it())
which zeros the _IO_FILE struct _flags field:

fp->_flags = _IO_MAGIC|CLOSED_FILEBUF_FLAGS;

following which _IO_new_fclose() invokes _IO_FINISH(fp) which calls
_IO_new_file_finish() (the _IO_wfile_jumps entry for __finish) which detects an
unset _IO_USER_BUF and free's the buffer spuriously.

Possible solutions:
1.) When the stream is reoriented set _IO_FILE->_wide_data->_IO_buf_base =
_IO_FILE->_IO_buf_base; _IO_FILE->_IO_buf_base = NULL;  This will cause wide
character printf to use the user supplied buffer.
 
2a.) Reset the _IO_USER_BUF bit flag to '1' after clearing _IO_FILE->_flags if
it was set before the clearing the _flags in _IO_file_close_it().

2b.) Provide a wide character centric 'finish' function and adjust the
_IO_wfile_jumps jump table entry to use the new function rather than reusing the
non-wide character centric version, i.e.:

JUMP_INIT(finish, _IO_wfile_finish),

instead of what currently exists:

JUMP_INIT(finish, _IO_new_file_finish),

Then, since the FILE stream has been reoriented to 'wide' the _IO_wfile_finish()
would properly only care about the wide character allocated buffer in the manner
of _IO_wsetb().

I'll investigate the specifications to see if wide character usage is supposed
to use the user supplied buffer.

In the meantime I can provide a patch for solution 2a).  It may not be the right
decision but we'll investigate.

Created attachment 1351 [details]
libio patch to prevent spurious deallocation of user supplied buffer.

Directing wide-character IO to use the user supplied buffer proved to be
problematic because the wide character operations make use of the non-wide
character buffer for write operations.

The least intrusive solution was to clean up the IO file finish path.  The wide
character jump vtable is initialized such that wide-character IO uses the
default (non-wide) _IO_file_finish() function (probably an oversight) which
invokes _IO_default_finish().  This ends up checking and clearing the non-wide
user buffer spuriously in _IO_new_fclose().

I created a wide-character oriented file finish function _IO_wfile_finish()
which calls the already existing _IO_wdefault_finish() function and I added it
to the wide IO jump table as the default IO file finish function.

This solved the problem and wide character IO now finishes in a manner
consistent with the IO orientation.

I've only tested this on PowerPC thus far.

Per this thread on libc-alpha Ulrich has suggested that this bug be left to him:

http://sources.redhat.com/ml/libc-alpha/2006-10/msg00014.html

Fixed in CVS.

Subject: Bug 2337

CVSROOT:	/cvs/glibc
Module name:	libc
Branch: 	glibc-2_5-branch
Changes by:	jakub@sourceware.org	2007-01-12 17:25:39

Modified files:
	.              : ChangeLog 
	libio          : Makefile fileops.c genops.c libio.h 
	                 wfiledoalloc.c wgenops.c wmemstream.c wstrops.c 
Added files:
	libio          : tst-setvbuf1.c 

Log message:
	[BZ #2337]
	* libio/Makefile (tests): Add tst-setvbuf1.
	* libio/tst-setvbuf1.c: New file.
	
	[BZ #2337]
	* libio/genops.c (__uflow): Fix a typo.
	* libio/wfiledoalloc.c (_IO_wfile_doallocate): Don't stat
	nor set _IO_LINE_BUF bit here.  Size the wide buffer based on
	the narrow buffer size.
	
	[BZ #2337]
	* libio/libio.h (_IO_FLAGS2_USER_WBUF): Define.
	* libio/wgenops.c (_IO_wsetb, _IO_wdefault_finish): Test and set
	_IO_FLAGS2_USER_WBUF bit in _flags2 instead of _IO_USER_BUF bit
	in _flags.
	* libio/wstrops.c (_IO_wstr_overflow, enlarge_userbuf,
	_IO_wstr_finish): Likewise.
	* libio/wmemstream.c (open_wmemstream): Likewise.
	* libio/fileops.c (_IO_new_file_close_it): Call _IO_set[bgp]
	even for wide streams.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/libc/ChangeLog.diff?cvsroot=glibc&only_with_tag=glibc-2_5-branch&r1=1.10362.2.21&r2=1.10362.2.22
http://sourceware.org/cgi-bin/cvsweb.cgi/libc/libio/tst-setvbuf1.c.diff?cvsroot=glibc&only_with_tag=glibc-2_5-branch&r1=NONE&r2=1.1.4.1
http://sourceware.org/cgi-bin/cvsweb.cgi/libc/libio/Makefile.diff?cvsroot=glibc&only_with_tag=glibc-2_5-branch&r1=1.86&r2=1.86.2.1
http://sourceware.org/cgi-bin/cvsweb.cgi/libc/libio/fileops.c.diff?cvsroot=glibc&only_with_tag=glibc-2_5-branch&r1=1.110&r2=1.110.2.1
http://sourceware.org/cgi-bin/cvsweb.cgi/libc/libio/genops.c.diff?cvsroot=glibc&only_with_tag=glibc-2_5-branch&r1=1.70&r2=1.70.2.1
http://sourceware.org/cgi-bin/cvsweb.cgi/libc/libio/libio.h.diff?cvsroot=glibc&only_with_tag=glibc-2_5-branch&r1=1.64&r2=1.64.2.1
http://sourceware.org/cgi-bin/cvsweb.cgi/libc/libio/wfiledoalloc.c.diff?cvsroot=glibc&only_with_tag=glibc-2_5-branch&r1=1.6&r2=1.6.8.1
http://sourceware.org/cgi-bin/cvsweb.cgi/libc/libio/wgenops.c.diff?cvsroot=glibc&only_with_tag=glibc-2_5-branch&r1=1.14&r2=1.14.2.1
http://sourceware.org/cgi-bin/cvsweb.cgi/libc/libio/wmemstream.c.diff?cvsroot=glibc&only_with_tag=glibc-2_5-branch&r1=1.3&r2=1.3.2.1
http://sourceware.org/cgi-bin/cvsweb.cgi/libc/libio/wstrops.c.diff?cvsroot=glibc&only_with_tag=glibc-2_5-branch&r1=1.11&r2=1.11.2.1

This reliably does not work (if an application does this, it will crash), so the bug (although it can lead to a dangling pointer or double-free) does not appear to be a security issue.