buglwIP - A Lightweight TCP/IP stack - Bugs: bug #48506, Possible crash when TCP poll...

 
 

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

bug #48506: Possible crash when TCP poll callback results in ERR_ABRT

Submitted by:  Ambroz Bizjak <abizjak>
Submitted on:  Wed 13 Jul 2016 11:10:21 PM UTC  
 
Category: TCPSeverity: 3 - Normal
Item Group: Crash ErrorStatus: Invalid
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: ClosedPlanned Release: None
lwIP version: git head

Tue 19 Jul 2016 07:09:54 AM UTC, comment #2:

You're correct on comment #1: tcp_active_pcbs_changed was introduced to allow returning ERR_ABRT.

As to the deceiving comments, you're free to provide constructive proposals for changes :-)

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Sat 16 Jul 2016 09:51:21 AM UTC, comment #1:

On second look the issue may not really exist due to the tcp_active_pcbs_changed check, thought the comments are deceiving in that case.

Ambroz Bizjak <abizjak>
Wed 13 Jul 2016 11:10:21 PM UTC, original submission:

While reading the code I found this issue: in tcp_slowtmr(), if TCP_EVENT_POLL results in ERR_ABRT, the "prev" PCB will have been deallocated, but "prev" is left unchanged and potentially used in the next iterations of the loop for removing PCBs from the active list. Note the comments acknowledge the possibility of "prev" retiring but its subsequent use is not considered.

} else {
/* get the 'next' element now and work with 'prev' below (in case of abort) */
prev = pcb;
pcb = pcb->next;

/* We check if we should poll the connection. */
++prev->polltmr;
if (prev->polltmr >= prev->pollinterval) {
prev->polltmr = 0;
LWIP_DEBUGF(TCP_DEBUG, ("tcp_slowtmr: polling application\n"));
tcp_active_pcbs_changed = 0;
TCP_EVENT_POLL(prev, err);
if (tcp_active_pcbs_changed) {
goto tcp_slowtmr_start;
}
/* if err == ERR_ABRT, 'prev' is already deallocated */
if (err == ERR_OK) {
tcp_output(prev);
}
}
}

Ambroz Bizjak <abizjak>

 

Attached Files
file #37866:  prev.patch added by abizjak (2KiB - text/x-patch - Possible fix - untested, no warranty :))

 

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 abizjak (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 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 19 Jul 2016 07:09:54 AM UTCgoldsimonStatusNone=>Invalid
      Assigned toNone=>goldsimon
      Open/ClosedOpen=>Closed
    Wed 13 Jul 2016 11:10:21 PM UTCabizjakAttached File-=>Added prev.patch, #37866

    Back to the top


    Powered by Savane 3.1-cleanup1