Re: [PATCH] fix sk->sk_filter field access

[David Miller <davem@davemloft.net>]

From: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Date: Thu, 31 Aug 2006 02:20:42 +0400

> 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().

Ok.

> 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.

Really?

It is used with needlock=0 by DCCP ipv6, for example.  This case seems
correct too.  What about sk_receive_skb()?  dn_queue_skb()?  In fact,
there seems to be numerous uses still with needlock=0, all legitimate.

> 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.

Let us to fix bugs first, and then consider rewriting the locking.
:-)