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

Submitter:  Ambroz Bizjak <abizjak>
Submitted:  Wed 13 Jul 2016 11:10:21 PM UTC
   
 
Category:  TCP Severity:  3 - Normal
Item Group:  Crash Error Status:  Invalid
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned 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>
Group administrator
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>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

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
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by abizjak (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-07-19 goldsimon StatusNone Invalid
        Assigned toNone goldsimon
        Open/ClosedOpen Closed
    2016-07-13 abizjak Attached File- Added prev.patch, #37866

    Back to the top

    Powered by Savane 3.13-bb6a.
    Corresponding source code