Re: [PATCH nf 1/3] netfilter: nft_dynset: fix panic if NFT_SET_HASH is not enabled

[Liping Zhang <zlpnobody@xxxxxxxxx>]

2016-10-25 22:25 GMT+08:00 Liping Zhang <zlpnobody@xxxxxxxxx>:
> Maybe it's better to treat the NFT_SET_EVAL as features, I will send V2 latter:
>
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index b70d3ea..8a39b2a 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -2350,7 +2350,8 @@ nft_select_set_ops(const struct nlattr * const nla[],
>         features = 0;
>         if (nla[NFTA_SET_FLAGS] != NULL) {
>                 features = ntohl(nla_get_be32(nla[NFTA_SET_FLAGS]));
> -               features &= NFT_SET_INTERVAL | NFT_SET_MAP | NFT_SET_TIMEOUT;
> +               features &= NFT_SET_INTERVAL | NFT_SET_MAP | NFT_SET_TIMEOUT |
> +                           NFT_SET_EVAL;
>         }
>
>         bops       = NULL;
> diff --git a/net/netfilter/nft_set_hash.c b/net/netfilter/nft_set_hash.c
> index 3794cb2..328d23c 100644
> --- a/net/netfilter/nft_set_hash.c
> +++ b/net/netfilter/nft_set_hash.c
> @@ -382,7 +382,7 @@ static struct nft_set_ops nft_hash_ops __read_mostly = {
>         .lookup         = nft_hash_lookup,
>         .update         = nft_hash_update,
>         .walk           = nft_hash_walk,
> -       .features       = NFT_SET_MAP | NFT_SET_TIMEOUT,
> +       .features       = NFT_SET_MAP | NFT_SET_TIMEOUT | NFT_SET_EVAL,
>         .owner          = THIS_MODULE,
>  };

Sorry for this noise, the original patch should be fine. :(

After I have a careful look at the implementation of the dynset expr,
it's not appropriate to treat the NFT_SET_EVAL as the features.
The NFTA_DYNSET_EXPR attr is optional, and when it is not
specified, we will report -EINVAL if (set->flags & NFT_SET_EVAL)
is true:

static int nft_dynset_init()
{
    ...
    if (tb[NFTA_DYNSET_EXPR] != NULL) {
        if (!(set->flags & NFT_SET_EVAL))
            return -EINVAL;
        ...
    } else if (set->flags & NFT_SET_EVAL)
        return -EINVAL;

So for dynset, NFT_SET_EVAL is not a must option, but set->ops->update is.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html