diff mbox series

[nf-next] netfilter: nf_tables: support timeouts larger than 23 days

Message ID 20180416160449.493-1-fw@strlen.de
State Accepted
Delegated to: Pablo Neira
Headers show
Series [nf-next] netfilter: nf_tables: support timeouts larger than 23 days | expand

Commit Message

Florian Westphal April 16, 2018, 4:04 p.m. UTC 
Marco De Benedetto says:
 I would like to use a timeout of 30 days for elements in a set but it
 seems there is a some kind of problem above 24d20h31m23s.

Fix this by using 'jiffies64' for timeout handling to get same behaviour
on 32 and 64bit systems.

nftables passes timeouts as u64 in milliseconds to the kernel,
but on kernel side we used a mixture of 'long' and jiffies conversions
rather than u64 and jiffies64.

Bugzilla: https://bugzilla.netfilter.org/show_bug.cgi?id=1237
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/net/netfilter/nf_tables.h |  4 ++--
 net/netfilter/nf_tables_api.c     | 50 +++++++++++++++++++++++++++++----------
 2 files changed, 39 insertions(+), 15 deletions(-)

Comments

Jan Engelhardt April 16, 2018, 4:22 p.m. UTC | #1 
On Monday 2018-04-16 18:04, Florian Westphal wrote:
>+	u64 max = (u64)(~((u64)0));
>+	max = div_u64(max, NSEC_PER_MSEC);
>+	if (ms >= max)

Why opencode, is there a problem with UINT64_MAX?
Just this:

	u64 max = div_u64(UINT64_MAX, NSEC_PER_MSEC);
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Florian Westphal April 16, 2018, 4:33 p.m. UTC | #2 
Jan Engelhardt <jengelh@inai.de> wrote:
> 
> On Monday 2018-04-16 18:04, Florian Westphal wrote:
> >+	u64 max = (u64)(~((u64)0));
> >+	max = div_u64(max, NSEC_PER_MSEC);
> >+	if (ms >= max)
> 
> Why opencode, is there a problem with UINT64_MAX?

There is no UINT64_MAX in kernel (some private defines, but
nothing generally available).
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
kernel test robot April 16, 2018, 9:13 p.m. UTC | #3 
Hi Florian,

I love your patch! Perhaps something to improve:

[auto build test WARNING on nf-next/master]

url:    https://github.com/0day-ci/linux/commits/Florian-Westphal/netfilter-nf_tables-support-timeouts-larger-than-23-days/20180417-032146
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   net/netfilter/nf_tables_api.c:1238:31: sparse: incorrect type in return expression (different address spaces) @@    expected struct nft_stats [noderef] <asn:3>* @@    got sn:3>* @@
   net/netfilter/nf_tables_api.c:1238:31:    expected struct nft_stats [noderef] <asn:3>*
   net/netfilter/nf_tables_api.c:1238:31:    got void *
   net/netfilter/nf_tables_api.c:1241:31: sparse: incorrect type in return expression (different address spaces) @@    expected struct nft_stats [noderef] <asn:3>* @@    got sn:3>* @@
   net/netfilter/nf_tables_api.c:1241:31:    expected struct nft_stats [noderef] <asn:3>*
   net/netfilter/nf_tables_api.c:1241:31:    got void *
   net/netfilter/nf_tables_api.c:1245:31: sparse: incorrect type in return expression (different address spaces) @@    expected struct nft_stats [noderef] <asn:3>* @@    got sn:3>* @@
   net/netfilter/nf_tables_api.c:1245:31:    expected struct nft_stats [noderef] <asn:3>*
   net/netfilter/nf_tables_api.c:1245:31:    got void *
   net/netfilter/nf_tables_api.c:1268:28: sparse: cast between address spaces (<asn:3>-><asn:4>)
   net/netfilter/nf_tables_api.c:1268:28: sparse: incompatible types in comparison expression (different address spaces)
   net/netfilter/nf_tables_api.c:1526:23: sparse: incorrect type in assignment (different address spaces) @@    expected struct nft_stats *stats @@    got struct nft_stats struct nft_stats *stats @@
   net/netfilter/nf_tables_api.c:1534:29: sparse: incorrect type in argument 1 (different address spaces) @@    expected void [noderef] <asn:3>*__pdata @@    got [noderef] <asn:3>*__pdata @@
   net/netfilter/nf_tables_api.c:1538:38: sparse: incorrect type in assignment (different address spaces) @@    expected struct nft_stats [noderef] <asn:3>*stats @@    got [noderef] <asn:3>*stats @@
   net/netfilter/nf_tables_api.c:1552:37: sparse: incorrect type in argument 1 (different address spaces) @@    expected void [noderef] <asn:3>*__pdata @@    got [noderef] <asn:3>*__pdata @@
