[nftables PATCH] payload: moderate the expr operation checking

[nftables PATCH] payload: moderate the expr operation checking

[Alvaro Neira Ayuso]

From: Álvaro Neira Ayuso <alvaroneay@gmail.com>

When we add this rule:

sudo nft add rule ip test input ip protocol != icmp

If we try to list the rules in the table test, nftables
show this error:

nft: src/payload.c:76: payload_expr_pctx_update: Assertion `expr->op
== OP_EQ' failed.

This patch moderates the checking by adding another clause like
the expr operation can be non equal, not always equal.

Signed-off-by: Alvaro Neira Ayuso <alvaroneay@gmail.com>
---
 src/payload.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/payload.c b/src/payload.c
index a1785a5..b9ad6f7 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -73,7 +73,7 @@ static void payload_expr_pctx_update(struct proto_ctx *ctx,
 	if (!(left->flags & EXPR_F_PROTOCOL))
 		return;
 
-	assert(expr->op == OP_EQ);
+	assert(expr->op == OP_EQ || expr->op == OP_NEQ);
 	base = ctx->protocol[left->payload.base].desc;
 	desc = proto_find_upper(base, mpz_get_uint32(right->value));
 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[nftables PATCH v2] payload: Relax assert in function payload_expr_pctx_update

[Alvaro Neira Ayuso]

From: Álvaro Neira Ayuso <alvaroneay@gmail.com>

If we add this rule:

sudo nft add rule ip test input ip protocol != icmp

and we try to list the rules in the table test, nftables
show this error:

nft: src/payload.c:76: payload_expr_pctx_update: Assertion `expr->op
== OP_EQ' failed.

This patch relaxes this assert adding another clause like
the expr operation can be non equal, not only equal.

Signed-off-by: Alvaro Neira Ayuso <alvaroneay@gmail.com>
---
[changes in v2]
* I have rewritten the title and the description of this patch

 src/payload.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/payload.c b/src/payload.c
index a1785a5..b9ad6f7 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -73,7 +73,7 @@ static void payload_expr_pctx_update(struct proto_ctx *ctx,
 	if (!(left->flags & EXPR_F_PROTOCOL))
 		return;
 
-	assert(expr->op == OP_EQ);
+	assert(expr->op == OP_EQ || expr->op == OP_NEQ);
 	base = ctx->protocol[left->payload.base].desc;
 	desc = proto_find_upper(base, mpz_get_uint32(right->value));
 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [nftables PATCH v2] payload: Relax assert in function payload_expr_pctx_update

[Patrick McHardy]

On Wed, May 28, 2014 at 11:47:34AM +0200, Alvaro Neira Ayuso wrote:
> From: Álvaro Neira Ayuso <alvaroneay@gmail.com>
> 
> If we add this rule:
> 
> sudo nft add rule ip test input ip protocol != icmp
> 
> and we try to list the rules in the table test, nftables
> show this error:
> 
> nft: src/payload.c:76: payload_expr_pctx_update: Assertion `expr->op
> == OP_EQ' failed.
> 
> This patch relaxes this assert adding another clause like
> the expr operation can be non equal, not only equal.

The function is supposed to update the context based on equality
relations. This change is very wrong, for != icmp it will update
the contect for upper protocol to be ICMP.

The correct fix would be to only invoke the function for equality
relations in payload_match_postprocess().


> Signed-off-by: Alvaro Neira Ayuso <alvaroneay@gmail.com>
> ---
> [changes in v2]
> * I have rewritten the title and the description of this patch
> 
>  src/payload.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/payload.c b/src/payload.c
> index a1785a5..b9ad6f7 100644
> --- a/src/payload.c
> +++ b/src/payload.c
> @@ -73,7 +73,7 @@ static void payload_expr_pctx_update(struct proto_ctx *ctx,
>  	if (!(left->flags & EXPR_F_PROTOCOL))
>  		return;
>  
> -	assert(expr->op == OP_EQ);
> +	assert(expr->op == OP_EQ || expr->op == OP_NEQ);
>  	base = ctx->protocol[left->payload.base].desc;
>  	desc = proto_find_upper(base, mpz_get_uint32(right->value));
>  
> -- 
> 1.7.10.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[nftables PATCH v3] payload: Update the context only in equality relations

[Alvaro Neira Ayuso]

If we add this rule:

sudo nft add rule ip test input ip protocol != icmp

and we try to list the rules in the table test, nftables
show this error:

nft: src/payload.c:76: payload_expr_pctx_update: Assertion `expr->op
== OP_EQ' failed.

This patch change the function payload_match_postprocess for updating
only the context in equality relations case.

Signed-off-by: Alvaro Neira Ayuso <alvaroneay@gmail.com>
---
[changes in v3]
*Changed the patch for updating the context only in case of equality relations
case and keep the assert without changes in the function
payload_expr_pctx_update.

 src/netlink_delinearize.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 479c643..ea33308 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -644,7 +644,8 @@ static void payload_match_postprocess(struct rule_pp_ctx *ctx,
 
 			nexpr = relational_expr_alloc(&expr->location, expr->op,
 						      left, tmp);
-			left->ops->pctx_update(&ctx->pctx, nexpr);
+			if (expr->op == OP_EQ)
+				left->ops->pctx_update(&ctx->pctx, nexpr);
 
 			nstmt = expr_stmt_alloc(&stmt->location, nexpr);
 			list_add_tail(&nstmt->list, &stmt->list);
-- 
1.7.10.4

Re: [nftables PATCH v3] payload: Update the context only in equality relations

[Pablo Neira Ayuso]

On Mon, Jun 02, 2014 at 04:44:11PM +0200, Alvaro Neira Ayuso wrote:
> If we add this rule:
> 
> sudo nft add rule ip test input ip protocol != icmp
> 
> and we try to list the rules in the table test, nftables
> show this error:
> 
> nft: src/payload.c:76: payload_expr_pctx_update: Assertion `expr->op
> == OP_EQ' failed.
> 
> This patch change the function payload_match_postprocess for updating
> only the context in equality relations case.

Applied, thanks.