Re: [nft PATCH] Reject invalid chain priority values in user space

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

On Fri, Mar 10, 2023 at 01:13:14AM +0100, Phil Sutter wrote:
> The kernel doesn't accept nat type chains with a priority of -200 or
> below. Catch this and provide a better error message than the kernel's
> EOPNOTSUPP.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  src/evaluate.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/src/evaluate.c b/src/evaluate.c
> index d24f8b66b0de8..af4844c1ef6cc 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -4842,6 +4842,8 @@ static int chain_evaluate(struct eval_ctx *ctx, struct chain *chain)
>  	}
>  
>  	if (chain->flags & CHAIN_F_BASECHAIN) {
> +		int priority;
> +
>  		chain->hook.num = str2hooknum(chain->handle.family,
>  					      chain->hook.name);
>  		if (chain->hook.num == NF_INET_NUMHOOKS)
> @@ -4854,6 +4856,14 @@ static int chain_evaluate(struct eval_ctx *ctx, struct chain *chain)
>  			return __stmt_binary_error(ctx, &chain->priority.loc, NULL,
>  						   "invalid priority expression %s in this context.",
>  						   expr_name(chain->priority.expr));
> +

maybe get this here to declutter the branch?

                mpz_export_data(&priority, chain->priority.expr->value,
                                BYTEORDER_HOST_ENDIAN, sizeof(int)));

this is in basechain context, so it should be fine.

> +		if (!strcmp(chain->type.str, "nat") &&
> +		    (mpz_export_data(&priority, chain->priority.expr->value,
> +				    BYTEORDER_HOST_ENDIAN, sizeof(int))) &&
> +		    priority <= -200)
> +			return __stmt_binary_error(ctx, &chain->priority.loc, NULL,
> +						   "Nat type chains must have a priority value above -200.");
                                                    ^^^

I'd suggest lower case 'nat' which is what the user specifies in the
chain declaration.

Thanks for addressing my feedback.

> +
>  		if (chain->policy) {
>  			expr_set_context(&ctx->ectx, &policy_type,
>  					 NFT_NAME_MAXLEN * BITS_PER_BYTE);
> -- 
> 2.38.0
>