This is the mail archive of the newlib@sources.redhat.com mailing list for the newlib project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA]: race safe fwalk


Thomas Pfaff wrote:
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.


No problem. I had to do some tweaking to get Linux to build with the change and I added a missing include in freopen64.c. Patch checked in. Thanks.


-- Jeff J.

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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]