Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> Export a new nf_queue() function that translates the NF_QUEUE verdict
> depending on the scenario:
>
> 1) Drop packet if queue is full.
> 2) Accept packet if bypass is enabled.
> 3) Return stolen if packet is enqueued.
>
> We can call this function from xt_NFQUEUE and nft_queue. Thus, we
> move packet queuing to userspace away from the core path.
>
> We still have to handle the old QUEUE standard target for
> {ip,ip6}_tables, which points to queue number zero. Just in case we
> still have any user relying on this behaviour. No need to handle this
> from arp and ebtables, they never got a native queue target.
>
> After this patch, we have to inconditionally set state->hook_entries
> before calling the hook since nf_iterate() since we need this to know
> from what hook the packet is escaping to userspace in nf_queue.
>
> From nft_verdict_init(), disallow NF_QUEUE as verdict since we always
> use the nft_queue expression for this and we don't have any userspace
> code using this since the beginning.
>
> Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> ---
> include/net/netfilter/nf_queue.h | 3 +++
> net/ipv4/netfilter/arp_tables.c | 1 +
> net/ipv4/netfilter/ip_tables.c | 4 ++++
> net/ipv6/netfilter/ip6_tables.c | 4 ++++
> net/netfilter/core.c | 14 ++---------
> net/netfilter/nf_internals.h | 2 --
> net/netfilter/nf_queue.c | 51 ++++++++++++++++++++++++++--------------
> net/netfilter/nf_tables_api.c | 3 +--
> net/netfilter/nf_tables_core.c | 3 +--
> net/netfilter/nft_queue.c | 6 ++---
> net/netfilter/xt_NFQUEUE.c | 29 ++++++++++++-----------
> 11 files changed, 67 insertions(+), 53 deletions(-)
>
> diff --git a/include/net/netfilter/nf_queue.h b/include/net/netfilter/nf_queue.h
> index 2280cfe86c56..807b9de72b43 100644
> --- a/include/net/netfilter/nf_queue.h
> +++ b/include/net/netfilter/nf_queue.h
> @@ -29,6 +29,9 @@ struct nf_queue_handler {
>
> void nf_register_queue_handler(struct net *net, const struct nf_queue_handler *qh);
> void nf_unregister_queue_handler(struct net *net);
> +
> +int nf_queue(struct sk_buff *skb, const struct nf_hook_state *state,
> + unsigned int queuenum, bool bypass);
> void nf_reinject(struct nf_queue_entry *entry, unsigned int verdict);
>
> void nf_queue_entry_get_refs(struct nf_queue_entry *entry);
> diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
> index e76ab23a2deb..83d82f6be8dd 100644
> --- a/net/ipv4/netfilter/arp_tables.c
> +++ b/net/ipv4/netfilter/arp_tables.c
> @@ -28,6 +28,7 @@
>
> #include <linux/netfilter/x_tables.h>
> #include <linux/netfilter_arp/arp_tables.h>
> +#include <net/netfilter/nf_queue.h>
> #include "../../netfilter/xt_repldata.h"
>
> MODULE_LICENSE("GPL");
This include looks unneeded.
> diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
> index de4fa03f46f3..7040842c34f4 100644
> --- a/net/ipv4/netfilter/ip_tables.c
> +++ b/net/ipv4/netfilter/ip_tables.c
> @@ -29,6 +29,7 @@
> #include <linux/netfilter/x_tables.h>
> #include <linux/netfilter_ipv4/ip_tables.h>
> #include <net/netfilter/nf_log.h>
> +#include <net/netfilter/nf_queue.h>
> #include "../../netfilter/xt_repldata.h"
>
> MODULE_LICENSE("GPL");
> @@ -329,6 +330,9 @@ ipt_do_table(struct sk_buff *skb,
> /* Pop from stack? */
> if (v != XT_RETURN) {
> verdict = (unsigned int)(-v) - 1;
> + if (verdict == NF_QUEUE)
> + verdict = nf_queue(skb, state,
> + 0, false);
Any reason why this is needed?
AFAICS xt_NFQUEUE will never return NF_QUEUE after this patch.
> diff --git a/net/netfilter/core.c b/net/netfilter/core.c
> index 2b3b2f8e39c4..9ae2febd86e3 100644
> --- a/net/netfilter/core.c
> +++ b/net/netfilter/core.c
> @@ -309,6 +309,7 @@ unsigned int nf_iterate(struct sk_buff *skb,
> unsigned int verdict;
>
> while (*entryp) {
> + RCU_INIT_POINTER(state->hook_entries, *entryp);
> repeat:
> verdict = (*entryp)->ops.hook((*entryp)->ops.priv, skb, state);
> if (verdict != NF_ACCEPT) {
> @@ -331,9 +332,8 @@ int nf_hook_slow(struct sk_buff *skb, struct nf_hook_state *state)
> int ret;
>
> entry = rcu_dereference(state->hook_entries);
> -next_hook:
> verdict = nf_iterate(skb, state, &entry);
> - switch (verdict & NF_VERDICT_MASK) {
> + switch (verdict) {
This looks buggy, verdict might encode errno for NF_DROP case.
What you could do is:
switch (verdict) {
case NF_ACCEPT:
/* something */
break;
case NF_STOLEN:
break;
case NF_DROP: /* fallthrough */
default: /* drop with error? */
kfree_skb(skb);
errno = ...
}
--
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
