On Mon, Aug 30, 2004 at 11:08:20PM -0700, David S. Miller wrote:

On Sun, 29 Aug 2004 16:50:31 +1000
Herbert Xu [off-list ref] wrote:

quoted

quoted

Secondly, until neigh_create() takes the tbl lock, it is not in
the hash tables and therefore neigh_ifdown() could not see it.

That part I agree with :) That's in fact what this race is about:
neigh_ifdown does not guarantee that the hash table will be without
references to idev.

I think we can clear this by putting neigh_parms_release() into an
RCU handler.  It can't be in in_dev_rcu_put.

Yes that should go a long way in resolving this problem.  However it
is still tricky because the neighbour table doesn't refer to idev
directly.  So we'll need to go through contortions to make sure that
the corresponding idev is still alive when we add a neighbour with
its neigh_parms to the hash table.  Or better yet we should ref count
the neigh_parms directly.

I also now believe that these sorts of races you are mentioning percolate
out into ip_mc_destroy_dev().

Well last time I looked at it I concluded that it was safe.  It seems
that the references are either held by timers who are all taken down
at the very start of the destruction, or they're taken at places which
are under spin locks.

Did I miss something else?

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt