buglwIP - A Lightweight TCP/IP stack - Bugs: bug #35574, Lockup in httpd (the older...

 
 

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

bug #35574: Lockup in httpd (the older non-SSI/CGI httpd)

Submitter:  Bill Auerbach <billauerbach>
Submitted:  Mon 20 Feb 2012 11:10:47 PM UTC
   
 
Category:  Contrib Severity:  3 - Normal
Item Group:  Crash Error Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  Other

Jump to the original submission

Wed 22 Feb 2012 07:57:16 PM UTC, comment #8: 


> http_send_data_nonssi is structured the same way as it is in
> mine which I guess a 1.3.2 version of httpd for httpserver_raw.


> Looks like tcp_write can be called (whatever the circumstance)
> with len == 0 after the if/else. For me hs->len was 0 and
> hs->file was NULL.


http_send_data_nonssi() should only be called with hs->file==NULL and len==0 any more, since this is checked in http_send() before:

  if (hs->left == 0) {
    if (!http_check_eof(pcb, hs)) {
      return 0;
    }
  }

.. where http_check_eof() returns 0 if hs->file == NULL.
Nonetheless, I added a check for len==0 in http_write() to prevent triggering the error.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 21 Feb 2012 08:35:21 PM UTC, comment #7: 

The issue was arg==NULL locking in the LWIP_ERROR.  It happened that len was 0 and my opinion the pointer isn't used with len==0 so maybe LWIP_ERROR shouldn't fail on arg==NULL.

I'm not sure what happens in tcp_write with len==0 and arg!=NULL.

I agree with the NULL pointer check being done and that httpd shouldn't call tcp_write with arg==NULL.

You're suggesting (I think) that calling tcp_write with len==0 is an API error too which isn't now checked.  Maybe it should to catch an application doing something unusual (at best).

Bill Auerbach <billauerbach>
Tue 21 Feb 2012 08:28:17 PM UTC, comment #6: 

This sounds to me like a fault in the httpd.  I can't see what benefit it would bring to tolerate len==0.  I think it's better to flag the error so the app can be fixed.

Kieran Mansley <kieranm>
Group Member
Tue 21 Feb 2012 04:53:46 PM UTC, comment #5: 


> This could be something that happens only in the 1.3.2 branch.


There's no such thing as a 1.3.2 branch. I'd apply the change to both master and  the 1.4.1 branch.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 21 Feb 2012 03:44:02 PM UTC, comment #4: 

I agree and think that both changes are worthwhile.  This could be something that happens only in the 1.3.2 branch.  The WEB interface is not a part of our application which is why I don't keep it current.  I do use 1.4.1 elsewhere.  I use it as a system test since I can check the IP address and send a lot of data easily to a browser.

Bill Auerbach <billauerbach>
Tue 21 Feb 2012 02:16:08 PM UTC, comment #3: 


> Why not just return from tcp_write if len == 0


I think the reason for that is that at least until 1.3.2, tcp_write() and tcp_send_ctrl() were direct mappers to tcp_enqueue() (with data+len or tcp header flags). The way it is now, I agree with you that we could just return instead of asserting when len is 0.

Still I think that http_send should not call into tcp_write with len==0.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 21 Feb 2012 01:27:04 PM UTC, comment #2: 

http_send_data_nonssi is structured the same way as it is in mine which I guess a 1.3.2 version of httpd for httpserver_raw.

Looks like tcp_write can be called (whatever the circumstance) with len == 0 after the if/else.  For me hs->len was 0 and hs->file was NULL.

Why not just return from tcp_write if len == 0.  However it happens that it might get called with len == 0, there is nothing to do (and technically there really is no error).

Bill Auerbach <billauerbach>
Tue 21 Feb 2012 11:53:54 AM UTC, comment #1: 

Hmm, I don't know which version you are using, but the one included with 1.4.0 already had SSI and CGI support. Plus the httpd shipped with contrib-1.4.1 will be the current master (it has no IPv6 dependencies but some bugs fixed compared to 1.4.0).

Now I would have imagined that the current master httpd also has this bug, or doesn't it?

Simon Goldschmidt <goldsimon>
Group administrator
Mon 20 Feb 2012 11:10:47 PM UTC, original submission:  

This is for the older version httpd that may be included with 1.4.1.  This may also be useful if it's in use by those not using the latest.

Bug: send_data under what appears to be a time dependent condition will call tcp_write with arg=NULL and len=0.  This causes an LWIP_ERROR halt error (programmer violates API).  One could argue that with len=0 it never access the NULL pointer and tcp_write could just return if len==0.  OTOH send_data could include a test for len==0 and simply return and not call tcp_write. In any case, this caused my program to hang (fortunately I was in a debugger and knew why).

Bill Auerbach <billauerbach>

 

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

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by kieranm (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by billauerbach (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 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2012-02-22 goldsimon StatusConfirmed Fixed
        Assigned toNone goldsimon
        Open/ClosedOpen Closed
        Planned Release 1.4.1
    2012-02-21 goldsimon StatusNeed Info Confirmed
    2012-02-21 goldsimon StatusNone Need Info

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code