buglwIP - A Lightweight TCP/IP stack - Bugs: bug #26405, Prematurely released semaphore...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #26405: Prematurely released semaphore causes lwip_select() to crash

Submitted by:  Konstantin <konstua>
Submitted on:  Thu 30 Apr 2009 07:19:38 AM UTC  
 
Category: sockets/netconnSeverity: 3 - Normal
Item Group: Crash ErrorStatus: Fixed
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: ClosedPlanned Release: None
lwIP version: 1.3.0

Tue 05 May 2009 05:21:34 PM UTC, comment #5:

Checked in. Thanks for reporting!

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Tue 05 May 2009 07:59:43 AM UTC, comment #4:

Aaahh, timeout! That's the bit that was missing, I must have overread it...

Thank you so much for clarifying this, I'll check it in as soon as I find the time.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Tue 05 May 2009 07:21:00 AM UTC, comment #3:

Simon,
I don't think my port is a problem. It does indeed understand the initial state, here is the sys_sem_new() I am using (modelled after a sample provided with FreeRTOS):

However, I think the problem may be that sys_sem_wait_timeout() may time out (hence the name). (In my case, the timeout was quite short.)
I am positive that in my case, a condition occured when by the time tcpip_thread signals selectsem, the application thread was already waiting in sys_sem_wait(selectsem), and as soon as the semaphore was released, the OS woke up the application thread which destroyed select_cb.sem.

Konstantin <konstua>
Mon 04 May 2009 08:02:20 PM UTC, comment #2:

Reading the code, I don't get how it could go wrong:

tcpip_thread calls:
sys_sem_signal(selectsem);
sys_sem_signal(scb->sem);
while the application thread calls:
i = sys_sem_wait_timeout(select_cb.sem, msectimeout);
sys_sem_wait(selectsem);
sys_sem_signal(selectsem);
sys_sem_free(select_cb.sem);

The only problem I see here is that the port used by Konstantin could maybe ignore the int parameter passed to sys_sem_new and always initialize the semaphore with 1, in which case it would wait only for selectsem... but that would make problems elswhere, too.

Anyway, since it's good practice (or at least that's what I've learnt:) to use resources in the same order in every place, I don't have that strong an opinion against changing the order of the sys_sem_signal calls... Nevertheless, I'd like to understand the bug before making the change.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 30 Apr 2009 08:03:09 AM UTC, comment #1:

Looks like a bug to me, and a good fix. Well spotted.

Kieran Mansley <kieranm>
Project Administrator
Thu 30 Apr 2009 07:19:38 AM UTC, original submission:

While trying to get lwIP to work on a FreeRTOS port for Blackfin BF53x, I have observed elusive memory corruption. After a lot of debugging I have traced the cause to the following code:

(sockets.c :: event_callback())

It seems that, at least on my system, a situation occured where lwip_select() was waiting for selectsem in another thread, and as soon as it was released, lwip_select() could be exited, leaving scb (pointing at a local variable inside lwip_select()) an invalid reference.

Suggestion: swapping the two subsequent calls to sys_sem_signal() seems to resolve the problem:

Konstantin <konstua>

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by kieranm (Posted a comment)
  • -unavailable- added by konstua (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 05 May 2009 05:21:34 PM UTCgoldsimonStatusIn Progress=>Fixed
      Open/ClosedOpen=>Closed
    Tue 05 May 2009 07:59:43 AM UTCgoldsimonStatusNone=>In Progress
      Assigned toNone=>goldsimon
    Thu 30 Apr 2009 08:03:09 AM UTCkieranmPlanned Release=>1.3.1

    Back to the top


    Powered by Savane 3.1-cleanup1