On Fri, Mar 10, 2023 at 10:12:36AM +0100, Pablo Neira Ayuso wrote:
> 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@xxxxxx>
> > ---
> > 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)));
Will do. Initially I wanted to use mpz_get_si() but it expects a larger
data size. It even works if one casts the return value to int, but I
guess it won't anymore on Big Endian (and is a hack anyway).
> this is in basechain context, so it should be fine.
Yes, indeed. Also the call to evaluate_priority() ensures
chain->priority.expr is as expected.
> > + 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.
I'll rewrite the sentence.
> Thanks for addressing my feedback.
Thanks for the quick review!
