From patchwork Wed Feb 13 21:09:59 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Zoltan Kiss X-Patchwork-Id: 220251 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 4F8832C0085 for ; Thu, 14 Feb 2013 08:10:16 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760649Ab3BMVKL (ORCPT ); Wed, 13 Feb 2013 16:10:11 -0500 Received: from smtp.citrix.com ([66.165.176.89]:5651 "EHLO SMTP.CITRIX.COM" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752010Ab3BMVKK (ORCPT ); Wed, 13 Feb 2013 16:10:10 -0500 X-IronPort-AV: E=Sophos;i="4.84,658,1355097600"; d="scan'208";a="7404840" Received: from accessns.citrite.net (HELO FTLPEX01CL02.citrite.net) ([10.9.154.239]) by FTLPIPO01.CITRIX.COM with ESMTP/TLS/AES128-SHA; 13 Feb 2013 21:10:09 +0000 Received: from [10.80.2.133] (10.80.2.133) by FTLPEX01CL02.citrite.net (10.13.107.79) with Microsoft SMTP Server id 14.2.318.1; Wed, 13 Feb 2013 16:10:09 -0500 Message-ID: <511C0127.1040604@citrix.com> Date: Wed, 13 Feb 2013 21:09:59 +0000 From: Zoltan Kiss User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130106 Thunderbird/17.0.2 MIME-Version: 1.0 To: CC: Frediano Ziglio Subject: tcp_use_frto crashes on empty tcp_write_queue X-Originating-IP: [10.80.2.133] Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Hi, I see the following WARN and then crash on 2.6.32.12: <4>WARNING: at net/ipv4/tcp_timer.c:293 tcp_retransmit_timer+0x5dd/0x630() ... <1>BUG: unable to handle kernel NULL pointer dereference at (null) <1>IP: [] tcp_use_frto+0x45/0x90 ... <0>Call Trace: <4> [] ? tcp_retransmit_timer+0xd9/0x630 <4> [] ? __wake_up_common+0x48/0x70 <4> [] ? tcp_write_timer+0xe0/0x1a0 <4> [] ? run_timer_softirq+0x151/0x200 <4> [] ? maybe_schedule_tx_action+0x39/0x40 <4> [] ? tcp_write_timer+0x0/0x1a0 <4> [] ? __do_softirq+0xba/0x180 <4> [] ? move_native_irq+0x47/0x50 ... I've checked the code, tcp_use_frto() crashes because skb = tcp_write_queue_head(sk); returns a NULL, as the queue is empty, and in the next line: if (tcp_skb_is_last(sk, skb)) Credit goes to Frediano for the patch. Regards, Zoli --- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ===> static inline bool tcp_skb_is_last(const struct sock *sk, const struct sk_buff *skb) { return skb_queue_is_last(&sk->sk_write_queue, skb); } ===> static inline bool skb_queue_is_last(const struct sk_buff_head *list, const struct sk_buff *skb) { return (skb->next == (struct sk_buff *) list); } That skb->next cause the NULL pointer dereference. I've checked this in upstream, and it seems this would fail in the same way. Wouldn't it be more reasonable to return from tcp_retransmit_timer() instead of just signing a WARN? Something like this: diff -r 7a748d2cb9f1 -r bb8257f0730a net/ipv4/tcp_timer.c --- a/net/ipv4/tcp_timer.c Wed Feb 13 15:02:50 2013 +0000 +++ b/net/ipv4/tcp_timer.c Wed Feb 13 15:03:18 2013 +0000 @@ -287,11 +287,9 @@ void tcp_retransmit_timer(struct sock *s struct tcp_sock *tp = tcp_sk(sk); struct inet_connection_sock *icsk = inet_csk(sk); - if (!tp->packets_out) + if (!tp->packets_out || tcp_write_queue_empty(sk)) goto out; - WARN_ON(tcp_write_queue_empty(sk)); - if (!tp->snd_wnd && !sock_flag(sk, SOCK_DEAD) && !((1 << sk->sk_state) & (TCPF_SYN_SENT | TCPF_SYN_RECV))) { /* Receiver dastardly shrinks window. Our retransmits