cvs commit: src/sys/netinet6 in6_src.c

[John Baldwin]

On Wednesday 17 August 2005 12:49 pm, Hajimu UMEMOTO wrote:
> Hi,
>
> >>>>> On Tue, 16 Aug 2005 15:59:22 -0400
> >>>>> John Baldwin <jhb at FreeBSD.org> 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 <jhb at FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve"  =  http://www.FreeBSD.org