Re: [nf_tables PATCH 3/3] netfilter: nf_tables: add new expression nft_redir

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

On Wed, Oct 15, 2014 at 01:14:12PM +0200, Arturo Borrero Gonzalez wrote:
> On 15 October 2014 12:06, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> >> +int nft_redir_init(const struct nft_ctx *ctx,
> >> +                const struct nft_expr *expr,
> >> +                const struct nlattr * const tb[])
> >> +{
> >> +     struct nft_redir *priv = nft_expr_priv(expr);
> >> +     int err;
> >> +
> >> +     if (tb[NFTA_REDIR_REG_PROTO_MIN]) {
> >> +             priv->sreg_proto_min = ntohl(nla_get_be32(
> >> +                                             tb[NFTA_REDIR_REG_PROTO_MIN]));
> >
> > I prefer this:
> >
> >                 priv->sreg_proto_min =
> >                         ntohl(nla_get_be32(tb[NFTA_REDIR_REG_PROTO_MIN]));
> >
> >
> >> +             err = nft_validate_input_register(priv->sreg_proto_min);
> >> +             if (err < 0)
> >> +                     return err;
> >> +     }
> >
> > No else here? ->sreg_proto_min is left uninitialized.
> >
> 
> Such behaviour is copied from nft_nat.
> I'm not sure to which value I should initialize sreg_proto_min.

This seems fine for redirect as ->sreg_proto_min will be zero. Keep in
mind that we have to support redirect with no port range specified. I
mean, iptables supports -j REDIRECT without arguments.

Regarding nft_nat, not good. I'm going to send a patch to reject
invalid configurations. At least one of the NFTA_NAT_REG_*_MIN needs to
be specified.