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]

[RFA]: race safe fwalk


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.


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

	* libc/stdio/fdopen.c (_fdopen_r): Add missing
	__lock_init_recursive.
	Set FILE flags as the last step.
	* libc/stdio/fopen.c (_fopen_r): Set FILE flags as the last
	step.
	* libc/stdio/fwalk.c: Check for _flags != 1 to make sure that
	file is really open.
	* libc/stdio64/fdopen64.c (_fdopen64_r): Set FILE flags as the
	last step.
	* libc/stdio64/fopen64.c (_fopen64_r): Ditto.

Index: stdio/fdopen.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio/fdopen.c,v
retrieving revision 1.6
diff -u -p -r1.6 fdopen.c
--- stdio/fdopen.c	19 Sep 2002 21:28:52 -0000	1.6
+++ stdio/fdopen.c	8 Mar 2004 20:01:06 -0000
@@ -76,18 +76,7 @@ _DEFUN (_fdopen_r, (ptr, fd, mode),
 
   if ((fp = __sfp (ptr)) == 0)
     return 0;
-  fp->_flags = flags;
-  /*
-   * If opened for appending, but underlying descriptor
-   * does not have O_APPEND bit set, assert __SAPP so that
-   * __swrite() will lseek to end before each write.
-   */
-  if ((oflags & O_APPEND)
-#ifdef HAVE_FCNTL
-       && !(fdflags & O_APPEND)
-#endif
-      )
-    fp->_flags |= __SAPP;
+
   fp->_file = fd;
   fp->_cookie = (_PTR) fp;
 
@@ -100,7 +89,21 @@ _DEFUN (_fdopen_r, (ptr, fd, mode),
   fp->_write = __swrite;
   fp->_seek = __sseek;
   fp->_close = __sclose;
-
+#ifndef __SINGLE_THREAD__
+  __lock_init_recursive (*(_LOCK_RECURSIVE_T *)&fp->_lock);
+#endif
+  fp->_flags = flags;
+  /*
+   * If opened for appending, but underlying descriptor
+   * does not have O_APPEND bit set, assert __SAPP so that
+   * __swrite() will lseek to end before each write.
+   */
+  if ((oflags & O_APPEND)
+#ifdef HAVE_FCNTL
+       && !(fdflags & O_APPEND)
+#endif
+      )
+    fp->_flags |= __SAPP;
 #ifdef __SCLE
   /* Explicit given mode results in explicit setting mode on fd */
   if (oflags & O_BINARY)
Index: stdio/fopen.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio/fopen.c,v
retrieving revision 1.5
diff -u -p -r1.5 fopen.c
--- stdio/fopen.c	22 Aug 2003 18:52:25 -0000	1.5
+++ stdio/fopen.c	8 Mar 2004 20:01:07 -0000
@@ -143,12 +143,15 @@ _DEFUN (_fopen_r, (ptr, file, mode),
     }
 
   fp->_file = f;
-  fp->_flags = flags;
   fp->_cookie = (_PTR) fp;
   fp->_read = __sread;
   fp->_write = __swrite;
   fp->_seek = __sseek;
   fp->_close = __sclose;
+#ifndef __SINGLE_THREAD__
+  __lock_init_recursive (*(_LOCK_RECURSIVE_T *)&fp->_lock);
+#endif
+  fp->_flags = flags;
 
   if (fp->_flags & __SAPP)
     _fseek_r (ptr, fp, 0, SEEK_END);
@@ -156,10 +159,6 @@ _DEFUN (_fopen_r, (ptr, file, mode),
 #ifdef __SCLE
   if (__stextmode (fp->_file))
     fp->_flags |= __SCLE;
-#endif
-
-#ifndef __SINGLE_THREAD__
-  __lock_init_recursive (*(_LOCK_RECURSIVE_T *)&fp->_lock);
 #endif
 
   return fp;
Index: stdio/fwalk.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio/fwalk.c,v
retrieving revision 1.3
diff -u -p -r1.3 fwalk.c
--- stdio/fwalk.c	31 Jan 2004 00:39:07 -0000	1.3
+++ stdio/fwalk.c	8 Mar 2004 20:01:07 -0000
@@ -40,14 +40,14 @@ _fwalk (ptr, function)
     {
       for (g = &ptr->__sglue; g != NULL; g = g->_next)
         for (fp = g->_iobs, n = g->_niobs; --n >= 0; fp++)
-          if (fp->_flags != 0)
+          if (fp->_flags != 0 && fp->_flags != 1)
 	    ret |= (*function) (fp);
     }
 
   /* Must traverse global list for all other streams.  */
   for (g = &_GLOBAL_REENT->__sglue; g != NULL; g = g->_next)
     for (fp = g->_iobs, n = g->_niobs; --n >= 0; fp++)
-      if (fp->_flags != 0)
+      if (fp->_flags != 0 && fp->_flags != 1)
 	ret |= (*function) (fp);
 
   return ret;
Index: stdio64/fdopen64.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio64/fdopen64.c,v
retrieving revision 1.2
diff -u -p -r1.2 fdopen64.c
--- stdio64/fdopen64.c	25 Jul 2003 16:19:55 -0000	1.2
+++ stdio64/fdopen64.c	8 Mar 2004 20:01:07 -0000
@@ -63,18 +63,6 @@ _DEFUN (_fdopen64_r, (ptr, fd, mode),
 
   if ((fp = __sfp (ptr)) == 0)
     return 0;
-  fp->_flags = flags;
-  /*
-   * If opened for appending, but underlying descriptor
-   * does not have O_APPEND bit set, assert __SAPP so that
-   * __swrite() will lseek to end before each write.
-   */
-  if ((oflags & O_APPEND)
-#ifdef HAVE_FCNTL
-       && !(fdflags & O_APPEND)
-#endif
-      )
-    fp->_flags |= __SAPP;
   fp->_file = fd;
   fp->_cookie = (_PTR) fp;
 
@@ -89,6 +77,21 @@ _DEFUN (_fdopen64_r, (ptr, fd, mode),
   fp->_seek64 = __sseek64;
   fp->_close = __sclose;
 
+#ifndef __SINGLE_THREAD__
+  __lock_init_recursive (*(_LOCK_RECURSIVE_T *)&fp->_lock);
+#endif
+  fp->_flags = flags;
+  /*
+   * If opened for appending, but underlying descriptor
+   * does not have O_APPEND bit set, assert __SAPP so that
+   * __swrite() will lseek to end before each write.
+   */
+  if ((oflags & O_APPEND)
+#ifdef HAVE_FCNTL
+       && !(fdflags & O_APPEND)
+#endif
+      )
+    fp->_flags |= __SAPP;
 #ifdef __SCLE
   /* Explicit given mode results in explicit setting mode on fd */
   if (oflags & O_BINARY)
@@ -99,9 +102,6 @@ _DEFUN (_fdopen64_r, (ptr, fd, mode),
     fp->_flags |= __SCLE;
 #endif
 
-#ifndef __SINGLE_THREAD__
-  __lock_init_recursive (*(_LOCK_RECURSIVE_T *)&fp->_lock);
-#endif
 
   fp->_flags |= __SL64;
 
Index: stdio64/fopen64.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio64/fopen64.c,v
retrieving revision 1.4
diff -u -p -r1.4 fopen64.c
--- stdio64/fopen64.c	22 Aug 2003 18:52:25 -0000	1.4
+++ stdio64/fopen64.c	8 Mar 2004 20:01:07 -0000
@@ -96,13 +96,17 @@ _DEFUN (_fopen64_r, (ptr, file, mode),
     }
 
   fp->_file = f;
-  fp->_flags = flags;
   fp->_cookie = (_PTR) fp;
   fp->_read = __sread;
   fp->_write = __swrite64;
   fp->_seek = __sseek;
   fp->_seek64 = __sseek64;
   fp->_close = __sclose;
+#ifndef __SINGLE_THREAD__
+  __lock_init_recursive (*(_LOCK_RECURSIVE_T *)&fp->_lock);
+#endif
+
+  fp->_flags = flags;
 
   if (fp->_flags & __SAPP)
     _fseeko64_r (ptr, fp, 0, SEEK_END);
@@ -113,10 +117,6 @@ _DEFUN (_fopen64_r, (ptr, file, mode),
 #endif
 
   fp->_flags |= __SL64;
-
-#ifndef __SINGLE_THREAD__
-  __lock_init_recursive (*(_LOCK_RECURSIVE_T *)&fp->_lock);
-#endif
 
   return fp;
 }

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