[Bug 2054] New: Race condition in FIFO/pipe implementation

[bugzilla-daemon at rtems.org]

https://www.rtems.org/bugzilla/show_bug.cgi?id=2054

             Bug #: 2054
           Summary: Race condition in FIFO/pipe implementation
    Classification: Unclassified
           Product: RTEMS
           Version: HEAD
          Platform: All
        OS/Version: RTEMS
            Status: NEW
          Severity: normal
          Priority: P3
         Component: cpukit
        AssignedTo: joel.sherrill at oarcorp.com
        ReportedBy: sebastian.huber at embedded-brains.de


Does anyone know why the FIFO/pipe implementation uses barriers for
synchronization?  For me the usage of the barrier looks like a hand crafted
condition variable.

I think there is also a race condition in its use.  The rtems_barrier_release()
releases only the threads currently waiting at the barrier.  Now we have
several code sections like this:

      PIPE_UNLOCK(pipe);
      if (! PIPE_WRITEWAIT(pipe))
        ret = -EINTR;

Thus the release may have no effect in case it happens after the PIPE_UNLOCK
and before the PIPE_WRITEWAIT (which is a rtems_barrier_wait() with no time
out).

The bdbuf code uses a similar approach with semaphores.  It disables the
preemption to avoid this race condition:

static void
rtems_bdbuf_anonymous_wait (rtems_bdbuf_waiters *waiters)
{
  rtems_status_code sc;
  rtems_mode        prev_mode;

  /*
   * Indicate we are waiting.
   */
  ++waiters->count;

  /*
   * Disable preemption then unlock the cache and block.  There is no POSIX
   * condition variable in the core API so this is a work around.
   *
   * The issue is a task could preempt after the cache is unlocked because it
is
   * blocking or just hits that window, and before this task has blocked on the
   * semaphore. If the preempting task flushes the queue this task will not see
   * the flush and may block for ever or until another transaction flushes this
   * semaphore.
   */
  prev_mode = rtems_bdbuf_disable_preemption ();

  /*
   * Unlock the cache, wait, and lock the cache when we return.
   */
  rtems_bdbuf_unlock_cache ();

  sc = rtems_semaphore_obtain (waiters->sema, RTEMS_WAIT,
RTEMS_BDBUF_WAIT_TIMEOUT);

  if (sc == RTEMS_TIMEOUT)
    rtems_fatal_error_occurred (RTEMS_BLKDEV_FATAL_BDBUF_CACHE_WAIT_TO);

  if (sc != RTEMS_UNSATISFIED)
    rtems_fatal_error_occurred (RTEMS_BLKDEV_FATAL_BDBUF_CACHE_WAIT_2);

  rtems_bdbuf_lock_cache ();

  rtems_bdbuf_restore_preemption (prev_mode);

  --waiters->count;
}

Please note that this approach is not SMP proof.  I think we need condition
variables in the RTEMS Classic API.

-- 
Configure bugmail: https://www.rtems.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.