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

[Patrick McHardy <kaber@trash.net>]

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