From owner-cvs-all@FreeBSD.ORG Wed Aug 17 17:27:51 2005 Return-Path: X-Original-To: cvs-all@FreeBSD.org Delivered-To: cvs-all@FreeBSD.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 467EB16A42C; Wed, 17 Aug 2005 17:27:51 +0000 (GMT) (envelope-from jhb@FreeBSD.org) Received: from mv.twc.weather.com (mv.twc.weather.com [65.212.71.225]) by mx1.FreeBSD.org (Postfix) with ESMTP id 9F52443D45; Wed, 17 Aug 2005 17:27:50 +0000 (GMT) (envelope-from jhb@FreeBSD.org) Received: from [10.50.40.201] (Not Verified[10.50.40.201]) by mv.twc.weather.com with NetIQ MailMarshal (v6, 0, 3, 8) id ; Wed, 17 Aug 2005 13:42:48 -0400 From: John Baldwin To: Hajimu UMEMOTO Date: Wed, 17 Aug 2005 13:18:25 -0400 User-Agent: KMail/1.8 References: <200508161949.j7GJnAaG015685@repoman.freebsd.org> <200508161559.24097.jhb@FreeBSD.org> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200508171318.27652.jhb@FreeBSD.org> Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/netinet6 in6_src.c X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 17 Aug 2005 17:27:51 -0000 On Wednesday 17 August 2005 12:49 pm, Hajimu UMEMOTO wrote: > Hi, > > >>>>> On Tue, 16 Aug 2005 15:59:22 -0400 > >>>>> John Baldwin said: > > jhb> 1) This should be using TAILQ_FOREACH() to be more readable. > > jhb> 2) It's not safe to drop the lock here as you've done as the list can > change jhb> out from under you. You might be able to use > TAILQ_FOREACH_SAFE() if you jhb> know what the next entry in the can't be > removed from the list while the lock jhb> is dropped, but that's usually > not an easy thing to ensure unless you set a jhb> flag or some such while > holding the lock to communicate that to the code that jhb> adds and removes > items to this list. You may need to use an sx lock here jhb> rather than a > mutex. > > jhb> As it is, this change opens up a race condition that can result in > deref'ing jhb> bogus pointers and kernel panics. > > Thank you for your suggestion. I've committed the fix as your > suggestion. If it is still wrong way, please let me know. > > Sincerely, Thanks. I see that you still kept the mutex and and properly lock both the sx and mutex when making updates, so it seems it is on purpose. The one place that doesn't use the sx lock is lookup_addrsel_policy() which is called from in6_selectsrc(). I guess it is not ok to sleep in that function and that is why you don't use the sx lock in that one place? -- John Baldwin <>< http://www.FreeBSD.org/~jhb/ "Power Users Use the Power to Serve" = http://www.FreeBSD.org