From: ebiederm@xmission.com (Eric W. Biederman)
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: netdev@vger.kernel.org, netfilter-devel@vger.kernel.org,
Stephen Hemminger <stephen@networkplumber.org>,
Juanjo Ciarlante <jjciarla@raiz.uncu.edu.ar>,
Wensong Zhang <wensong@linux-vs.org>,
Simon Horman <horms@verge.net.au>, Julian Anastasov <ja@ssi.bg>,
Patrick McHardy <kaber@trash.net>,
Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>,
Jamal Hadi Salim <jhs@mojatatu.com>,
Steffen Klassert <steffen.klassert@secunet.com>,
Herbert Xu <herbert@gondor.apana.org.au>,
David Miller <davem@davemloft.net>
Subject: Re: [PATCH -next 5/6] netfilter: Per network namespace netfilter hooks.
Date: Wed, 15 Jul 2015 15:22:09 -0500 [thread overview]
Message-ID: <87twt5i32m.fsf@x220.int.ebiederm.org> (raw)
In-Reply-To: <20150715190011.GA1203@salvia> (Pablo Neira Ayuso's message of "Wed, 15 Jul 2015 21:00:11 +0200")
Pablo Neira Ayuso <pablo@netfilter.org> writes:
> On Fri, Jul 10, 2015 at 06:15:06PM -0500, Eric W. Biederman wrote:
>> @@ -102,13 +112,35 @@ int nf_register_hook(struct nf_hook_ops *reg)
>> #endif
>> return 0;
>> }
>> -EXPORT_SYMBOL(nf_register_hook);
>> +EXPORT_SYMBOL(nf_register_net_hook);
>>
>> -void nf_unregister_hook(struct nf_hook_ops *reg)
>> +void nf_unregister_net_hook(struct net *net, const struct nf_hook_ops *reg)
>> {
>> + struct list_head *nf_hook_list;
>> + struct nf_hook_ops *elem;
>> +
>> + nf_hook_list = find_nf_hook_list(net, reg);
>> + if (!nf_hook_list)
>> + return;
>> +
>> mutex_lock(&nf_hook_mutex);
>> - list_del_rcu(®->list);
>> + list_for_each_entry(elem, nf_hook_list, list) {
>> + if ((reg->hook == elem->hook) &&
>> + (reg->dev == elem->dev) &&
>> + (reg->owner == elem->owner) &&
>> + (reg->priv == elem->priv) &&
>> + (reg->pf == elem->pf) &&
>> + (reg->hooknum == elem->hooknum) &&
>> + (reg->priority == elem->priority)) {
>> + list_del_rcu(&elem->list);
>> + break;
>> + }
>> + }
>
> I think I found a problem with this code above.
>
> If we register two hooks from the same module using exactly the same
> tuple that identifies this, we delete the hook that we don't want, eg.
>
> nft add table filter
> nft add chain filter test { type filter hook input priority 0\; }
> nft add chain filter test2 { type filter hook input priority 0\; }
>
> then, you delete 'test':
>
> nft delete chain filter test
>
> This will delete 'test2' hook instead of 'test' as it will find this
> in first place on the list.
So we have two adjacent entries on the same chain that perform the exact
same action. We delete one of them.
I do not see how that is noticable. Registration order plays a small
role but especially with the priority bit we don't strongly honor
registration order.
In your example above we will distinguish between the two chains
as nf_hook_ops->priv will point the nf tables chain. So that specific
case is at least safe.
> I think we should add a cookie field that stores the address of the
> original hook object that is passed as parameter, so we are sure we
> kill the right hook.
I don't believe that is necessary. To the best of my knowledge for a
registration to be meaningful we must always change at least one of
those fields I am comparing. Typically either priv or hook.
It is a real change from what we have been doing but there is a lot
of freedom in not needing to keep the original structure around.
Eric
next prev parent reply other threads:[~2015-07-15 20:28 UTC|newest]
Thread overview: 90+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-15 3:07 [PATCH net-next 00/15] Simplify netfilter and network namespaces Eric W. Biederman
2015-06-15 3:12 ` [PATCH net-next 01/15] netfilter: Kill unused copies of RCV_SKB_FAIL Eric W. Biederman
2015-06-15 3:13 ` [PATCH net-next 02/15] netfilter: Pass struct net into the netfilter hooks Eric W. Biederman
2015-06-15 3:13 ` [PATCH net-next 03/15] netfilter: Use nf_hook_state.net Eric W. Biederman
2015-06-15 3:13 ` [PATCH net-next 04/15] ebtables: Simplify the arguments to ebt_do_table Eric W. Biederman
2015-06-15 3:13 ` [PATCH net-next 05/15] inet netfilter: Remove hook from ip6t_do_table, arp_do_table, ipt_do_table Eric W. Biederman
2015-06-15 3:13 ` [PATCH net-next 06/15] inet netfilter: Prefer state->hook to ops->hooknum Eric W. Biederman
2015-06-15 3:13 ` [PATCH net-next 07/15] nftables: kill nft_pktinfo.ops Eric W. Biederman
2015-06-15 3:13 ` [PATCH net-next 08/15] tc: Simplify em_ipset_match Eric W. Biederman
2015-06-15 3:13 ` [PATCH net-next 09/15] x_tables: Pass struct net in xt_action_param Eric W. Biederman
2015-06-15 3:13 ` [PATCH net-next 10/15] x_tables: Use par->net instead of computing from the passed net devices Eric W. Biederman
2015-06-15 3:13 ` [PATCH net-next 11/15] nftables: Pass struct net in nft_pktinfo Eric W. Biederman
2015-06-15 3:13 ` [PATCH net-next 12/15] nf_tables: Use pkt->net instead of computing net from the passed net_devices Eric W. Biederman
2015-06-15 3:13 ` [PATCH net-next 13/15] nf_conntrack: Add a struct net parameter to l4_pkt_to_tuple Eric W. Biederman
2015-06-15 3:13 ` [PATCH net-next 14/15] ipv4: Pass struct net into ip_defrag and ip_check_defrag Eric W. Biederman
2015-06-15 3:13 ` [PATCH net-next 15/15] ipv6: Pass struct net into nf_ct_frag6_gather Eric W. Biederman
2015-06-15 7:06 ` [PATCH net-next 00/15] Simplify netfilter and network namespaces Pablo Neira Ayuso
2015-06-15 15:06 ` Eric W. Biederman
2015-06-15 15:20 ` Pablo Neira Ayuso
2015-06-16 0:10 ` David Miller
2015-06-16 0:26 ` Eric W. Biederman
2015-06-16 2:14 ` David Miller
2015-06-16 10:32 ` Pablo Neira Ayuso
2015-06-16 21:00 ` Eric W. Biederman
2015-06-17 15:09 ` [PATCH net-next 00/43] Simplify netfilter and network namespaces (take 2) Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 01/43] netfilter: Kill unused copies of RCV_SKB_FAIL Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 02/43] netfilter: Pass struct net into the netfilter hooks Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 03/43] netfilter: Use nf_hook_state.net Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 04/43] ebtables: Simplify the arguments to ebt_do_table Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 05/43] inet netfilter: Remove hook from ip6t_do_table, arp_do_table, ipt_do_table Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 06/43] inet netfilter: Prefer state->hook to ops->hooknum Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 07/43] nftables: kill nft_pktinfo.ops Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 08/43] tc: Simplify em_ipset_match Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 09/43] x_tables: Pass struct net in xt_action_param Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 10/43] x_tables: Use par->net instead of computing from the passed net devices Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 11/43] nftables: Pass struct net in nft_pktinfo Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 12/43] nf_tables: Use pkt->net instead of computing net from the passed net_devices Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 13/43] nf_conntrack: Add a struct net parameter to l4_pkt_to_tuple Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 14/43] ipv4: Pass struct net into ip_defrag and ip_check_defrag Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 15/43] ipv6: Pass struct net into nf_ct_frag6_gather Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 16/43] net: include missing headers in net/net_namespace.h Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 17/43] netfilter: use forward declaration instead of including linux/proc_fs.h Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 18/43] netfilter: don't pull include/linux/netfilter.h from netns headers Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 19/43] ipvs: Read hooknum from state rather than ops->hooknum Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 20/43] netfilter: Pass priv instead of nf_hook_ops to netfilter hooks Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 21/43] netfilter: Add a network namespace Kconfig conflict Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 22/43] netfilter: Add a struct net parameter to nf_register_hook[s] Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 23/43] netfilter: Add a struct net parameter to nf_unregister_hook[s] Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 24/43] netfilter: Make the netfilter hooks per network namespace Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 25/43] netfilter: Make nf_hook_ops just a parameter structure Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 26/43] netfitler: Remove spurios included of netfilter.h Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 27/43] x_tables: Add magical hook registration in the common case Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 28/43] x_tables: Where possible convert to the new hook registration method Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 29/43] x_tables: Kill xt_[un]hook_link Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 30/43] x_tables: Update ip?table_nat to register their hooks in all network namespaces Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 31/43] netfilter: nf_tables: adapt it to pernet hooks Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 32/43] netfilter: ipt_CLUSTERIP: adapt it to support " Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 33/43] netfilter: ebtables: adapt the filter and nat table to " Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 34/43] netfilter: bridge: adapt it " Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 35/43] ipvs: Register netfilter hooks in all network namespaces Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 36/43] netfilter: nf_conntract: " Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 37/43] netfilter: nf_defrag: " Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 38/43] netfilter: synproxy: " Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 39/43] selinux: adapt it to pernet hooks Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 40/43] smack: " Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 41/43] netfilter: Remove the network namespace Kconfig conflict Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 42/43] netfilter bridge: Make the sysctl knobs per network namespace Eric W. Biederman
2015-06-17 15:28 ` [PATCH net-next 43/43] netfilter: Skip unnecessary calls to synchronize_net Eric W. Biederman
2015-06-17 17:20 ` Patrick McHardy
2015-06-17 20:32 ` Eric W. Biederman
2015-06-18 15:49 ` [PATCH net-next 00/43] Simplify netfilter and network namespaces (take 2) Andreas Schultz
2015-06-18 19:40 ` Pablo Neira Ayuso
2015-07-10 23:11 ` [PATCH -next 0/6] Per network namespace netfilter chains Eric W. Biederman
2015-07-10 23:12 ` [PATCH -next 1/6] netfilter: nf_queue: Don't recompute the hook_list head Eric W. Biederman
2015-07-10 23:13 ` [PATCH -next 2/6] netfilter: kill nf_hooks_active Eric W. Biederman
2015-07-10 23:13 ` [PATCH -next 3/6] netfilter: Simply the tests for enabling and disabling the ingress queue hook Eric W. Biederman
2015-07-10 23:14 ` [PATCH -next 4/6] netfilter: Factor out the hook list selection from nf_register_hook Eric W. Biederman
2015-07-10 23:15 ` [PATCH -next 5/6] netfilter: Per network namespace netfilter hooks Eric W. Biederman
2015-07-15 19:00 ` Pablo Neira Ayuso
2015-07-15 20:22 ` Eric W. Biederman [this message]
2015-07-10 23:15 ` [PATCH -next 6/6] netfilter: nftables: Only run the nftables chains in the proper netns Eric W. Biederman
2015-07-15 17:20 ` [PATCH -next 0/6] Per network namespace netfilter chains Pablo Neira Ayuso
2015-07-15 20:05 ` Eric W. Biederman
2015-07-16 11:01 ` Pablo Neira Ayuso
2015-06-17 19:38 ` [PATCH net-next 00/15] Simplify netfilter and network namespaces Julian Anastasov
2015-06-17 20:55 ` Eric W. Biederman
2015-06-17 22:01 ` Julian Anastasov
2015-06-18 14:45 ` Eric W. Biederman
2015-06-18 19:21 ` Julian Anastasov
2015-06-19 14:24 ` Eric W. Biederman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87twt5i32m.fsf@x220.int.ebiederm.org \
--to=ebiederm@xmission.com \
--cc=davem@davemloft.net \
--cc=herbert@gondor.apana.org.au \
--cc=horms@verge.net.au \
--cc=ja@ssi.bg \
--cc=jhs@mojatatu.com \
--cc=jjciarla@raiz.uncu.edu.ar \
--cc=kaber@trash.net \
--cc=kadlec@blackhole.kfki.hu \
--cc=netdev@vger.kernel.org \
--cc=netfilter-devel@vger.kernel.org \
--cc=pablo@netfilter.org \
--cc=steffen.klassert@secunet.com \
--cc=stephen@networkplumber.org \
--cc=wensong@linux-vs.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.