From c2287ba2cff59a50848151833404bce0e3cf0a70 Mon Sep 17 00:00:00 2001
From: Sebastian Huber <sebastian.huber@embedded-brains.de>
Date: Tue, 10 Mar 2020 17:07:19 +0100
Subject: 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.
---
 cpukit/libcsupport/src/fcntl.c           |  4 ++--
 cpukit/libcsupport/src/libio.c           | 15 ++++++++++++++-
 cpukit/libcsupport/src/open.c            |  6 +-----
 cpukit/libcsupport/src/sup_fs_deviceio.c |  6 +++---
 cpukit/libcsupport/src/termios.c         |  6 +++---
 5 files changed, 23 insertions(+), 14 deletions(-)

(limited to 'cpukit/libcsupport')

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);
-- 
cgit v1.2.3