buglwIP - A Lightweight TCP/IP stack - Bugs: bug #25695, Segmentation fault in...

 
 

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

bug #25695: Segmentation fault in do_writemore(struct netconn *conn)

Submitter:  Koen Deforche <kdeforche>
Submitted:  Tue 24 Feb 2009 04:11:38 PM UTC
   
 
Category:  TCP Severity:  3 - Normal
Item Group:  Crash Error Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  1.3.0

Jump to the original submission

Sat 18 Apr 2009 05:49:07 PM UTC, comment #12: 

Simon, having a deeper look on the code and your explanation, I have to admit you're right and the only clean solution is setting state to NONE like Koen initially suggested. I was previously looking at the wrong line in the code because of different versions.

This is fixed now.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 17 Apr 2009 10:34:15 AM UTC, comment #11: 

OK, done the change to the unix port for sys_sem_wait() -> sys_arch_sem_wait()

It's not clear whether that completely resolves the problem, so I'm going to leave this bug open for the time being.

Kieran Mansley <kieranm>
Group Member
Fri 17 Apr 2009 10:22:38 AM UTC, comment #10: 

Planned release -> 1.3.1

We should at least fix the sem handling of the unix port until then. Simon (Braun)'s problem is another issue, though. I'll try to think about it :-)

Simon Goldschmidt <goldsimon>
Group administrator
Thu 05 Mar 2009 07:49:21 AM UTC, comment #9: 

OK, now I get it: I think you are talking about a different scenario. The one Koen reported does not involve API_EVENT but is a problem with loopif calling tcpip_input from tcpip_thread.

But I think you are right that the event_callback() in sockets.c can also produce this problem. That has to be protected, but using state NETCONN_NONE is not a good idea. We would have to protect that with a different check.

Simon Goldschmidt <goldsimon>
Group administrator
Thu 05 Mar 2009 07:09:31 AM UTC, comment #8: 

Hi Simon, I'm using the FreeRTOS port, which does call timers from within sys_sem_wait.  Any port that can call poll_tcp from within API_EVENT can suffer this problem, but I'm not familiar with other ports so I don't know how general the problem is.  The unexplored consequences I was talking about, are what happens when the timers are not running while waiting for a semaphore, in this case socksem or selectsem.  You are far more familiar with lwip than I so I have to defer to your judgement of how 'clean' the solution is.

Simon Braun <sjb>
Thu 05 Mar 2009 05:57:10 AM UTC, comment #7: 

Setting netconn->state to _NONE would mean we would have a general problem here, not a port-only problem, which 'm not convinced of. Also, I still don't think that would be a clean solution.

Did you experience this on the unix port? And which "unexplored consequences" do you mean? This is only an implementation of an mbox, other ports don't call the timeouts in that case.

Simon Goldschmidt <goldsimon>
Group administrator
Thu 05 Mar 2009 01:35:56 AM UTC, comment #6: 

Hi guys, I just diagnosed exactly the same problem here at InnerRange, and came up with the same solution (setting netconn->state to NETCONN_NONE before the call to API_EVENT).  I have tested this solution overnight and the problem has not recurred (was occuring within 1 hour).  I think this is the correct fix because when write_msg is set to NULL, the netconn is in an intermediate, invalid state, until the 'finished' code is executed.  API_EVENT must not be called while the netconn is invalid, so the simple solution is to set the state to NETCONN_NONE early.  I can see how using sys_arch_sem_wait fixes the problem but it will have other unexplored consequences.

Simon Braun <sjb>
Tue 03 Mar 2009 03:32:38 PM UTC, comment #5: 

Hey all. I understand this change and it makes sense at least from the point of view that we saw this problem on the UNIX platform, but not on Win32. The problem is unfortunately not easily reproducible, but at least with this the sys_arch_sem_wait() change we have not seen it occur again.

Koen Deforche <kdeforche>
Tue 03 Mar 2009 01:47:09 PM UTC, comment #4: 

Yes, looks sensible to me.  Perhaps Koen Deforche could try this out, and if it works we can commit the change to CVS.

Kieran Mansley <kieranm>
Group Member
Sun 01 Mar 2009 05:52:38 PM UTC, comment #3: 

Kieran, since you seemed to be something like the unix port maintainer, do you agree with changing the sys_sem_wait calls to sys_arch_sem_wait?

Simon Goldschmidt <goldsimon>
Group administrator
Sun 01 Mar 2009 10:55:43 AM UTC, comment #2: 

I don't think the fix is correct: resetting the connection state should not be done in that way.

The unix version of sys_mbox_trypost calling sys_sem_wait seems wrong to me, too.

I'd just change that to sys_arch_sem_wait... (and also the 2 other occurences of sys_sem_wait in that file) - any objections by anyone using the unix port?

Simon Goldschmidt <goldsimon>
Group administrator
Tue 24 Feb 2009 08:23:10 PM UTC, comment #1: 

