On Thu, Mar 23, 2017 at 10:36:59PM +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>
> ---
> V2: rebase on the latest nf tree
>
> net/netfilter/nfnetlink_cthelper.c | 182 ++++++++++++++++++-------------------
> 1 file changed, 88 insertions(+), 94 deletions(-)
>
> diff --git a/net/netfilter/nfnetlink_cthelper.c b/net/netfilter/nfnetlink_cthelper.c
> index 2b987d2..304aab8 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;
> +};
I overlook this. Any reason for not using this declaration instead?
struct nfnl_cthelper {
struct list_head list;
struct nf_conntrack_helper helper;
};
We would simplify this a bit as the helper would be embedded into the
new nfnl_cthelper structure.
--
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
