Bug 2337 - libio in wide mode deallocates user supplied buffer
Summary: libio in wide mode deallocates user supplied buffer
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: libc (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Ulrich Drepper
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-02-14 17:05 UTC by Petr.Salinger
Modified: 2016-05-12 15:02 UTC (History)
2 users (show)

See Also:
Host: i486-linux-gnu
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments
code snippet (486 bytes, text/x-csrc)
2006-02-14 17:07 UTC, Petr.Salinger
Details
libio patch to prevent spurious deallocation of user supplied buffer. (1.05 KB, patch)
2006-10-06 06:00 UTC, Ryan S. Arnold
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Petr.Salinger 2006-02-14 17:05:27 UTC
Hello.

Please try to run attached source under "strace -e trace=mmap2,munmap"
At the end there will be something like:

hello.
buf = 0x8049160, fp = 0x804a008,  fp+delta = 0x804a04f
munmap(0x804a04f, 4096)                 = -1 EINVAL (Invalid argument)

On file close libio tries to free buffer, which have not been allocated by libio.
In first case it is internal 1byte buffer, but it can be also user specified buffer,
just compile with  -DUSER_BUF.  It leads to crashes.

Petr
Comment 1 Petr.Salinger 2006-02-14 17:07:01 UTC
Created attachment 864 [details]
code snippet
Comment 2 Petr.Salinger 2006-03-02 14:06:52 UTC
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
Comment 3 Petr.Salinger 2006-05-03 11:30:40 UTC
libio deallocates user supplied buffer - same behaviour also for  glibc-20060501
CVS snapshot
Comment 4 Ryan S. Arnold 2006-10-04 16:46:07 UTC
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.
Comment 5 Ryan S. Arnold 2006-10-06 06:00:24 UTC
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.
Comment 6 Ryan S. Arnold 2006-10-06 17:09:56 UTC
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
Comment 7 Ulrich Drepper 2006-12-13 23:18:01 UTC
Fixed in CVS.
Comment 8 cvs-commit@gcc.gnu.org 2007-01-12 17:25:58 UTC
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

Comment 9 Florian Weimer 2016-05-12 15:02:34 UTC
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.