Rhe fix you propose seems to be good at the first sight.

But perhaps I'm wrong, I think the problem is more in the unix port : in contrib\ports\unix\sys_arch.c, the sys_mbox_trypost implementation use a call to sys_sem_wait, which cause the "switching" to tcpip_tcp_timer, causing the reentrance problem.

Perhaps replacing sys_sem_wait by sys_arch_sem_wait in sys_arch.c good fix this problem...

Frédéric Bernon <fbernon>
Group Member
Tue 24 Feb 2009 04:11:38 PM UTC, original submission:  

Hey all,

Very occasionally we hit a segmentation fault in do_writemore(). A back-trace shows the following stack trace:

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x00000004
[Switching to process 54758 thread 0x3203]
0x000dfbb6 in do_writemore (conn=0x281394) at lwip/lwip/src/api/api_msg.c:1001
1001   dataptr = (u8_t*)conn->write_msg->msg.w.dataptr + conn->write_offset;
(gdb) bt
#0  0x000dfbb6 in do_writemore (conn=0x281394) at lwip/lwip/src/api/api_msg.c:1001
#1  0x000de8be in poll_tcp (arg=0x281394, pcb=0x27fd3c) at lwip/lwip/src/api/api_msg.c:240
#2  0x000d2d6f in tcp_slowtmr () at lwip/lwip/src/core/tcp.c:746
#3  0x000d1aa2 in tcp_tmr () at lwip/lwip/src/core/tcp.c:98
#4  0x000e01d9 in tcpip_tcp_timer (arg=0x0) at lwip/lwip/src/api/tcpip.c:82
#5  0x000d16a2 in sys_sem_wait (sem=0x21bbf30) at lwip/lwip/src/core/sys.c:158
#6  0x000ed9d7 in sys_mbox_trypost (mbox=0x100ee00, msg=0x281d8c) at lwip/contrib/ports/unix/sys_arch.c:222
#7  0x000e0529 in tcpip_input (p=0x184e84, inp=0x184b00) at lwip/lwip/src/api/tcpip.c:335
#8  0x000e3fb0 in loopif_output (netif=0x184b00, p=0x184e2c, ipaddr=0x27fd40) at lwip/lwip/src/netif/loopif.c:160
#9  0x000dc4f3 in ip_output_if (p=0x184e2c, src=0x27fd3c, dest=0x27fd40, ttl=255 '?', tos=0 '\0', proto=6 '\006', netif=0x184b00) at lwip/lwip/src/core/ipv4/ip.c:536
#10 0x000dc571 in ip_output (p=0x184e2c, src=0x27fd3c, dest=0x27fd40, ttl=255 '?', tos=0 '\0', proto=6 '\006') at lwip/lwip/src/core/ipv4/ip.c:566
#11 0x000d7c8b in tcp_output_segment (seg=0x280b1c, pcb=0x27fd3c) at lwip/lwip/src/core/tcp_out.c:672
#12 0x000d7863 in tcp_output (pcb=0x27fd3c) at lwip/lwip/src/core/tcp_out.c:549
#13 0x000dfd43 in do_writemore (conn=0x281394) at lwip/lwip/src/api/api_msg.c:1029
#14 0x000dfe49 in do_write (msg=0xb079c350) at lwip/lwip/src/api/api_msg.c:1097
#15 0x000e0409 in tcpip_thread (arg=0x0) at lwip/lwip/src/api/tcpip.c:269
#16 0x9029a095 in _pthread_start ()
#17 0x90299f52 in thread_start ()
Current language:  auto; currently c

The problem is: conn->write_msg = NULL. It is reset in stack frame #13, before a call to tcp_output_nagle(). That this occurs rarely is because the tcp slow timer needs to fire at exactly the right moment to trigger this wrong behavior ?

A solution could be the patch below, which resets the conn->state before calling tcp_output_nagle:

--- a/lwip/lwip/src/api/api_msg.c
+++ b/lwip/lwip/src/api/api_msg.c
@@ -1025,6 +1025,7 @@ do_writemore(struct netconn *conn)
       write_finished = 1;
       conn->write_msg = NULL;
       conn->write_offset = 0;
+      conn->state = NETCONN_NONE; /* must before tcp_output_nagle() */
     }
     err = tcp_output_nagle(conn->pcb.tcp);
     conn->err = err;

Koen Deforche <kdeforche>

 

(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 sjb (Posted a comment)
  • -email is unavailable- added by kieranm (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by fbernon (Posted a comment)
  • -email is unavailable- added by kdeforche (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 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2009-04-18 goldsimon StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2009-04-17 goldsimon StatusNone In Progress
        Assigned toNone goldsimon
        Planned Release 1.3.1

    Back to the top

    Powered by Savane 3.13-aa77.
    Corresponding source code