cpukit/libio: Support close with IOP references held

- Provide an option for a file system to support close wtih
  references held. This can happen in more complex file systems
  and file descriptor handling with more complete reference
  handling implementations where an fd can hold other fds and
  close can be call on any fd and succeed.

- Fix open IOP leaks in the error paths.

- Provide better definition of the IOP flags to help clarify
  the code.

Fixes #5201

cpukit/include/rtems/libio.h

@@ -18,6 +18,8 @@
  * Modifications to support reference counting in the file system are
  * Copyright (C) 2012 embedded brains GmbH & Co. KG
  *
+ * Copyright (C) 2025 Contemporary Software
+ *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
@@ -1363,18 +1365,23 @@ typedef struct {
 } rtems_libio_ioctl_args_t;
 
 /**
- * @name Flag Values
+ * @name Flag Values and Masks
  */
 /**@{**/
 
-#define LIBIO_FLAGS_NO_DELAY      0x0001U  /* return immediately if no data */
-#define LIBIO_FLAGS_READ          0x0002U  /* reading */
-#define LIBIO_FLAGS_WRITE         0x0004U  /* writing */
-#define LIBIO_FLAGS_OPEN          0x0100U  /* device is open */
-#define LIBIO_FLAGS_APPEND        0x0200U  /* all writes append */
-#define LIBIO_FLAGS_CLOSE_ON_EXEC 0x0800U  /* close on process exec() */
-#define LIBIO_FLAGS_READ_WRITE    (LIBIO_FLAGS_READ | LIBIO_FLAGS_WRITE)
-#define LIBIO_FLAGS_REFERENCE_INC 0x1000U
+#define LIBIO_FLAGS_FREE           0x0001U  /* on the free list */
+#define LIBIO_FLAGS_NO_DELAY       0x0002U  /* return immediately if no data */
+#define LIBIO_FLAGS_READ           0x0004U  /* reading */
+#define LIBIO_FLAGS_WRITE          0x0008U  /* writing */
+#define LIBIO_FLAGS_OPEN           0x0100U  /* device is open */
+#define LIBIO_FLAGS_APPEND         0x0200U  /* all writes append */
+#define LIBIO_FLAGS_CLOSE_BUSY     0x0400U  /* close with refs held */
+#define LIBIO_FLAGS_CLOSE_ON_EXEC  0x0800U  /* close on process exec() */
+#define LIBIO_FLAGS_REFERENCE_INC  0x1000U
+/* masks */
+#define LIBIO_FLAGS_READ_WRITE     (LIBIO_FLAGS_READ | LIBIO_FLAGS_WRITE)
+#define LIBIO_FLAGS_FLAGS_MASK     (LIBIO_FLAGS_REFERENCE_INC - 1U)
+#define LIBIO_FLAGS_REFERENCE_MASK (~LIBIO_FLAGS_FLAGS_MASK)
 
 /** @} */
 
@@ -1383,6 +1390,20 @@ static inline unsigned int rtems_libio_iop_flags( const rtems_libio_t *iop )
   return _Atomic_Load_uint( &iop->flags, ATOMIC_ORDER_RELAXED );
 }
 
+/**
+ * @brief Returns true if the iop is a bad file descriptor, otherwise
+ * returns false. A bad file descriptor means free or not open.
+ *
+ * @param[in] flags The flags.
+ */
+static inline unsigned int rtems_libio_iop_flags_bad_fd(
+  const unsigned int flags
+)
+{
+  return ( ( flags & LIBIO_FLAGS_FREE ) != 0 )
+    || ( ( flags & LIBIO_FLAGS_OPEN ) == 0 );
+}
+
 /**
  * @brief Returns true if this is a no delay iop, otherwise returns false.
  *
@@ -1413,6 +1434,16 @@ static inline bool rtems_libio_iop_is_writeable( const rtems_libio_t *iop )
   return ( rtems_libio_iop_flags( iop ) & LIBIO_FLAGS_WRITE ) != 0;
 }
 
+/**
+ * @brief Returns true if the iop is open, otherwise returns false.
+ *
+ * @param[in] iop The iop.
+ */
+static inline bool rtems_libio_iop_is_open( const rtems_libio_t *iop )
+{
+  return ( rtems_libio_iop_flags( iop ) & LIBIO_FLAGS_OPEN ) != 0;
+}
+
 /**
  * @brief Returns true if this is an append iop, otherwise returns false.
  *
@@ -1423,6 +1454,28 @@ static inline bool rtems_libio_iop_is_append( const rtems_libio_t *iop )
   return ( rtems_libio_iop_flags( iop ) & LIBIO_FLAGS_APPEND ) != 0;
 }
 
+/**
+ * @brief Returns true if the iop is held, otherwise returns false.
+ *
+ * @param[in] iop The iop.
+ */
+static inline bool rtems_libio_iop_is_held( const rtems_libio_t *iop )
+{
+  const unsigned int ref_count =
+    rtems_libio_iop_flags( iop ) & LIBIO_FLAGS_REFERENCE_MASK;
+  return ref_count != 0;
+}
+
+/**
+ * @brief Returns true if the iop is free, otherwise returns false.
+ *
+ * @param[in] iop The iop.
+ */
+static inline bool rtems_libio_iop_is_free( const rtems_libio_t *iop )
+{
+  return ( rtems_libio_iop_flags( iop ) & LIBIO_FLAGS_FREE ) != 0;
+}
+
 /**
  * @name External I/O Handlers
  */

