[RFA]: race safe fwalk

[Thomas Pfaff]

Jeff Johnston wrote:
> Thomas Pfaff wrote:
> 
>> This time with attachment.
>>
>> There is a possible race between fwalk and fopen:
>>
>> When a thread make a call to fopen the FILE * _flags will be set to 1 
>> in findfp to mark it used and later it will be changed to the real 
>> FILE flag.
>>
>> When another thread calls fwalk during that time fwalk will treat the 
>> FILE as already opened and calls the callback functions with the yet 
>> unopened and only partially initialized FILE *.
>>
>> This can be avoided by checking for fp->_flags != 0 && fp->_flags != 
>> 1. Since _flags is signed short i did not check for _flags > 1. The 
>> flag should be set as the last step in an open call.
>> I do not think that 1 is a valid _flag for an open file. Correct me if 
>> am wrong.
>>
> 
> Unfortunately, 1 is also line-buffered: __SLBF.
> 
> What if instead, we use the _file field to check for a valid file.  It 
> gets set to -1 by __sfp.  Now, if we set _file inside the __sfp_lock and 
> then had fwalk() use the __sfp lock as well and check for _file != -1, 
> plus have the open routines set the _file field last, this should work 
> equally as well.  Comments?
> 

I do not think that this can be done that easy anymore. The whole file 
list stuff must be better protected.

Attached is a new patch. I am sorry for the delay.

2004-03-22  Thomas Pfaff  <tpfaff@gmx.net>

	* libc/stdio/fclose.c (fclose): Protect file pointer list when
	releasing a file.
  	* libc/stdio/fcloseall.c (_fcloseall_r): Close all files via
	fwalk.
	* libc/stdio/fdopen.c (_fdopen_r): Add calls to
	_flockfile/_funlockfile.
	* libc/stdio/findfp.c: Move __sfp_lock. Change __sfp_lock type
	to recursive.
	Change __lock_acquire/__lock_release calls for __sfp_lock to
	__sfp_lock_acquire/__sfp_lock_release throughout.
	(std): Make sure that file lock is only initialized once.
	(__sfp): Move _file initialization. Initialize file lock.
	(__sfp_lock_acquire): New function.
	(__sfp_lock_release): Ditto.
	(__fp_lock_all): Remove __sfp_lock_acquire call.
	(__fp_unlock_all): Remove __sfp_lock_release call.
	* libc/stdio/fopen.c (_fopen_r): Protect file pointer list.
	Add calls to _flockfile/_funlockfile. Remove
	__lock_init_recursive call.
	* libc/stdio/freopen.c (_freopen_r): Protect file pointer list.
	* libc/stdio/fwalk.c (__fwalk): New static function.
	(_fwalk): Protect file pointer list. Use __fwalk to walk through
	file pointers.
	* libc/stdio/local.h: Add defines for
	__sfp_lock_acquire/__sfp_lock_release when
	single threaded. Add function prototypes otherwise.
	* libc/stdio64/fdopen64.c (_fdopen64_r): Add calls to
	_flockfile/_funlockfile.
	* libc/stdio/fopen64.c (_fopen64_r): Protect file pointer list.
	Add calls to _flockfile/_funlockfile. Remove
	 __lock_init_recursive call.
	* libc/stdio/freopen64.c (_freopen64_r): Protect file pointer
	list.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: newlib-stdio.patch
URL: <http://sourceware.org/pipermail/newlib/attachments/20040323/57793f6e/attachment.ksh>