>> net/netfilter/nf_tables_api.c:2789:16: sparse: incorrect type in return expression (different base types) @@    expected unsigned long long @@    got restricted unsigned long long @@
>> net/netfilter/nf_tables_api.c:2839:47: sparse: incorrect type in argument 3 (different base types) @@    expected restricted __be64 [usertype] value @@    got __be64 [usertype] value @@
   net/netfilter/nf_tables_api.c:3477:47: sparse: incorrect type in argument 3 (different base types) @@    expected restricted __be64 [usertype] value @@    got __be64 [usertype] value @@
   net/netfilter/nf_tables_api.c:3491:55: sparse: incorrect type in argument 3 (different base types) @@    expected restricted __be64 [usertype] value @@    got __be64 [usertype] value @@

vim +2789 net/netfilter/nf_tables_api.c

  2784	
  2785	static u64 nf_jiffies64_to_msecs(u64 input)
  2786	{
  2787		u64 ms = jiffies64_to_nsecs(input);
  2788	
> 2789		return cpu_to_be64(div_u64(ms, NSEC_PER_MSEC));
  2790	}
  2791	
  2792	static int nf_tables_fill_set(struct sk_buff *skb, const struct nft_ctx *ctx,
  2793				      const struct nft_set *set, u16 event, u16 flags)
  2794	{
  2795		struct nfgenmsg *nfmsg;
  2796		struct nlmsghdr *nlh;
  2797		struct nlattr *desc;
  2798		u32 portid = ctx->portid;
  2799		u32 seq = ctx->seq;
  2800	
  2801		event = nfnl_msg_type(NFNL_SUBSYS_NFTABLES, event);
  2802		nlh = nlmsg_put(skb, portid, seq, event, sizeof(struct nfgenmsg),
  2803				flags);
  2804		if (nlh == NULL)
  2805			goto nla_put_failure;
  2806	
  2807		nfmsg = nlmsg_data(nlh);
  2808		nfmsg->nfgen_family	= ctx->family;
  2809		nfmsg->version		= NFNETLINK_V0;
  2810		nfmsg->res_id		= htons(ctx->net->nft.base_seq & 0xffff);
  2811	
  2812		if (nla_put_string(skb, NFTA_SET_TABLE, ctx->table->name))
  2813			goto nla_put_failure;
  2814		if (nla_put_string(skb, NFTA_SET_NAME, set->name))
  2815			goto nla_put_failure;
  2816		if (nla_put_be64(skb, NFTA_SET_HANDLE, cpu_to_be64(set->handle),
  2817				 NFTA_SET_PAD))
  2818			goto nla_put_failure;
  2819		if (set->flags != 0)
  2820			if (nla_put_be32(skb, NFTA_SET_FLAGS, htonl(set->flags)))
  2821				goto nla_put_failure;
  2822	
  2823		if (nla_put_be32(skb, NFTA_SET_KEY_TYPE, htonl(set->ktype)))
  2824			goto nla_put_failure;
  2825		if (nla_put_be32(skb, NFTA_SET_KEY_LEN, htonl(set->klen)))
  2826			goto nla_put_failure;
  2827		if (set->flags & NFT_SET_MAP) {
  2828			if (nla_put_be32(skb, NFTA_SET_DATA_TYPE, htonl(set->dtype)))
  2829				goto nla_put_failure;
  2830			if (nla_put_be32(skb, NFTA_SET_DATA_LEN, htonl(set->dlen)))
  2831				goto nla_put_failure;
  2832		}
  2833		if (set->flags & NFT_SET_OBJECT &&
  2834		    nla_put_be32(skb, NFTA_SET_OBJ_TYPE, htonl(set->objtype)))
  2835			goto nla_put_failure;
  2836	
  2837		if (set->timeout &&
  2838		    nla_put_be64(skb, NFTA_SET_TIMEOUT,
> 2839				 nf_jiffies64_to_msecs(set->timeout),
  2840				 NFTA_SET_PAD))
  2841			goto nla_put_failure;
  2842		if (set->gc_int &&
  2843		    nla_put_be32(skb, NFTA_SET_GC_INTERVAL, htonl(set->gc_int)))
  2844			goto nla_put_failure;
  2845	
  2846		if (set->policy != NFT_SET_POL_PERFORMANCE) {
  2847			if (nla_put_be32(skb, NFTA_SET_POLICY, htonl(set->policy)))
  2848				goto nla_put_failure;
  2849		}
  2850	
  2851		if (nla_put(skb, NFTA_SET_USERDATA, set->udlen, set->udata))
  2852			goto nla_put_failure;
  2853	
  2854		desc = nla_nest_start(skb, NFTA_SET_DESC);
  2855		if (desc == NULL)
  2856			goto nla_put_failure;
  2857		if (set->size &&
  2858		    nla_put_be32(skb, NFTA_SET_DESC_SIZE, htonl(set->size)))
  2859			goto nla_put_failure;
  2860		nla_nest_end(skb, desc);
  2861	
  2862		nlmsg_end(skb, nlh);
  2863		return 0;
  2864	
  2865	nla_put_failure:
  2866		nlmsg_trim(skb, nlh);
  2867		return -1;
  2868	}
  2869	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pablo Neira Ayuso April 26, 2018, 10:11 p.m. UTC | #4 
