[bug #27791] null pointer dereference (pcb->unacked == 0) in tcp_in.c:tcp_process

[Christophe Combelles-4]

URL:
  <http://savannah.nongnu.org/bugs/?27791>

                 Summary: null pointer dereference (pcb->unacked == 0) in
tcp_in.c:tcp_process
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: gdupontavice
            Submitted on: ven 23 oct 2009 14:36:59 GMT
                Category: TCP
                Severity: 3 - Normal
              Item Group: Crash Error
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release:
            lwIP version: CVS Head

    _______________________________________________________

Details:

the following problems happens on my platform

lwip tree = CVS HEAD 20091023

during a TCP connect, it happens that I got null pointer dereference
(pcb->unacked = 0) in tcp_in.c :559 (in fact line 560) in function
tcp_process()

559:    if ((flags & TCP_ACK) && (flags & TCP_SYN)
560:        && ackno == ntohl(pcb->unacked->tcphdr->seqno) + 1) {


as we can see, in tcp_process() we assume that pcb->unacked != 0

the TCP SYN packet is sent in tcp_out.c (thanks to a call to
tcp_output_segment(seg, pcb); line 586)
but the problem is that pcb->unacked is set after the packet is actually sent
(it is set in tcp_out.c line 596)

in my environment, it happens from time to time that the TCP SYN ACK answer
arrives before the pcb->unacked is set to something not null .... hence, the
null pointer dereference.

see backtrace below :


thread 1 doing the tcp connect :
--------------------------------
#0  0x84aa7528 in tcp_output (pcb=0x84dc8aac) at
head/lwip/src/core/tcp_out.c:589
589         snd_nxt = ntohl(seg->tcphdr->seqno) + TCP_TCPLEN(seg);
(gdb) bt
#0  0x84aa7528 in tcp_output (pcb=0x84dc8aac) at
head/lwip/src/core/tcp_out.c:589
#1  0x84aa3c06 in tcp_connect (pcb=0x84dc8aac, ipaddr=0x89430670, port=80,
connected=0x84aaa7a0 <do_connected>) at head/lwip/src/core/tcp.c:563
#2  0x84aab53e in do_connect (msg=0x89430654) at
head/lwip/src/api/api_msg.c:812
#3  0x84aa8848 in tcpip_thread (arg=<value optimized out>) at
head/lwip/src/api/tcpip.c:269

as we can see, the packet has been sent, but pcb->unacked has not yet been
set (it is still null here, as we have not reached tcp_out.c:596)


thread 2 : ethernet driver callback:
--------------------------------------------------
#0  0x84aa6a8c in tcp_input (p=<value optimized out>, inp=<value optimized
out>) at head/lwip/src/core/tcp_in.c:559
559         if ((flags & TCP_ACK) && (flags & TCP_SYN)
(gdb) bt
#0  0x84aa6a8c in tcp_input (p=<value optimized out>, inp=<value optimized
out>) at head/lwip/src/core/tcp_in.c:559
#1  0x84a9c608 in ip_input (p=0x84dfbb9c, inp=0x84e1bd20) at
head/lwip/src/core/ipv4/ip.c:427


(gdb) print pcb
$1 = (struct tcp_pcb *) 0x84dc8aac
(gdb) print pcb->unacked
$2 = (struct tcp_seg *) 0x0



here we are processing the TCP SYN ACK answer, & we are already considering
that pcb->unacked is != NULL


=> there is something wrong here, I would rather suggest that




    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27791>

_______________________________________________
  Message posté via/par Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Christophe Combelles-4]

Follow-up Comment #1, bug #27791 (project lwip):

It sounds like you're saying that the response to the SYN is processed before
you've finished sending the SYN itself.  You should not be able to process a
received packet while you're still active in the stack with a transmitted
packet.  

Perhaps you can help by describing how your driver interfaces to the stack
(how does it pass received packets for processing) and how your application
interfaces to the stack.  What environment are you running in (e.g. do you
have an OS?)

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27791>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Christophe Combelles-4]

