Re: [PATCH][MCAST]IPv6: doubt about ipv6_sk_mc_lock usage.

[David Stevens]

> I have one more question. 
> Why ip6_mc_source() uses read_lock_bh(&ipv6_sk_mc_lock) and 
ip6_mc_msfilter() 
> doesn't use ipv6_sk_mc_lock at all. 
> when ipv6_mc_list's sflist are accessed by inet6_mc_check(), Can it be 
> modified by ip6_mc_source() or ip6_mc_msfilter() ?
> (For example ipv6_mc_list's sflist is freed up by sock_kfree_s(), when 
> inet6_mc_check() uses it)

Yan,

There certainly may be a bug, but removing the lock isn't the fix. :-)

inet6_mc_check() does not have the socket locked, but is acquiring a
read lock on ipv6_sk_mc_lock.

I've looked some more into this, and I believe ip6_mc_msfilter() needs
at least a read lock on ipv6_sk_mc_lock to protect it from races with
changes to the list, just as ip6_mc_source() has.

I convinced myself at the time that the sflist does not require an
additional lock, but I don't see that now. It seems to me now that
there should be a lock on each individual socklist entry and changes
to the socket source filter should be protected by that. A simpler,
but less performant, fix would be to make both ipv6_mc_source() and
ip6_mc_msfilter() acquire ipv6_sk_mc_lock for writing, to prevent
races with inet6_mc_check's search of the sflist.

It'd be much better if only that socklist entry is locked, of course.

I'll look some more.

                        +-DLS

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/