netdev - Re: [PATCH net-next 2/2] net: filter: don't release unattached filter through call

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMEtUuxGN_v5SP6HJ74p6fEuop6qXhe0Q-6tkpcCkrnsAtQUvw@mail.gmail.com>
Date:	Tue, 29 Jul 2014 09:09:46 -0700
From:	Alexei Starovoitov <ast@...mgrid.com>
To:	Pablo Neira Ayuso <pablo@...filter.org>
Cc:	netfilter-devel <netfilter-devel@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Network Development <netdev@...r.kernel.org>,
	Daniel Borkmann <dborkman@...hat.com>,
	Willem de Bruijn <willemb@...gle.com>,
	Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH net-next 2/2] net: filter: don't release unattached filter
 through call_rcu()

On Tue, Jul 29, 2014 at 8:36 AM, Pablo Neira Ayuso <pablo@...filter.org> wrote:
> 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.

Correct.
There are also:
5) ptp_classifer - which is ok as well, since there is no filter_destroy.

6) seccomp - there I think it's also ok, since filter is called from syscall
and filter freeing is done on task exit
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html