Follow-up Comment #2, bug #27791 (project lwip):

Hi,

the TCP SYN packet is sent in

tcp_output_segment(seg, pcb); (line 588)


588:    tcp_output_segment(seg, pcb);
589:    snd_nxt = ntohl(seg->tcphdr->seqno) + TCP_TCPLEN(seg);
590:    if (TCP_SEQ_LT(pcb->snd_nxt, snd_nxt)) {
591:      pcb->snd_nxt = snd_nxt;
592:    }
593:    /* put segment on unacknowledged list if length > 0 */
594:    if (TCP_TCPLEN(seg) > 0) {
595:      seg->next = NULL;
596:      /* unacked list is empty? */
597:      if (pcb->unacked == NULL) {
598:        pcb->unacked = seg;
599:        useg = seg;



see stack trace below:
# 0 tcp_output_segment
# 1 ip_output
# 2 ip_output_if
# 3 netif->output


if you look at my stack trace, I have reached line 589 (the packet is sent),

the pb comes from the fact that pcb->unacked is set line 598.

=> if the TCP SYN ACK comes back before I reach line 598, then I have a pb.

 


    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27791>

_______________________________________________
  Message posté via/par Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Christophe Combelles-4]

Follow-up Comment #3, bug #27791 (project lwip):

Yes, but the received packet should not interrupt tcp_output_segment.

Perhaps you can help by describing how your driver interfaces to the stack
(how does it pass received packets for processing) and how your application
interfaces to the stack. What environment are you running in (e.g. do you have
an OS?)

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27791>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Christophe Combelles-4]

Follow-up Comment #4, bug #27791 (project lwip):

please find more details on my environment :

platform ST7109 chipset (SH4 core)
I use OS21 operating system (ST operating system, supporting thread,
semaphore, mailbox, ...)
ethernet chipset : STMAC (supporting 100 Mb/s FD, DMA ...)

my application uses sockets.

ethernet driver :
reception is done thanks to ISR.
ISR wakes up a dedicated task that calls ip_input()

transmission is synchronous. when exiting netif->output function, the packet
is really sent on the network.

in my current config, the asynchronous reception task can be woken up in the
middle of a packet transmission.

however, even if I add a lock that forbids a packet reception while a packet
is transmitted, it won't solve the problem :

in my scenario, the packet reception does not happen when i am in
tcp_output_segment. it happens just after.

maybe a workaround could be to put a higher priority on tcpip_thread task, to
ensure that the asynchronous reception task won't disturb it ?

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27791>

_______________________________________________
  Message posté via/par Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Christophe Combelles-4]

Follow-up Comment #5, bug #27791 (project lwip):

In lwIP, there is no such thing as asynchronous reception (it's simply not
supported).

What you have to do is: if you are using the tcpip_thread, your netif->input
must be set to tcpip_input, not ip_input or ethernet_input (pass tcpip_input
to netif_add). This ensures RX packets are passed to an mbox and are not
processed until tcp_output returns.

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27791>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Christophe Combelles-4]

Update of bug #27791 (project lwip):

                  Status:                    None => Need Info              


    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27791>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Christophe Combelles-4]

Follow-up Comment #6, bug #27791 (project lwip):

hi Simon,

understood. I will modify my code & use tcpip_input instead of ip_input so
that every Rx packet will be queued in tcpip_thread.

definitely not a bug, you can close it.
thanks for the help

regards,

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27791>

_______________________________________________
  Message posté via/par Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel

[Christophe Combelles-4]

Update of bug #27791 (project lwip):

                  Status:               Need Info => Invalid                
             Open/Closed:                    Open => Closed                

    _______________________________________________________

Follow-up Comment #7:

Thanks for clarifying this.

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?27791>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.nongnu.org/



_______________________________________________
lwip-devel mailing list
lwip-devel@...
http://lists.nongnu.org/mailman/listinfo/lwip-devel