RTEMS | Backport Chris's libio fix to 6 branch (#5311)

[Sebastian Huber (@sebhub)]

Sebastian Huber commented: https://gitlab.rtems.org/rtems/rtos/rtems/-/issues/5311#note_130878


The commit 0cf6de01df734c11212e63134551ee344a10ecec introduced a couple of issues:

* ABI changes on a release branch
* Redundant LIBIO_FLAGS_FREE flag: an iop is free if and only if it is on the free list.
* When the iop is freed, its reference count was set to zero. The iop reference count must never be set to a specific value after initialization at system start. The only valid operations are increments and decrements.
* In the iop free handling, there was a deadlock potential by obtaining the libio lock while owning a file system lock.
* The reference counting did not work at all under concurrent access. This was indicated by the failing spintrcritical24 test program.

The spintrcritical24 test was specifically added to test the file descriptor reference counting. It fails now on at least the sparc/gr740 and arm/xilinx_zynq_zc702 simulators. It fails since `rtems_libio_free()` may free the iop by looking at the current flags. This is plainly wrong, actions must be carried out when certain state changes happen. Consider this sequence of events:

```c
int close(
  int  fd
)
{
  rtems_libio_t *iop;
  unsigned int   flags;
  int            rc;

  if ( (uint32_t) fd >= rtems_libio_number_iops ) {
    rtems_set_errno_and_return_minus_one( EBADF );
  }

  iop = rtems_libio_iop( fd );
  flags = rtems_libio_iop_flags( iop );

  while ( true ) {
    unsigned int desired;
    bool         success;

    if ( ( flags & LIBIO_FLAGS_OPEN ) == 0 ) {
      rtems_set_errno_and_return_minus_one( EBADF );
    }

    /* 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(
      &iop->flags,
      &flags,
      desired,
      ATOMIC_ORDER_ACQ_REL,
      ATOMIC_ORDER_RELAXED
    );

    if ( success ) {
      break;
    }

    if ( ( flags & LIBIO_FLAGS_REFERENCE_MASK ) != 0 ) {
      rtems_set_errno_and_return_minus_one( EBUSY );
    }
  }

<--- Lets suppose LIBIO_FLAGS_CLOSE_BUSY is cleared. At this point we cleared LIBIO_FLAGS_OPEN above and the reference count is zero. Let an interrupt call for example fstat(fd). It will hold and drop the iop. The drop will call rtems_libio_free() which will calle rtems_libio_free_iop() since the iop is no longer open and has a zero reference count. This will clear the iop->pathinfo. This will lead to a NULL pointer access here:

  rc = (*iop->pathinfo.handlers->close_h)( iop );

  rtems_libio_free( iop );

  return rc;
}
```

-- 
View it on GitLab: https://gitlab.rtems.org/rtems/rtos/rtems/-/issues/5311#note_130878
You're receiving this email because of your account on gitlab.rtems.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/bugs/attachments/20250915/c7e17f57/attachment-0001.htm>