Hello!
> > Function sk_filter() is called from tcp_v{4,6}_rcv() functions with argue
> > needlock = 0, while socket is not locked at that moment. In order to avoid
> > this and similar issues in the future, use rcu for sk->sk_filter field read
> > protection.
> >
> > Patch is for net-2.6.19
>
> What bug are you fixing? The current code looks correct to me.
>
> Why is it important to avoid the needlock stuff in the future?
>
> This patch description is very incomplete, you should list the
> precise reason why you are doing something instead of using
> vague language.
Yes, explanation could be better.
Current code in tcp_v4_rcv() calls sk_filter() _before_ it takes socket lock.
This happened when LSM patches were applied. Apparently, LSM does not
want to see socket locked in security_sock_rcv_skb().
Obvious solution is to change the third argument of sk_filter "needlock" to 1.
Then we see that sk_filter() is not used with needlock=0 anymore, therefore
it can be completely eliminated. It was original fix.
I suggested to remove ugly misuse of bh_lock_sock() (introduced by me,
just because there was no better lock to use) and replace it with RCU, which
is logical and clean.
The patch looks decent. I had one doubt about misuse of rcu_read_lock_bh()
in sk_attach_filter(). Probably, it should be plain local_bh_disable(),
I do not know. But because rcu_read_lock_bh() actually is local_bh_disable(),
it seems to be not a serious issue.
Alexey
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
