libio: Robust file descriptor reference counting

There was a race conditon in the reference counting of file descriptors
during a close() operation.  After the call to the close handler, the
rtems_libio_free() function cleared the flags to zero.  However, at this
point in time there may still exist some holders of the file descriptor.
With RTEMS_DEBUG enabled this could lead to failed assertions in
rtems_libio_iop_drop().

Change the code to use only atomic read-modify-write operations on the
rtems_libio_iop::flags.

9 files changed, 34 insertions, 43 deletions

diff --git a/cpukit/include/rtems/libio.h b/cpukit/include/rtems/libio.h
index b5362320e8..bbaae10efd 100644
--- a/cpukit/include/rtems/libio.h
+++ b/cpukit/include/rtems/libio.h
@@ -1318,8 +1318,8 @@ extern const rtems_filesystem_limits_and_options_t
  * to (eg: offset, driver, pathname should be in that)
  */
 struct rtems_libio_tt {
-  off_t                                   offset;    /* current offset into file */
   Atomic_Uint                             flags;
+  off_t                                   offset;    /* current offset into file */
   rtems_filesystem_location_info_t        pathinfo;
   uint32_t                                data0;     /* private to "driver" */
   void                                   *data1;     /* ... */
diff --git a/cpukit/include/rtems/libio_.h b/cpukit/include/rtems/libio_.h
index 916ba85b31..e9eb46263e 100644
--- a/cpukit/include/rtems/libio_.h
+++ b/cpukit/include/rtems/libio_.h
@@ -85,28 +85,6 @@ extern rtems_filesystem_mount_table_entry_t rtems_filesystem_null_mt_entry;
 extern rtems_filesystem_global_location_t rtems_filesystem_global_location_null;
 
 /**
- * @brief Sets the iop flags to the specified flags together with
- * LIBIO_FLAGS_OPEN.
- *
- * Use this once a file descriptor allocated via rtems_libio_allocate() is
- * fully initialized.
- *
- * @param[in] iop The iop.
- * @param[in] flags The flags.
- */
-static inline void rtems_libio_iop_flags_initialize(
-  rtems_libio_t *iop,
-  uint32_t       flags
-)
-{
-  _Atomic_Store_uint(
-    &iop->flags,
-    LIBIO_FLAGS_OPEN | flags,
-    ATOMIC_ORDER_RELEASE
-  );
-}
-
-/**
  * @brief Sets the specified flags in the iop.
  *
  * @param[in] iop The iop.
@@ -222,7 +200,7 @@ static inline void rtems_libio_iop_drop( rtems_libio_t *iop )
 
 #define rtems_libio_check_is_open(_iop) \
   do {                                               \
-      if (((_iop)->flags & LIBIO_FLAGS_OPEN) == 0) { \
+      if ((rtems_libio_iop_flags(_iop) & LIBIO_FLAGS_OPEN) == 0) { \
           errno = EBADF;                             \
           return -1;                                 \
       }                                              \
diff --git a/cpukit/libcsupport/src/fcntl.c b/cpukit/libcsupport/src/fcntl.c
index ddbf5538f9..86b757e511 100644
--- a/cpukit/libcsupport/src/fcntl.c
+++ b/cpukit/libcsupport/src/fcntl.c
@@ -45,9 +45,9 @@ static int duplicate_iop( rtems_libio_t *iop )
      */
     rv = (*diop->pathinfo.handlers->open_h)( diop, NULL, oflag, 0 );
     if ( rv == 0 ) {
-      rtems_libio_iop_flags_initialize(
+      rtems_libio_iop_flags_set(
         diop,
-        rtems_libio_fcntl_flags( oflag )
+        LIBIO_FLAGS_OPEN | rtems_libio_fcntl_flags( oflag )
       );
       rv = rtems_libio_iop_to_descriptor( diop );
     } else {
diff --git a/cpukit/libcsupport/src/libio.c b/cpukit/libcsupport/src/libio.c
index 0abb082a66..c9490c7aa5 100644
--- a/cpukit/libcsupport/src/libio.c
+++ b/cpukit/libcsupport/src/libio.c
@@ -134,11 +134,24 @@ void rtems_libio_free(
   rtems_libio_t *iop
 )
 {
+  size_t zero;
+
   rtems_filesystem_location_free( &iop->pathinfo );
 
   rtems_libio_lock();
 
-  iop = memset( iop, 0, sizeof( *iop ) );
+  /*
+   * Clear everything except the reference count part.  At this point in time
+   * there may be still some holders of this file descriptor.
+   */
+  rtems_libio_iop_flags_clear( iop, LIBIO_FLAGS_REFERENCE_INC - 1U );
+  zero = offsetof( rtems_libio_t, offset );
+  memset( (char *) iop + zero, 0, sizeof( *iop ) - zero );
+
+  /*
+   * Append it to the free list.  This increases the likelihood that a use
+   * after close is detected.
+   */
   *rtems_libio_iop_free_tail = iop;
   rtems_libio_iop_free_tail = &iop->data1;
 
diff --git a/cpukit/libcsupport/src/open.c b/cpukit/libcsupport/src/open.c
index 86d0bbeab9..b21dcb1876 100644
--- a/cpukit/libcsupport/src/open.c
+++ b/cpukit/libcsupport/src/open.c
@@ -115,11 +115,7 @@ static int do_open(
   rtems_filesystem_eval_path_extract_currentloc( &ctx, &iop->pathinfo );
   rtems_filesystem_eval_path_cleanup( &ctx );
 
-  _Atomic_Store_uint(
-    &iop->flags,
-    rtems_libio_fcntl_flags( oflag ),
-    ATOMIC_ORDER_RELAXED
-  );
+  rtems_libio_iop_flags_set( iop, rtems_libio_fcntl_flags( oflag ) );
 
   rv = (*iop->pathinfo.handlers->open_h)( iop, path, oflag, mode );
 
diff --git a/cpukit/libcsupport/src/sup_fs_deviceio.c b/cpukit/libcsupport/src/sup_fs_deviceio.c
index 294e14a260..4a7e0dd13c 100644
--- a/cpukit/libcsupport/src/sup_fs_deviceio.c
+++ b/cpukit/libcsupport/src/sup_fs_deviceio.c
@@ -34,7 +34,7 @@ int rtems_deviceio_open(
   rtems_libio_open_close_args_t args;
 
   args.iop = iop;
-  args.flags = iop->flags;
+  args.flags = rtems_libio_iop_flags( iop );
   args.mode = mode;
 
   status = rtems_io_open( major, minor, &args );
@@ -75,7 +75,7 @@ ssize_t rtems_deviceio_read(
   args.offset = iop->offset;
   args.buffer = buf;
   args.count = nbyte;
-  args.flags = iop->flags;
+  args.flags = rtems_libio_iop_flags( iop );
   args.bytes_moved = 0;
 
   status = rtems_io_read( major, minor, &args );
@@ -103,7 +103,7 @@ ssize_t rtems_deviceio_write(
   args.offset = iop->offset;
   args.buffer = RTEMS_DECONST( void *, buf );
   args.count = nbyte;
-  args.flags = iop->flags;
+  args.flags = rtems_libio_iop_flags( iop );
   args.bytes_moved = 0;
 
   status = rtems_io_write( major, minor, &args );
diff --git a/cpukit/libcsupport/src/termios.c b/cpukit/libcsupport/src/termios.c
index b10a4ad774..81518f36de 100644
--- a/cpukit/libcsupport/src/termios.c
+++ b/cpukit/libcsupport/src/termios.c
@@ -2110,7 +2110,7 @@ rtems_termios_imfs_open (rtems_libio_t *iop,
 
   memset (&args, 0, sizeof (args));
   args.iop = iop;
-  args.flags = iop->flags;
+  args.flags = rtems_libio_iop_flags (iop);
   args.mode = mode;
 
   rtems_termios_obtain ();
@@ -2162,7 +2162,7 @@ rtems_termios_imfs_read (rtems_libio_t *iop, void *buffer, size_t count)
     args.iop = iop;
     args.buffer = buffer;
     args.count = count;
-    args.flags = iop->flags;
+    args.flags = rtems_libio_iop_flags (iop);
 
     sc = rtems_termios_linesw[tty->t_line].l_read (tty, &args);
     tty->tty_rcvwakeup = false;
@@ -2202,7 +2202,7 @@ rtems_termios_imfs_write (rtems_libio_t *iop, const void *buffer, size_t count)
     args.iop = iop;
     args.buffer = RTEMS_DECONST (void *, buffer);
     args.count = count;
-    args.flags = iop->flags;
+    args.flags = rtems_libio_iop_flags (iop);
 
     sc = rtems_termios_linesw[tty->t_line].l_write (tty, &args);
     rtems_mutex_unlock (&tty->osem);
diff --git a/cpukit/libnetworking/rtems/rtems_syscall.c b/cpukit/libnetworking/rtems/rtems_syscall.c
index 5fa0e7d10c..56a8530564 100644
--- a/cpukit/libnetworking/rtems/rtems_syscall.c
+++ b/cpukit/libnetworking/rtems/rtems_syscall.c
@@ -87,7 +87,7 @@ rtems_bsdnet_makeFdForSocket (void *so)
   iop->pathinfo.handlers = &socket_handlers;
   iop->pathinfo.mt_entry = &rtems_filesystem_null_mt_entry;
   rtems_filesystem_location_add_to_mt_entry(&iop->pathinfo);
-  rtems_libio_iop_flags_initialize(iop, LIBIO_FLAGS_READ_WRITE);
+  rtems_libio_iop_flags_set(iop, LIBIO_FLAGS_OPEN | LIBIO_FLAGS_READ_WRITE);
   return fd;
 }
 
@@ -739,14 +739,18 @@ rtems_bsdnet_write (rtems_libio_t *iop, const void *buffer, size_t count)
 static int
 so_ioctl (rtems_libio_t *iop, struct socket *so, uint32_t   command, void *buffer)
 {
+	unsigned int nonblock;
+
 	switch (command) {
 	case FIONBIO:
+		nonblock = rtems_libio_fcntl_flags (O_NONBLOCK);
+
 		if (*(int *)buffer) {
-			iop->flags |= O_NONBLOCK;
+			rtems_libio_iop_flags_set (iop, nonblock);
 			so->so_state |= SS_NBIO;
 		}
 		else {
-			iop->flags &= ~O_NONBLOCK;
+			rtems_libio_iop_flags_clear (iop, nonblock);
 			so->so_state &= ~SS_NBIO;
 		}
 		return 0;
diff --git a/cpukit/posix/src/shmopen.c b/cpukit/posix/src/shmopen.c
index 22b5868bda..054a035f16 100644
--- a/cpukit/posix/src/shmopen.c
+++ b/cpukit/posix/src/shmopen.c
@@ -286,14 +286,14 @@ int shm_open( const char *name, int oflag, mode_t mode )
   iop->pathinfo.mt_entry = &rtems_filesystem_null_mt_entry;
   rtems_filesystem_location_add_to_mt_entry( &iop->pathinfo );
 
-  flags = LIBIO_FLAGS_CLOSE_ON_EXEC;
+  flags = LIBIO_FLAGS_OPEN | LIBIO_FLAGS_CLOSE_ON_EXEC;
   if ( (oflag & O_ACCMODE) == O_RDONLY ) {
     flags |= LIBIO_FLAGS_READ;
   } else {
     flags |= LIBIO_FLAGS_READ_WRITE;
   }
 
-  rtems_libio_iop_flags_initialize( iop, flags );
+  rtems_libio_iop_flags_set( iop, flags );
 
   return fd;
 }