From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pablo Neira Subject: Re: ctnetlink & helper_locking_fix conflict Date: Wed, 10 Nov 2004 19:45:28 +0100 Message-ID: <419261C8.2010707@eurodev.net> References: <20041109171106.GF22257@sunbeam.de.gnumonks.org> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Netfilter Developers , Henrik Nordstrom Return-path: To: Harald Welte In-Reply-To: <20041109171106.GF22257@sunbeam.de.gnumonks.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: netfilter-devel-bounces@lists.netfilter.org Errors-To: netfilter-devel-bounces@lists.netfilter.org List-Id: netfilter-devel.vger.kernel.org Harald Welte wrote: >On Tue, Nov 09, 2004 at 08:13:33AM +0100, Henrik Nordstrom wrote: > > >>Hi, >> >>there is a conflict between the above two patches. The helper_locking_fix >>removes the locks from the helpers, but ctnetlink adds code using these >>locks. >> >>>>From reading the code I think the helper side locking should be removed >>from the ctnetlink additions as well, but just wanted to verify with you. >> >> > >I think it is safe. We're holding a write lock on ip_conntrack_lock and >thus it's safe to change the helper data. > >If you have a fix for ctnetlink, that would be appreciated > > I think that there's a minor race. In ip_conntrack_in() 855 ret = ct->helper->help(*pskb, ct, ctinfo) This call isn't in a locked section. Someone could be calling the helper at the same time that ctnetlink is modifying the private helper info. I've annotated this issue since I'll be working in that code (change API) once I've done with the event stuff. Pablo