On Sun, Mar 19, 2017 at 10:36:02PM +0800, Liping Zhang wrote:
> From: Liping Zhang <zlpnobody@xxxxxxxxx>
>
> The nf_ct_helper_hash table is protected by nf_ct_helper_mutex, while
> nfct_helper operation is protected by nfnl_lock(NFNL_SUBSYS_CTHELPER).
> So it's possible that one CPU is walking the nf_ct_helper_hash for
> cthelper add/get/del, another cpu is doing nf_conntrack_helpers_unregister
> at the same time. This is dangrous, and may cause use after free error.
>
> Note, delete operation will flush all cthelpers added via nfnetlink, so
> using rcu to do protect is not easy.
>
> Now introduce a dummy list to record all the cthelpers added via
> nfnetlink, then we can walk the dummy list instead of walking the
> nf_ct_helper_hash. Also, keep nfnl_cthelper_dump_table unchanged, it
> may be invoked without nfnl_lock(NFNL_SUBSYS_CTHELPER) held.
>
> Signed-off-by: Liping Zhang <zlpnobody@xxxxxxxxx>
> ---
> net/netfilter/nfnetlink_cthelper.c | 176 +++++++++++++++++++------------------
> 1 file changed, 89 insertions(+), 87 deletions(-)
>
> diff --git a/net/netfilter/nfnetlink_cthelper.c b/net/netfilter/nfnetlink_cthelper.c
> index fc4733f..47424ec 100644
> --- a/net/netfilter/nfnetlink_cthelper.c
> +++ b/net/netfilter/nfnetlink_cthelper.c
> @@ -32,6 +32,13 @@ MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>");
> MODULE_DESCRIPTION("nfnl_cthelper: User-space connection tracking helpers");
>
> +struct nfnl_cthelper {
> + struct list_head list;
> + struct nf_conntrack_helper *helper;
> +};
> +
> +static LIST_HEAD(nfnl_cthelper_list);
We need a field possible_net_t so we can store what netns this helper
belongs to, thus in case of flush command, we just remove the helpers
that this netns owns.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
