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)

Submitted by:  Bill Auerbach <billauerbach>
Submitted on:  Mon 20 Feb 2012 11:10:47 PM UTC  
 
Category: ContribSeverity: 3 - Normal
Item Group: Crash ErrorStatus: Fixed
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: ClosedPlanned Release: None
lwIP version: Other

(Jump to the original submission 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>
Project AdministratorIn charge of this item.
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>
Project Administrator
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>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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>

 

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

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 22 Feb 2012 07:57:16 PM UTCgoldsimonStatusConfirmed=>Fixed
      Assigned toNone=>goldsimon
      Open/ClosedOpen=>Closed
      Planned Release=>1.4.1
    Tue 21 Feb 2012 04:53:46 PM UTCgoldsimonStatusNeed Info=>Confirmed
    Tue 21 Feb 2012 11:53:54 AM UTCgoldsimonStatusNone=>Need Info

    Back to the top


    Powered by Savane 3.1-cleanup1