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@xxxxxxxxxxxxxxxxxxx>
> Bisected-by: Salvatore Bonaccorso <carnil@xxxxxxxxxx>
> Fixes: 7f582b248d0a ("tcp: purge write queue in tcp_connect_init()")
> Cc: <stable@xxxxxxxxxxxxxxx> # before 4.15
> Cc: Eric Dumazet <edumazet@xxxxxxxxxx>
> Signed-off-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
> ---
> 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
