[lwip-devel] [bug #41318] Bad memory ref in tcp_input() after tcp_close()

[Per Ekman]

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

                 Summary: Bad memory ref in tcp_input() after tcp_close()
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: perekman
            Submitted on: Thu 23 Jan 2014 10:17:28 AM 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: 1.4.1

    _______________________________________________________

Details:

Hello,

I've observed the following with the rawapi in lwip 1.4.1 :

- tcp_input():356 calls the application receive callback on a pcb
which is in CLOSE_WAIT. 

- The application receive callback calls tcp_close()

- tcp_close_shutdown() sees that the pcb is in CLOSE_WAIT and that
  pcb->rcv_wnd != TCP_WND which causes it to free the pcb
  and return ERR_OK

- The application receive callback returns ERR_OK to tcp_input()
  which proceeds to dereference the freed pcb and passes it to
  tcp_output() (tcp_input():386)

It seems like a bug in tcp_close(). It frees the pcb if the pcb is in
CLOSE_WAIT since no RST needs to be sent but this breaks if
tcp_close() is called from the RX callback since the lwip code
invoking the RX callback assumes that the pcb is still valid after the
RX callback has returned (and the RX callback has no way of knowing
that the pcb was freed).

The following fix solved the immediate issue for me. 

Sincerely
Per Ekman
H&D Wireless AB


>From 147c37d9d43a496669092c1fc2febe7aaf5fe3df Mon Sep 17 00:00:00 2001
From: Per Ekman <address@hidden>
Date: Fri, 5 Jul 2013 12:52:32 +0200
Subject: [PATCH 1/1] Attempt to fix invalid pcb free in tcp_close().

---
 src/core/tcp.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/core/tcp.c b/src/core/tcp.c
index 8690cd2..b4bd896 100644
--- a/src/core/tcp.c
+++ b/src/core/tcp.c
@@ -185,15 +185,14 @@ tcp_close_shutdown(struct tcp_pcb *pcb, u8_t 
rst_on_unacked_data)
       tcp_rst(pcb->snd_nxt, pcb->rcv_nxt, &pcb->local_ip, &pcb->remote_ip,
                pcb->local_port, pcb->remote_port, PCB_ISIPV6(pcb));
 
+      if (pcb->state == CLOSE_WAIT)
+              return ERR_OK;
       tcp_pcb_purge(pcb);
       TCP_RMV_ACTIVE(pcb);
       if (pcb->state == ESTABLISHED) {
         /* move to TIME_WAIT since we close actively */
         pcb->state = TIME_WAIT;
         TCP_REG(&tcp_tw_pcbs, pcb);
-      } else {
-        /* CLOSE_WAIT: deallocate the pcb since we already sent a RST for it
*/
-        memp_free(MEMP_TCP_PCB, pcb);
       }
       return ERR_OK;
     }
@@ -913,6 +912,12 @@ tcp_slowtmr_start:
         }
       }
     }
+    if (pcb->state == CLOSE_WAIT) {
+      if (pcb->flags & TF_RXCLOSED) {
+          ++pcb_remove;
+          LWIP_DEBUGF(TCP_DEBUG, ("tcp_slowtmr: removing pcb in 
CLOSE_WAIT\n"));
+      }
+    }
     /* Check if this PCB has stayed too long in FIN-WAIT-2 */
     if (pcb->state == FIN_WAIT_2) {
       /* If this PCB is in FIN_WAIT_2 because of SHUT_WR don't let it time 
out. */
-- 
1.7.0.4





    _______________________________________________________

Reply to this item at:

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

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