From patchwork Wed May 4 20:02:26 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 94124 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 B8C351007DB for ; Thu, 5 May 2011 06:02:33 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755532Ab1EDUCb (ORCPT ); Wed, 4 May 2011 16:02:31 -0400 Received: from mail-wy0-f174.google.com ([74.125.82.174]:45205 "EHLO mail-wy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755146Ab1EDUCa (ORCPT ); Wed, 4 May 2011 16:02:30 -0400 Received: by wya21 with SMTP id 21so1119489wya.19 for ; Wed, 04 May 2011 13:02:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:subject:from:to:cc:in-reply-to:references :content-type:date:message-id:mime-version:x-mailer :content-transfer-encoding; bh=g8ucwjwSSoYKG46m1sLnIe9rsrornmUbW5M7U43i7Nw=; b=NYw+9POQ2lfZxXq1VGdYmQDX5TEciMd2yLl48l3yGDtpfNSOzCOAKqfezW2scZ/9t8 kpe0D14bN2fYd85DquRxGHIksQjXDhiwUAbhiL8fgJLGKCMo06OtuKcLuPGFN2M/Y7ek Rh9rVEv4MJ8eikDvPAHst+Rl1W2qgQeI3e1tA= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=subject:from:to:cc:in-reply-to:references:content-type:date :message-id:mime-version:x-mailer:content-transfer-encoding; b=Qvwqbr6GLFMvvVYTQGYOJvlIZxKW38qFUTHrNfj5JsqCkez3KqEVf9KvYZMaoF7xMv TYVbUkQcxRD2jPNqw9n3aPE/DOzpfUGdwiAbcgtXZHF8emd4koisrW1yfBgu+rpf12Tq C10g40q1feHXwJz+6/BZGtqezjWq+Z9QBEwDg= Received: by 10.227.12.18 with SMTP id v18mr1600563wbv.72.1304539349397; Wed, 04 May 2011 13:02:29 -0700 (PDT) Received: from [10.150.51.210] (gw0.net.jmsp.net [212.23.165.14]) by mx.google.com with ESMTPS id x13sm425670wby.42.2011.05.04.13.02.27 (version=SSLv3 cipher=OTHER); Wed, 04 May 2011 13:02:28 -0700 (PDT) Subject: Re: 2.6.38.2, kernel panic, probably related to framentation handling From: Eric Dumazet To: Denys Fedoryshchenko , David Miller Cc: netdev@vger.kernel.org In-Reply-To: <1304532674.32152.16.camel@edumazet-laptop> References: <1304528581.32152.15.camel@edumazet-laptop> <1304532674.32152.16.camel@edumazet-laptop> Date: Wed, 04 May 2011 22:02:26 +0200 Message-ID: <1304539346.32152.81.camel@edumazet-laptop> Mime-Version: 1.0 X-Mailer: Evolution 2.30.3 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Le mercredi 04 mai 2011 à 20:11 +0200, Eric Dumazet a écrit : > Le mercredi 04 mai 2011 à 19:03 +0200, Eric Dumazet a écrit : > > > Hi Denys > > > > Is it reproductible, and possibly on latest kernel ? > > > > We fixed some bugs lately (assuming you also use a bridge ?) > > > > Could you send the disassembled code on your kernel of icmp_send() ? > > Oh well, I think I found the problem, I am working on a patch and send > it shortly. > > Thanks > I believe bug is one year old (2.6.35), please try following patch. Thanks ! [PATCH] net: ip_expire() must revalidate route Commit 4a94445c9a5c (net: Use ip_route_input_noref() in input path) added a bug in IP defragmentation handling, in case timeout is fired. When a frame is defragmented, we use last skb dst field when building final skb. Its dst is valid, since we are in rcu read section. But if a timeout occurs, we take first queued fragment to build one ICMP TIME EXCEEDED message. Problem is all queued skb have weak dst pointers, since we escaped RCU critical section after their queueing. icmp_send() might dereference a now freed (and possibly reused) part of memory. Calling skb_dst_drop() and ip_route_input_noref() to revalidate route is the only possible choice. Reported-by: Denys Fedoryshchenko Signed-off-by: Eric Dumazet --- net/ipv4/ip_fragment.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) -- 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 diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c index a1151b8..b1d282f 100644 --- a/net/ipv4/ip_fragment.c +++ b/net/ipv4/ip_fragment.c @@ -223,31 +223,30 @@ static void ip_expire(unsigned long arg) if ((qp->q.last_in & INET_FRAG_FIRST_IN) && qp->q.fragments != NULL) { struct sk_buff *head = qp->q.fragments; + const struct iphdr *iph; + int err; rcu_read_lock(); head->dev = dev_get_by_index_rcu(net, qp->iif); if (!head->dev) goto out_rcu_unlock; + /* skb dst is stale, drop it, and perform route lookup again */ + skb_dst_drop(head); + iph = ip_hdr(head); + err = ip_route_input_noref(head, iph->daddr, iph->saddr, + iph->tos, head->dev); + if (err) + goto out_rcu_unlock; + /* - * Only search router table for the head fragment, - * when defraging timeout at PRE_ROUTING HOOK. + * Only an end host needs to send an ICMP + * "Fragment Reassembly Timeout" message, per RFC792. */ - if (qp->user == IP_DEFRAG_CONNTRACK_IN && !skb_dst(head)) { - const struct iphdr *iph = ip_hdr(head); - int err = ip_route_input(head, iph->daddr, iph->saddr, - iph->tos, head->dev); - if (unlikely(err)) - goto out_rcu_unlock; - - /* - * Only an end host needs to send an ICMP - * "Fragment Reassembly Timeout" message, per RFC792. - */ - if (skb_rtable(head)->rt_type != RTN_LOCAL) - goto out_rcu_unlock; + if (qp->user == IP_DEFRAG_CONNTRACK_IN && + skb_rtable(head)->rt_type != RTN_LOCAL) + goto out_rcu_unlock; - } /* Send an ICMP "Fragment Reassembly Timeout" message. */ icmp_send(head, ICMP_TIME_EXCEEDED, ICMP_EXC_FRAGTIME, 0);