[lwip-devel] [bug #43278] event_callback() handle context switch when calling sys_sem_signal()

[Joel Cunningham]

URL:
  <http://savannah.nongnu.org/bugs/?43278>

                 Summary: event_callback() handle context switch when calling
sys_sem_signal()
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: jcunningham
            Submitted on: Mon 22 Sep 2014 02:18:05 PM GMT
                Category: sockets/netconn
                Severity: 3 - Normal
              Item Group: Crash Error
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: 
            lwIP version: 1.4.1

    _______________________________________________________

Details:

In event_callback(), the for loop at the bottom that handles signaling waiting
application threads that a select is ready, operates under the assumption that
the call to sys_sem_signal() won't cause a context switch.  The entire loop is
protected by a pair of SYS_ARCH_PROTECT() and SYS_ARCH_UNPROTECT().

On certain ports, the _PROTECT and _UNPROTECT calls just disable/enable
interrupts.  This leaves the possibility of a context switch happening in the
call to sys_sem_signal() if the OS primitive preempts the LwIP thread with the
waiting task.

The for loop already has support for handling a context switch each iteration
and saves the rolling sequence counter in order to detect a context switch
when SYS_ARCH_UNPROTECT() is called.

We can address the potential context switch during the call to
sys_sem_signal() by moving the line of last_select_cb_ctr = select_ct_ctr to
the top of the for loop body, thus ensure any context switch within the loop
body is detected.  Since we don't modify the select list within the loop body,
this is safe

SYS_ARCH_PROTECT(lev);
...
again:
  for (scb = select_cb_list; scb != NULL; scb = scb->next) {
    if (scb->sem_signalled == 0) {
      /* semaphore not signalled yet */
      int do_signal = 0;
      /* Test this select call for our socket */
      if (sock->rcvevent > 0) {
        if (scb->readset && FD_ISSET(s, scb->readset)) {
          do_signal = 1;
        }
      }
      if (sock->sendevent != 0) {
        if (!do_signal && scb->writeset && FD_ISSET(s, scb->writeset)) {
          do_signal = 1;
        }
      }
      if (sock->errevent != 0) {
        if (!do_signal && scb->exceptset && FD_ISSET(s, scb->exceptset)) {
          do_signal = 1;
        }
      }
      if (do_signal) {
        scb->sem_signalled = 1;
        /* Don't call SYS_ARCH_UNPROTECT() before signaling the semaphore, as
this might
           lead to the select thread taking itself off the list, invalidagin
the semaphore. */
        sys_sem_signal(&scb->sem);
      }
    }
    last_select_cb_ctr = select_cb_ctr;
    /* unlock interrupts with each step */
    SYS_ARCH_UNPROTECT(lev);
    /* this makes sure interrupt protection time is short */
    SYS_ARCH_PROTECT(lev);
    if (last_select_cb_ctr != select_cb_ctr) {
      /* someone has changed select_cb_list, restart at the beginning */
      goto again;
    }



    _______________________________________________________

File Attachments:


-------------------------------------------------------
Date: Mon 22 Sep 2014 02:18:05 PM GMT  Name: event-handle.diff  Size: 691B  
By: jcunningham
Diff moves last_select_cb_ctr to top of loop body
<http://savannah.nongnu.org/bugs/download.php?file_id=32167>

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?43278>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/