Commit d3e2a111 authored by Anders K. Pedersen's avatar Anders K. Pedersen Committed by Pablo Neira Ayuso

netfilter: nf_tables: fix inconsistent element expiration calculation

As Liping Zhang reports, after commit a8b1e36d ("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: a8b1e36d ("netfilter: nft_dynset: fix element timeout for HZ != 1000")
Reported-by: default avatarLiping Zhang <zlpnobody@gmail.com>
Signed-off-by: default avatarAnders K. Pedersen <akp@cohaesio.com>
Acked-by: default avatarLiping Zhang <zlpnobody@gmail.com>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
parent 7223ecd4
...@@ -313,7 +313,7 @@ void nft_unregister_set(struct nft_set_ops *ops); ...@@ -313,7 +313,7 @@ void nft_unregister_set(struct nft_set_ops *ops);
* @size: maximum set size * @size: maximum set size
* @nelems: number of elements * @nelems: number of elements
* @ndeact: number of deactivated elements queued for removal * @ndeact: number of deactivated elements queued for removal
* @timeout: default timeout value in msecs * @timeout: default timeout value in jiffies
* @gc_int: garbage collection interval in msecs * @gc_int: garbage collection interval in msecs
* @policy: set parameterization (see enum nft_set_policies) * @policy: set parameterization (see enum nft_set_policies)
* @udlen: user data length * @udlen: user data length
......
...@@ -2570,7 +2570,8 @@ static int nf_tables_fill_set(struct sk_buff *skb, const struct nft_ctx *ctx, ...@@ -2570,7 +2570,8 @@ static int nf_tables_fill_set(struct sk_buff *skb, const struct nft_ctx *ctx,
} }
if (set->timeout && if (set->timeout &&
nla_put_be64(skb, NFTA_SET_TIMEOUT, cpu_to_be64(set->timeout), nla_put_be64(skb, NFTA_SET_TIMEOUT,
cpu_to_be64(jiffies_to_msecs(set->timeout)),
NFTA_SET_PAD)) NFTA_SET_PAD))
goto nla_put_failure; goto nla_put_failure;
if (set->gc_int && if (set->gc_int &&
...@@ -2859,7 +2860,8 @@ static int nf_tables_newset(struct net *net, struct sock *nlsk, ...@@ -2859,7 +2860,8 @@ static int nf_tables_newset(struct net *net, struct sock *nlsk,
if (nla[NFTA_SET_TIMEOUT] != NULL) { if (nla[NFTA_SET_TIMEOUT] != NULL) {
if (!(flags & NFT_SET_TIMEOUT)) if (!(flags & NFT_SET_TIMEOUT))
return -EINVAL; return -EINVAL;
timeout = be64_to_cpu(nla_get_be64(nla[NFTA_SET_TIMEOUT])); timeout = msecs_to_jiffies(be64_to_cpu(nla_get_be64(
nla[NFTA_SET_TIMEOUT])));
} }
gc_int = 0; gc_int = 0;
if (nla[NFTA_SET_GC_INTERVAL] != NULL) { if (nla[NFTA_SET_GC_INTERVAL] != NULL) {
...@@ -3178,7 +3180,8 @@ static int nf_tables_fill_setelem(struct sk_buff *skb, ...@@ -3178,7 +3180,8 @@ static int nf_tables_fill_setelem(struct sk_buff *skb,
if (nft_set_ext_exists(ext, NFT_SET_EXT_TIMEOUT) && if (nft_set_ext_exists(ext, NFT_SET_EXT_TIMEOUT) &&
nla_put_be64(skb, NFTA_SET_ELEM_TIMEOUT, nla_put_be64(skb, NFTA_SET_ELEM_TIMEOUT,
cpu_to_be64(*nft_set_ext_timeout(ext)), cpu_to_be64(jiffies_to_msecs(
*nft_set_ext_timeout(ext))),
NFTA_SET_ELEM_PAD)) NFTA_SET_ELEM_PAD))
goto nla_put_failure; goto nla_put_failure;
...@@ -3447,7 +3450,7 @@ void *nft_set_elem_init(const struct nft_set *set, ...@@ -3447,7 +3450,7 @@ void *nft_set_elem_init(const struct nft_set *set,
memcpy(nft_set_ext_data(ext), data, set->dlen); memcpy(nft_set_ext_data(ext), data, set->dlen);
if (nft_set_ext_exists(ext, NFT_SET_EXT_EXPIRATION)) if (nft_set_ext_exists(ext, NFT_SET_EXT_EXPIRATION))
*nft_set_ext_expiration(ext) = *nft_set_ext_expiration(ext) =
jiffies + msecs_to_jiffies(timeout); jiffies + timeout;
if (nft_set_ext_exists(ext, NFT_SET_EXT_TIMEOUT)) if (nft_set_ext_exists(ext, NFT_SET_EXT_TIMEOUT))
*nft_set_ext_timeout(ext) = timeout; *nft_set_ext_timeout(ext) = timeout;
...@@ -3535,7 +3538,8 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set, ...@@ -3535,7 +3538,8 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
if (nla[NFTA_SET_ELEM_TIMEOUT] != NULL) { if (nla[NFTA_SET_ELEM_TIMEOUT] != NULL) {
if (!(set->flags & NFT_SET_TIMEOUT)) if (!(set->flags & NFT_SET_TIMEOUT))
return -EINVAL; return -EINVAL;
timeout = be64_to_cpu(nla_get_be64(nla[NFTA_SET_ELEM_TIMEOUT])); timeout = msecs_to_jiffies(be64_to_cpu(nla_get_be64(
nla[NFTA_SET_ELEM_TIMEOUT])));
} else if (set->flags & NFT_SET_TIMEOUT) { } else if (set->flags & NFT_SET_TIMEOUT) {
timeout = set->timeout; timeout = set->timeout;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment