Re: [lwip-devel] Bad memory ref in tcp_input() after tcp_close()

[Per Ekman]

Ping? I would have entered this as a bug but I could not
get the Savannah login to work.

Per Ekman <address@hidden> writes:

> 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)
>
> I'm testing the following fix. I'd appreciate feedback if this is the
> way to go or not.
>
> 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