[PATCH 1/2 nf] netfilter: nf_queue: don't re-enter same hook on packet reinjection

[PATCH 1/2 nf] netfilter: nf_queue: don't re-enter same hook on packet reinjection

[Pablo Neira Ayuso]

Make sure we skip the current hook from where the packet was enqueued,
otherwise the packets gets enqueued over and over again.

Fixes: e3b37f11e6e4 ("netfilter: replace list_head with single linked list")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
I managed to reproduce this with a simple test.

 # iptables -I OUTPUT -j QUEUE
 # cd libnetfilter_queue/utils/
 # ./nfqnl_test

Without my patch, netfilter munches packets that are reinjected.

@Aaron: Please, I'd appreciate if you can have a look to confirm this bug
        and the fix. Thanks.

 net/netfilter/nf_queue.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c
index 96964a0070e1..221d7a5c2fec 100644
--- a/net/netfilter/nf_queue.c
+++ b/net/netfilter/nf_queue.c
@@ -184,6 +184,7 @@ void nf_reinject(struct nf_queue_entry *entry, unsigned int verdict)
 			verdict = NF_DROP;
 	}
 
+	hook_entry = rcu_dereference(hook_entry->next);
 	entry->state.thresh = INT_MIN;
 
 	if (verdict == NF_ACCEPT) {
-- 
2.1.4

[PATCH 2/2 nf] netfilter: nft_range: validate operation netlink attribute

[Pablo Neira Ayuso]

Use nft_parse_u32_check() to make sure we don't get a value over the
unsigned 8-bit integer. Moreover, make sure this value doesn't go over
the two supported range comparison modes.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_range.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nft_range.c b/net/netfilter/nft_range.c
index c6d5358482d1..9bc4586c3006 100644
--- a/net/netfilter/nft_range.c
+++ b/net/netfilter/nft_range.c
@@ -59,6 +59,7 @@ static int nft_range_init(const struct nft_ctx *ctx, const struct nft_expr *expr
 	struct nft_range_expr *priv = nft_expr_priv(expr);
 	struct nft_data_desc desc_from, desc_to;
 	int err;
+	u32 op;
 
 	err = nft_data_init(NULL, &priv->data_from, sizeof(priv->data_from),
 			    &desc_from, tb[NFTA_RANGE_FROM_DATA]);
@@ -80,7 +81,20 @@ static int nft_range_init(const struct nft_ctx *ctx, const struct nft_expr *expr
 	if (err < 0)
 		goto err2;
 
-	priv->op  = ntohl(nla_get_be32(tb[NFTA_RANGE_OP]));
+	err = nft_parse_u32_check(tb[NFTA_RANGE_OP], U8_MAX, &op);
+	if (err < 0)
+		goto err2;
+
+	switch (op) {
+	case NFT_RANGE_EQ:
+	case NFT_RANGE_NEQ:
+		break;
+	default:
+		err = -EINVAL;
+		goto err2;
+	}
+
+	priv->op  = op;
 	priv->len = desc_from.len;
 	return 0;
 err2:
-- 
2.1.4

Re: [PATCH 1/2 nf] netfilter: nf_queue: don't re-enter same hook on packet reinjection

[Aaron Conole]

Pablo Neira Ayuso <pablo@netfilter.org> writes:

> Make sure we skip the current hook from where the packet was enqueued,
> otherwise the packets gets enqueued over and over again.
>
> Fixes: e3b37f11e6e4 ("netfilter: replace list_head with single linked list")
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
> I managed to reproduce this with a simple test.
>
>  # iptables -I OUTPUT -j QUEUE
>  # cd libnetfilter_queue/utils/
>  # ./nfqnl_test
>
> Without my patch, netfilter munches packets that are reinjected.
>
> @Aaron: Please, I'd appreciate if you can have a look to confirm this bug
>         and the fix. Thanks.

Looks like I missed this in my testing.

Reviewed-by: Aaron Conole <aconole@bytheb.org>

>  net/netfilter/nf_queue.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c
> index 96964a0070e1..221d7a5c2fec 100644
> --- a/net/netfilter/nf_queue.c
> +++ b/net/netfilter/nf_queue.c
> @@ -184,6 +184,7 @@ void nf_reinject(struct nf_queue_entry *entry, unsigned int verdict)
>  			verdict = NF_DROP;
>  	}
>  
> +	hook_entry = rcu_dereference(hook_entry->next);
>  	entry->state.thresh = INT_MIN;
>  
>  	if (verdict == NF_ACCEPT) {