On Mon, Apr 16, 2018 at 06:04:49PM +0200, Florian Westphal wrote:
> Marco De Benedetto says:
>  I would like to use a timeout of 30 days for elements in a set but it
>  seems there is a some kind of problem above 24d20h31m23s.
> 
> Fix this by using 'jiffies64' for timeout handling to get same behaviour
> on 32 and 64bit systems.
> 
> nftables passes timeouts as u64 in milliseconds to the kernel,
> but on kernel side we used a mixture of 'long' and jiffies conversions
> rather than u64 and jiffies64.

Applied, thanks Florian.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox series

Patch

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index cd368d1b8cb8..16ff6fdaa399 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -589,7 +589,7 @@  static inline u64 *nft_set_ext_timeout(const struct nft_set_ext *ext)
 	return nft_set_ext(ext, NFT_SET_EXT_TIMEOUT);
 }
 
-static inline unsigned long *nft_set_ext_expiration(const struct nft_set_ext *ext)
+static inline u64 *nft_set_ext_expiration(const struct nft_set_ext *ext)
 {
 	return nft_set_ext(ext, NFT_SET_EXT_EXPIRATION);
 }
@@ -607,7 +607,7 @@  static inline struct nft_expr *nft_set_ext_expr(const struct nft_set_ext *ext)
 static inline bool nft_set_elem_expired(const struct nft_set_ext *ext)
 {
 	return nft_set_ext_exists(ext, NFT_SET_EXT_EXPIRATION) &&
-	       time_is_before_eq_jiffies(*nft_set_ext_expiration(ext));
+	       time_is_before_eq_jiffies64(*nft_set_ext_expiration(ext));
 }
 
 static inline struct nft_set_ext *nft_set_elem_ext(const struct nft_set *set,
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 9134cc429ad4..72352f53fe03 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -2768,6 +2768,27 @@  static int nf_tables_set_alloc_name(struct nft_ctx *ctx, struct nft_set *set,
 	return 0;
 }
 
+static int nf_msecs_to_jiffies64(const struct nlattr *nla, u64 *result)
+{
+	u64 ms = be64_to_cpu(nla_get_be64(nla));
+	u64 max = (u64)(~((u64)0));
+
+	max = div_u64(max, NSEC_PER_MSEC);
+	if (ms >= max)
+		return -ERANGE;
+
+	ms *= NSEC_PER_MSEC;
+	*result = nsecs_to_jiffies64(ms);
+	return 0;
+}
+
+static u64 nf_jiffies64_to_msecs(u64 input)
+{
+	u64 ms = jiffies64_to_nsecs(input);
+
+	return cpu_to_be64(div_u64(ms, NSEC_PER_MSEC));
+}
+
 static int nf_tables_fill_set(struct sk_buff *skb, const struct nft_ctx *ctx,
 			      const struct nft_set *set, u16 event, u16 flags)
 {
@@ -2815,7 +2836,7 @@  static int nf_tables_fill_set(struct sk_buff *skb, const struct nft_ctx *ctx,
 
 	if (set->timeout &&
 	    nla_put_be64(skb, NFTA_SET_TIMEOUT,
-			 cpu_to_be64(jiffies_to_msecs(set->timeout)),
+			 nf_jiffies64_to_msecs(set->timeout),
 			 NFTA_SET_PAD))
 		goto nla_put_failure;
 	if (set->gc_int &&
@@ -3110,8 +3131,10 @@  static int nf_tables_newset(struct net *net, struct sock *nlsk,
 	if (nla[NFTA_SET_TIMEOUT] != NULL) {
 		if (!(flags & NFT_SET_TIMEOUT))
 			return -EINVAL;
-		timeout = msecs_to_jiffies(be64_to_cpu(nla_get_be64(
-						nla[NFTA_SET_TIMEOUT])));
+
+		err = nf_msecs_to_jiffies64(nla[NFTA_SET_TIMEOUT], &timeout);
+		if (err)
+			return err;
 	}
 	gc_int = 0;
 	if (nla[NFTA_SET_GC_INTERVAL] != NULL) {
@@ -3360,8 +3383,8 @@  const struct nft_set_ext_type nft_set_ext_types[] = {
 		.align	= __alignof__(u64),
 	},
 	[NFT_SET_EXT_EXPIRATION]	= {
-		.len	= sizeof(unsigned long),
-		.align	= __alignof__(unsigned long),
+		.len	= sizeof(u64),
+		.align	= __alignof__(u64),
 	},
 	[NFT_SET_EXT_USERDATA]		= {
 		.len	= sizeof(struct nft_userdata),
@@ -3451,22 +3474,21 @@  static int nf_tables_fill_setelem(struct sk_buff *skb,
 
 	if (nft_set_ext_exists(ext, NFT_SET_EXT_TIMEOUT) &&
 	    nla_put_be64(skb, NFTA_SET_ELEM_TIMEOUT,
-			 cpu_to_be64(jiffies_to_msecs(
-						*nft_set_ext_timeout(ext))),
+			 nf_jiffies64_to_msecs(*nft_set_ext_timeout(ext)),
 			 NFTA_SET_ELEM_PAD))
 		goto nla_put_failure;
 
 	if (nft_set_ext_exists(ext, NFT_SET_EXT_EXPIRATION)) {
-		unsigned long expires, now = jiffies;
+		u64 expires, now = get_jiffies_64();
 
 		expires = *nft_set_ext_expiration(ext);
-		if (time_before(now, expires))
+		if (time_before64(now, expires))
 			expires -= now;
 		else
 			expires = 0;
 
 		if (nla_put_be64(skb, NFTA_SET_ELEM_EXPIRATION,
-				 cpu_to_be64(jiffies_to_msecs(expires)),
+				 nf_jiffies64_to_msecs(expires),
 				 NFTA_SET_ELEM_PAD))
 			goto nla_put_failure;
 	}
@@ -3841,7 +3863,7 @@  void *nft_set_elem_init(const struct nft_set *set,
 		memcpy(nft_set_ext_data(ext), data, set->dlen);
 	if (nft_set_ext_exists(ext, NFT_SET_EXT_EXPIRATION))
 		*nft_set_ext_expiration(ext) =
-			jiffies + timeout;
+			get_jiffies_64() + timeout;
 	if (nft_set_ext_exists(ext, NFT_SET_EXT_TIMEOUT))
 		*nft_set_ext_timeout(ext) = timeout;
 
@@ -3928,8 +3950,10 @@  static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
 	if (nla[NFTA_SET_ELEM_TIMEOUT] != NULL) {
 		if (!(set->flags & NFT_SET_TIMEOUT))
 			return -EINVAL;
-		timeout = msecs_to_jiffies(be64_to_cpu(nla_get_be64(
-					nla[NFTA_SET_ELEM_TIMEOUT])));
+		err = nf_msecs_to_jiffies64(nla[NFTA_SET_ELEM_TIMEOUT],
+					    &timeout);
+		if (err)
+			return err;
 	} else if (set->flags & NFT_SET_TIMEOUT) {
 		timeout = set->timeout;
 	}