linux-kernel - Re: netfilter: nft_ct: add zone id set support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMuHMdUJ=2+9ko0KEU7rX3rb7vghNj3Xc-HSLSK=-kqQ+CZM6w@mail.gmail.com>
Date:   Thu, 23 Feb 2017 10:54:54 +0100
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Florian Westphal <fw@...len.de>,
        Pablo Neira Ayuso <pablo@...filter.org>
Cc:     Jozsef Kadlecsik <kadlec@...ckhole.kfki.hu>,
        "David S. Miller" <davem@...emloft.net>,
        netfilter-devel@...r.kernel.org, coreteam@...filter.org,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: netfilter: nft_ct: add zone id set support

Hi Florian,

On Wed, Feb 22, 2017 at 8:02 PM, Linux Kernel Mailing List
<linux-kernel@...r.kernel.org> wrote:
> Web:        https://git.kernel.org/torvalds/c/edee4f1e92458299505ff007733f676b00c516a1
> Commit:     edee4f1e92458299505ff007733f676b00c516a1
> Parent:     5c178d81b69f08ca3195427a6ea9a46d9af23127
> Refname:    refs/heads/master
> Author:     Florian Westphal <fw@...len.de>
> AuthorDate: Fri Feb 3 13:35:50 2017 +0100
> Committer:  Pablo Neira Ayuso <pablo@...filter.org>
> CommitDate: Wed Feb 8 14:16:23 2017 +0100
>
>     netfilter: nft_ct: add zone id set support
>
>     zones allow tracking multiple connections sharing identical tuples,
>     this is needed e.g. when tracking distinct vlans with overlapping ip
>     addresses (conntrack is l2 agnostic).
>
>     Thus the zone has to be set before the packet is picked up by the
>     connection tracker.  This is done by means of 'conntrack templates' which
>     are conntrack structures used solely to pass this info from one netfilter
>     hook to the next.
>
>     The iptables CT target instantiates these connection tracking templates
>     once per rule, i.e. the template is fixed/tied to particular zone, can
>     be read-only and therefore be re-used by as many skbs simultaneously as
>     needed.
>
>     We can't follow this model because we want to take the zone id from
>     an sreg at rule eval time so we could e.g. fill in the zone id from
>     the packets vlan id or a e.g. nftables key : value maps.
>
>     To avoid cost of per packet alloc/free of the template, use a percpu
>     template 'scratch' object and use the refcount to detect the (unlikely)
>     case where the template is still attached to another skb (i.e., previous
>     skb was nfqueued ...).
>
>     Signed-off-by: Florian Westphal <fw@...len.de>
>     Signed-off-by: Pablo Neira Ayuso <pablo@...filter.org>

> --- a/net/netfilter/nft_ct.c
> +++ b/net/netfilter/nft_ct.c

> @@ -407,6 +503,7 @@ static int nft_ct_set_init(const struct nft_ctx *ctx,
>         unsigned int len;

With gcc-4.1.2 and -Os:

    net/netfilter/nft_ct.c: In function ‘nft_ct_set_init’:
    net/netfilter/nft_ct.c:503: warning: ‘len’ may be used
uninitialized in this function

>         int err;
>
> +       priv->dir = IP_CT_DIR_MAX;
>         priv->key = ntohl(nla_get_be32(tb[NFTA_CT_KEY]));
>         switch (priv->key) {
>  #ifdef CONFIG_NF_CONNTRACK_MARK
> @@ -426,10 +523,28 @@ static int nft_ct_set_init(const struct nft_ctx *ctx,
>                         return err;
>                 break;
>  #endif
> +#ifdef CONFIG_NF_CONNTRACK_ZONES
> +       case NFT_CT_ZONE:
> +               if (!nft_ct_tmpl_alloc_pcpu())
> +                       return -ENOMEM;
> +               nft_ct_pcpu_template_refcnt++;

Unlike for the other cases of the switch statement, "len" is not initialized
here...

> +               break;
> +#endif
>         default:
>                 return -EOPNOTSUPP;
>         }
>
> +       if (tb[NFTA_CT_DIRECTION]) {
> +               priv->dir = nla_get_u8(tb[NFTA_CT_DIRECTION]);
> +               switch (priv->dir) {
> +               case IP_CT_DIR_ORIGINAL:
> +               case IP_CT_DIR_REPLY:
> +                       break;
> +               default:
> +                       return -EINVAL;
> +               }
> +       }
> +
>         priv->sreg = nft_parse_register(tb[NFTA_CT_SREG]);
>         err = nft_validate_register_load(priv->sreg, len);

... and used here, which may lead to spurious failures of
nft_validate_register_load().

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds