diff options
author | Anders K. Pedersen <akp@cohaesio.com> | 2016-11-20 11:38:47 -0500 |
---|---|---|
committer | Pablo Neira Ayuso <pablo@netfilter.org> | 2016-11-24 08:43:34 -0500 |
commit | d3e2a1110cae6ee5eeb1f9a97addf03e974f12e6 (patch) | |
tree | 81cbe225d30cc88bb084a810a1d7d98f7026ac3a /net | |
parent | 7223ecd4669921cb2a709193521967aaa2b06862 (diff) |
netfilter: nf_tables: fix inconsistent element expiration calculation
As Liping Zhang reports, after commit a8b1e36d0d1d ("netfilter: nft_dynset:
fix element timeout for HZ != 1000"), priv->timeout was stored in jiffies,
while set->timeout was stored in milliseconds. This is inconsistent and
incorrect.
Firstly, we already call msecs_to_jiffies in nft_set_elem_init, so
priv->timeout will be converted to jiffies twice.
Secondly, if the user did not specify the NFTA_DYNSET_TIMEOUT attr,
set->timeout will be used, but we forget to call msecs_to_jiffies
when do update elements.
Fix this by using jiffies internally for traditional sets and doing the
conversions to/from msec when interacting with userspace - as dynset
already does.
This is preferable to doing the conversions, when elements are inserted or
updated, because this can happen very frequently on busy dynsets.
Fixes: a8b1e36d0d1d ("netfilter: nft_dynset: fix element timeout for HZ != 1000")
Reported-by: Liping Zhang <zlpnobody@gmail.com>
Signed-off-by: Anders K. Pedersen <akp@cohaesio.com>
Acked-by: Liping Zhang <zlpnobody@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Diffstat (limited to 'net')
-rw-r--r-- | net/netfilter/nf_tables_api.c | 14 |
1 files changed, 9 insertions, 5 deletions
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 026581b04ea8..e5194f6f906c 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c | |||
@@ -2570,7 +2570,8 @@ static int nf_tables_fill_set(struct sk_buff *skb, const struct nft_ctx *ctx, | |||
2570 | } | 2570 | } |
2571 | 2571 | ||
2572 | if (set->timeout && | 2572 | if (set->timeout && |
2573 | nla_put_be64(skb, NFTA_SET_TIMEOUT, cpu_to_be64(set->timeout), | 2573 | nla_put_be64(skb, NFTA_SET_TIMEOUT, |
2574 | cpu_to_be64(jiffies_to_msecs(set->timeout)), | ||
2574 | NFTA_SET_PAD)) | 2575 | NFTA_SET_PAD)) |
2575 | goto nla_put_failure; | 2576 | goto nla_put_failure; |
2576 | if (set->gc_int && | 2577 | if (set->gc_int && |
@@ -2859,7 +2860,8 @@ static int nf_tables_newset(struct net *net, struct sock *nlsk, | |||
2859 | if (nla[NFTA_SET_TIMEOUT] != NULL) { | 2860 | if (nla[NFTA_SET_TIMEOUT] != NULL) { |
2860 | if (!(flags & NFT_SET_TIMEOUT)) | 2861 | if (!(flags & NFT_SET_TIMEOUT)) |
2861 | return -EINVAL; | 2862 | return -EINVAL; |
2862 | timeout = be64_to_cpu(nla_get_be64(nla[NFTA_SET_TIMEOUT])); | 2863 | timeout = msecs_to_jiffies(be64_to_cpu(nla_get_be64( |
2864 | nla[NFTA_SET_TIMEOUT]))); | ||
2863 | } | 2865 | } |
2864 | gc_int = 0; | 2866 | gc_int = 0; |
2865 | if (nla[NFTA_SET_GC_INTERVAL] != NULL) { | 2867 | if (nla[NFTA_SET_GC_INTERVAL] != NULL) { |
@@ -3178,7 +3180,8 @@ static int nf_tables_fill_setelem(struct sk_buff *skb, | |||
3178 | 3180 | ||
3179 | if (nft_set_ext_exists(ext, NFT_SET_EXT_TIMEOUT) && | 3181 | if (nft_set_ext_exists(ext, NFT_SET_EXT_TIMEOUT) && |
3180 | nla_put_be64(skb, NFTA_SET_ELEM_TIMEOUT, | 3182 | nla_put_be64(skb, NFTA_SET_ELEM_TIMEOUT, |
3181 | cpu_to_be64(*nft_set_ext_timeout(ext)), | 3183 | cpu_to_be64(jiffies_to_msecs( |
3184 | *nft_set_ext_timeout(ext))), | ||
3182 | NFTA_SET_ELEM_PAD)) | 3185 | NFTA_SET_ELEM_PAD)) |
3183 | goto nla_put_failure; | 3186 | goto nla_put_failure; |
3184 | 3187 | ||
@@ -3447,7 +3450,7 @@ void *nft_set_elem_init(const struct nft_set *set, | |||
3447 | memcpy(nft_set_ext_data(ext), data, set->dlen); | 3450 | memcpy(nft_set_ext_data(ext), data, set->dlen); |
3448 | if (nft_set_ext_exists(ext, NFT_SET_EXT_EXPIRATION)) | 3451 | if (nft_set_ext_exists(ext, NFT_SET_EXT_EXPIRATION)) |
3449 | *nft_set_ext_expiration(ext) = | 3452 | *nft_set_ext_expiration(ext) = |
3450 | jiffies + msecs_to_jiffies(timeout); | 3453 | jiffies + timeout; |
3451 | if (nft_set_ext_exists(ext, NFT_SET_EXT_TIMEOUT)) | 3454 | if (nft_set_ext_exists(ext, NFT_SET_EXT_TIMEOUT)) |
3452 | *nft_set_ext_timeout(ext) = timeout; | 3455 | *nft_set_ext_timeout(ext) = timeout; |
3453 | 3456 | ||
@@ -3535,7 +3538,8 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set, | |||
3535 | if (nla[NFTA_SET_ELEM_TIMEOUT] != NULL) { | 3538 | if (nla[NFTA_SET_ELEM_TIMEOUT] != NULL) { |
3536 | if (!(set->flags & NFT_SET_TIMEOUT)) | 3539 | if (!(set->flags & NFT_SET_TIMEOUT)) |
3537 | return -EINVAL; | 3540 | return -EINVAL; |
3538 | timeout = be64_to_cpu(nla_get_be64(nla[NFTA_SET_ELEM_TIMEOUT])); | 3541 | timeout = msecs_to_jiffies(be64_to_cpu(nla_get_be64( |
3542 | nla[NFTA_SET_ELEM_TIMEOUT]))); | ||
3539 | } else if (set->flags & NFT_SET_TIMEOUT) { | 3543 | } else if (set->flags & NFT_SET_TIMEOUT) { |
3540 | timeout = set->timeout; | 3544 | timeout = set->timeout; |
3541 | } | 3545 | } |