Re: [PATCH 3.16-4.14] tcp: Clear sk_send_head after purging the write queue

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Tue, Aug 13, 2019 at 12:53:17PM +0100, Ben Hutchings wrote:
> Denis Andzakovic discovered a potential use-after-free in older kernel
> versions, using syzkaller.  tcp_write_queue_purge() frees all skbs in
> the TCP write queue and can leave sk->sk_send_head pointing to freed
> memory.  tcp_disconnect() clears that pointer after calling
> tcp_write_queue_purge(), but tcp_connect() does not.  It is
> (surprisingly) possible to add to the write queue between
> disconnection and reconnection, so this needs to be done in both
> places.
> 
> This bug was introduced by backports of commit 7f582b248d0a ("tcp:
> purge write queue in tcp_connect_init()") and does not exist upstream
> because of earlier changes in commit 75c119afe14f ("tcp: implement
> rb-tree based retransmit queue").  The latter is a major change that's
> not suitable for stable.
> 
> Reported-by: Denis Andzakovic <denis.andzakovic@pulsesecurity.co.nz>
> Bisected-by: Salvatore Bonaccorso <carnil@debian.org>
> Fixes: 7f582b248d0a ("tcp: purge write queue in tcp_connect_init()")
> Cc: <stable@vger.kernel.org> # before 4.15
> Cc: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
>  include/net/tcp.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index fed2a78fb8cb..f9b985d4d779 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -1517,6 +1517,8 @@ struct tcp_fastopen_context {
>  	struct rcu_head		rcu;
>  };
>  
> +static inline void tcp_init_send_head(struct sock *sk);
> +
>  /* write queue abstraction */
>  static inline void tcp_write_queue_purge(struct sock *sk)
>  {
> @@ -1524,6 +1526,7 @@ static inline void tcp_write_queue_purge(struct sock *sk)
>  
>  	while ((skb = __skb_dequeue(&sk->sk_write_queue)) != NULL)
>  		sk_wmem_free_skb(sk, skb);
> +	tcp_init_send_head(sk);
>  	sk_mem_reclaim(sk);
>  	tcp_clear_all_retrans_hints(tcp_sk(sk));
>  	inet_csk(sk)->icsk_backoff = 0;


Nice catch, thanks for this.  Now queued up everywhere.

greg k-h