cpukit/libcsupport/src/close.c

@@ -63,8 +63,11 @@ int close(
       rtems_set_errno_and_return_minus_one( EBADF );
     }
 
-    /* The expected flags */
-    flags &= LIBIO_FLAGS_REFERENCE_INC - 1U;
+    /* The expected flags depends on close when busy flag. If set
+     * there can be references held when calling the close handler */
+    if ( ( flags & LIBIO_FLAGS_CLOSE_BUSY ) == 0 ) {
+      flags &= LIBIO_FLAGS_FLAGS_MASK;
+    }
 
     desired = flags & ~LIBIO_FLAGS_OPEN;
     success = _Atomic_Compare_exchange_uint(
@@ -79,7 +82,7 @@ int close(
       break;
     }
 
-    if ( ( flags & ~( LIBIO_FLAGS_REFERENCE_INC - 1U ) ) != 0 ) {
+    if ( ( flags & LIBIO_FLAGS_REFERENCE_MASK ) != 0 ) {
       rtems_set_errno_and_return_minus_one( EBUSY );
     }
   }

cpukit/libcsupport/src/fcntl.c

@@ -58,7 +58,7 @@ static int duplicate_iop( rtems_libio_t *iop )
     rtems_filesystem_location_clone( &diop->pathinfo, &iop->pathinfo );
     rtems_filesystem_instance_unlock( &iop->pathinfo );
 
-    rtems_libio_iop_flags_set( diop, rtems_libio_fcntl_flags( oflag ) );
+    rtems_libio_iop_flags_set( diop, rtems_libio_from_fcntl_flags( oflag ) );
     /*
      * XXX: We call the open handler here to have a proper open and close pair.
      *
@@ -99,7 +99,7 @@ static int duplicate2_iop( rtems_libio_t *iop, int fd2 )
 
     if (rv == 0) {
       oflag = rtems_libio_to_fcntl_flags( rtems_libio_iop_flags( iop ) );
-      rtems_libio_iop_flags_set( iop2, rtems_libio_fcntl_flags( oflag ) );
+      rtems_libio_iop_flags_set( iop2, rtems_libio_from_fcntl_flags( oflag ) );
 
       rtems_filesystem_instance_lock( &iop->pathinfo );
       rtems_filesystem_location_clone( &iop2->pathinfo, &iop->pathinfo );
@@ -177,7 +177,7 @@ static int vfcntl(
       break;
 
     case F_SETFL:
-      flags = rtems_libio_fcntl_flags( va_arg( ap, int ) );
+      flags = rtems_libio_from_fcntl_flags( va_arg( ap, int ) );
       mask = LIBIO_FLAGS_NO_DELAY | LIBIO_FLAGS_APPEND;
 
       /*
@@ -226,7 +226,10 @@ static int vfcntl(
 
   if (ret >= 0) {
     int err = (*iop->pathinfo.handlers->fcntl_h)( iop, cmd );
-    if (err) {
+    if (err == 0 && !rtems_libio_iop_is_open( iop ) ) {
+      err = EBADF;
+    }
+    if (err != 0) {
       errno = err;
       ret = -1;
     }

cpukit/libcsupport/src/freenode.c

@@ -43,8 +43,10 @@
 
 void rtems_filesystem_location_free( rtems_filesystem_location_info_t *loc )
 {
-  rtems_filesystem_instance_lock( loc );
-  (*loc->mt_entry->ops->freenod_h)( loc );
-  rtems_filesystem_instance_unlock( loc );
-  rtems_filesystem_location_remove_from_mt_entry( loc );
+  if ( loc->mt_entry != NULL ) {
+    rtems_filesystem_instance_lock( loc );
+    (*loc->mt_entry->ops->freenod_h)( loc );
+    rtems_filesystem_instance_unlock( loc );
+    rtems_filesystem_location_remove_from_mt_entry( loc );
+  }
 }

cpukit/libcsupport/src/fstat.c

@@ -65,6 +65,10 @@ int fstat(
   memset( sbuf, 0, sizeof(struct stat) );
 
   rv = (*iop->pathinfo.handlers->fstat_h)( &iop->pathinfo, sbuf );
+  if (rv == 0 && !rtems_libio_iop_is_open( iop ) ) {
+    errno = EBADF;
+    rv = -1;
+  }
   rtems_libio_iop_drop( iop );
   return rv;
 }

cpukit/libcsupport/src/libio.c

@@ -77,7 +77,7 @@ static const rtems_assoc_t status_flags_assoc[] = {
   { 0, 0, 0 },
 };
 
-unsigned int rtems_libio_fcntl_flags( int fcntl_flags )
+unsigned int rtems_libio_from_fcntl_flags( int fcntl_flags )
 {
   unsigned int   flags = 0;
   uint32_t   access_modes;
@@ -136,6 +136,8 @@ rtems_libio_t *rtems_libio_allocate( void )
   if ( iop != NULL ) {
     void *next;
 
+    rtems_libio_iop_flags_clear( iop, LIBIO_FLAGS_FREE );
+
     next = iop->data1;
     rtems_libio_iop_free_head = next;
 
@@ -149,30 +151,32 @@ rtems_libio_t *rtems_libio_allocate( void )
   return iop;
 }
 
-void rtems_libio_free(
+void rtems_libio_free_iop(
   rtems_libio_t *iop
 )
 {
   size_t zero;
 
-  rtems_filesystem_location_free( &iop->pathinfo );
-
   rtems_libio_lock();
 
-  /*
-   * 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 );
+  if ( !rtems_libio_iop_is_free( iop ) ) {
+    /*
+     * Clear the flags. All references should have been dropped.
+     */
+    _Atomic_Store_uint( &iop->flags, LIBIO_FLAGS_FREE, ATOMIC_ORDER_RELAXED );
 
-  /*
-   * 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;
+    rtems_filesystem_location_free( &iop->pathinfo );
+
+    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;
+  }
 
   rtems_libio_unlock();
 }

cpukit/libcsupport/src/libio_init.c

@@ -72,8 +72,10 @@ static void rtems_libio_init( void )
     if (rtems_libio_number_iops > 0)
     {
         iop = rtems_libio_iop_free_head = &rtems_libio_iops[0];
-        for (i = 0 ; (i + 1) < rtems_libio_number_iops ; i++, iop++)
+        for (i = 0 ; (i + 1) < rtems_libio_number_iops ; i++, iop++) {
+          rtems_libio_iop_flags_set( iop, LIBIO_FLAGS_FREE );
           iop->data1 = iop + 1;
+        }
         iop->data1 = NULL;
         rtems_libio_iop_free_tail = &iop->data1;
     }

cpukit/libcsupport/src/open.c

@@ -137,7 +137,7 @@ static int do_open(
   rtems_filesystem_eval_path_extract_currentloc( &ctx, &iop->pathinfo );
   rtems_filesystem_eval_path_cleanup( &ctx );
 
-  rtems_libio_iop_flags_set( iop, rtems_libio_fcntl_flags( oflag ) );
+  rtems_libio_iop_flags_set( iop, rtems_libio_from_fcntl_flags( oflag ) );
 
   rv = (*iop->pathinfo.handlers->open_h)( iop, path, oflag, mode );
 
@@ -168,10 +168,6 @@ static int do_open(
     }
   }
 
-  if ( rv < 0 ) {
-    rtems_libio_free( iop );
-  }
-
   return rv;
 }
 
@@ -192,6 +188,9 @@ int open( const char *path, int oflag, ... )
   iop = rtems_libio_allocate();
   if ( iop != NULL ) {
     rv = do_open( iop, path, oflag, mode );
+    if ( rv < 0 ) {
+      rtems_libio_free( iop );
+    }
   } else {
     errno = ENFILE;
     rv = -1;

testsuites/fstests/support/fstest.h

@@ -35,7 +35,7 @@
 
 #define FS_PASS() do {puts("PASS");} while (0)
 #define FS_FAIL() do {\
-  printf( "FAIL   %s: %d \n", __FILE__, __LINE__ );\
+  printf( "FAIL errno=%s  %s: %d \n", strerror(errno), __FILE__, __LINE__ );\
   fs_test_notify_failure(); \
  } while (0)