From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Miller Subject: Re: [PATCH net-next 2/2] net: filter: don't release unattached filter through call_rcu() Date: Wed, 30 Jul 2014 19:57:16 -0700 (PDT) Message-ID: <20140730.195716.246796820845840784.davem@davemloft.net> References: <1406648188-3681-1-git-send-email-pablo@netfilter.org> <1406648188-3681-2-git-send-email-pablo@netfilter.org> Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: netfilter-devel@vger.kernel.org, netdev@vger.kernel.org, ast@plumgrid.com, dborkman@redhat.com, willemb@google.com, keescook@chromium.org To: pablo@netfilter.org Return-path: Received: from shards.monkeyblade.net ([149.20.54.216]:38801 "EHLO shards.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755956AbaGaC5R (ORCPT ); Wed, 30 Jul 2014 22:57:17 -0400 In-Reply-To: <1406648188-3681-2-git-send-email-pablo@netfilter.org> Sender: netfilter-devel-owner@vger.kernel.org List-ID: From: Pablo Neira Ayuso Date: Tue, 29 Jul 2014 17:36:28 +0200 > sk_unattached_filter_destroy() does not always need to release the > filter object via rcu. Since this filter is never attached to the > socket, the caller should be responsible for releasing the filter > in a safe way, which may not necessarily imply rcu. > > This is a short summary of clients of this function: > > 1) xt_bpf.c and cls_bpf.c use the bpf matchers from rules, these rules > are removed from the packet path before the filter is released. Thus, > the framework makes sure the filter is safely removed. > > 2) In the ppp driver, the ppp_lock ensures serialization between the > xmit and filter attachment/detachment path. This doesn't use rcu > so deferred release via rcu makes no sense. > > 3) In the isdn/ppp driver, it is called from isdn_ppp_release() > the isdn_ppp_ioctl(). This driver uses mutex and spinlocks, no rcu. > Thus, deferred rcu makes no sense to me either, the deferred releases > may be just masking the effects of wrong locking strategy, which > should be fixed in the driver itself. > > 4) In the team driver, this is the only place where the rcu > synchronization with unattached filter is used. Therefore, this > patch introduces synchronize_rcu() which is called from the > genetlink path to make sure the filter doesn't go away while packets > are still walking over it. I think we can revisit this once struct > bpf_prog (that only wraps specific bpf code bits) is in place, then > add some specific struct rcu_head in the scope of the team driver if > Jiri thinks this is needed. > > Deferred rcu release for unattached filters was originally introduced > in 302d663 ("filter: Allow to create sk-unattached filters"). > > Signed-off-by: Pablo Neira Ayuso Also applied, thanks.