From mboxrd@z Thu Jan 1 00:00:00 1970 From: Patrick McHardy Subject: Re: crash in death_by_timeout() Date: Wed, 19 Nov 2008 13:47:04 +0100 Message-ID: <49240AC8.6040002@trash.net> References: <20081117221855.GD3271@zebra.home> <4922A1E8.7080405@trash.net> <20081118123830.GD3201@zebra.home> <4922C0F7.3050604@trash.net> <4922C2D0.9060207@trash.net> <492340EE.1020607@netfilter.org> <492400B3.6070706@trash.net> <49240874.1090208@netfilter.org> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Cc: BORBELY Zoltan , Netfilter Development Mailinglist To: Pablo Neira Ayuso Return-path: Received: from stinky.trash.net ([213.144.137.162]:62852 "EHLO stinky.trash.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752603AbYKSMrM (ORCPT ); Wed, 19 Nov 2008 07:47:12 -0500 In-Reply-To: <49240874.1090208@netfilter.org> Sender: netfilter-devel-owner@vger.kernel.org List-ID: Pablo Neira Ayuso wrote: > Patrick McHardy wrote: >> Pablo Neira Ayuso wrote: >>> Patrick McHardy wrote: >>>> Pablo, do you recall the reason why the lock isn't held in >>>> ctnetlink_create_conntrack()? >>> The creation is done under the nfnl_mutex so that requests to create >>> identical entries cannot race. Of course, this is not enough to avoid >>> the race with the timer if we set a very small timer for a conntrack :(. >> Its also not enough to avoid the race against packet processing, >> which takes nf_conntrack_lock. >> >>> AFAICS, we don't need to enclose the whole conntrack creation path. >>> Would you prefer the patch attached? This patch should apply fine to >>> 2.6.28-rc. >> That fixes the timer race, but the race between lookup and creation >> remains. We really need to either hold the lock the entire time or >> redo the lookup before inserting the entry into the hash tables. > > I see, I forgot about that case. Your patch should be fine then. Thanks, I've queued it up, but will wait for confirmation from Zoltan.