Re: [PATCH nft v2] evaluate: fix expr_set_context call for shift binops.

[Pablo Neira Ayuso <pablo@netfilter.org>]

Hi Jeremy,

On Tue, Dec 24, 2019 at 11:14:28PM +0000, Jeremy Sowden wrote:
> expr_evaluate_binop calls expr_set_context for shift expressions to set
> the context data-type to `integer`.  This clobbers the byte-order of the
> context, resulting in unexpected conversions to NBO.  For example:
> 
>   $ sudo nft flush ruleset
>   $ sudo nft add table t
>   $ sudo nft add chain t c '{ type filter hook output priority mangle; }'
>   $ sudo nft add rule t c oif lo tcp dport ssh ct mark set '0x10 | 0xe'
>   $ sudo nft add rule t c oif lo tcp dport ssh ct mark set '0xf << 1'
>   $ sudo nft list table t
>   table ip t {
>           chain c {
>                   type filter hook output priority mangle; policy accept;
>                   oif "lo" tcp dport 22 ct mark set 0x0000001e
>                   oif "lo" tcp dport 22 ct mark set 0x1e000000
>           }
>   }
> 
> Replace it with a call to __expr_set_context in order to preserve host
> endianness.
> 
> Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
> ---
>  src/evaluate.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Change-log:
> 
>   In v1, I just dropped the expr_set_context call; however, it is needed
>   to ensure that the right operand has integer type.  Instead, I now
>   change it to __expr_set_context in order to ensure that the byte-order
>   is correct.
> 
> diff --git a/src/evaluate.c b/src/evaluate.c
> index a865902c0fc7..43637e1cf6c8 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -1145,7 +1145,8 @@ static int expr_evaluate_binop(struct eval_ctx *ctx, struct expr **expr)
>  	left = op->left;
>  
>  	if (op->op == OP_LSHIFT || op->op == OP_RSHIFT)
> -		expr_set_context(&ctx->ectx, &integer_type, ctx->ectx.len);
> +		__expr_set_context(&ctx->ectx, &integer_type,
> +				   BYTEORDER_HOST_ENDIAN, ctx->ectx.len, 0);

tests/py spews a few warnings after this patch.