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.


_______________________________________________
bugs mailing list
[email protected]
http://lists.rtems.org/mailman/listinfo/bugs

Reply via email to