Re: [PATCH v3] netfilter/ipset: replace a strncpy() with strscpy()

[Jozsef Kadlecsik]

Hi,

On Sat, 1 Dec 2018, Qian Cai wrote:

> To make overflows as obvious as possible and to prevent code from blithely
> proceeding with a truncated string. This also has a side-effect to fix a
> compilation warning when using GCC 8.2.1.
> 
> net/netfilter/ipset/ip_set_core.c: In function 'ip_set_sockfn_get':
> net/netfilter/ipset/ip_set_core.c:2027:3: warning: 'strncpy' writing 32
> bytes into a region of size 2 overflows the destination
> [-Wstringop-overflow=]
> 
> Signed-off-by: Qian Cai 

Patch is applied with a slight modification, see below:
 
> Changelog:
> * v2:
> - Released the lock for the error-path as well.
> * v1:
> - Checked the return value.
> 
>  net/netfilter/ipset/ip_set_core.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/net/netfilter/ipset/ip_set_core.c 
> b/net/netfilter/ipset/ip_set_core.c
> index 1577f2f76060..33929fb645b6 100644
> --- a/net/netfilter/ipset/ip_set_core.c
> +++ b/net/netfilter/ipset/ip_set_core.c
> @@ -2024,9 +2024,11 @@ ip_set_sockfn_get(struct sock *sk, int optval, void 
> __user *user, int *len)
>   }
>   nfnl_lock(NFNL_SUBSYS_IPSET);
>   set = ip_set(inst, req_get->set.index);
> - strncpy(req_get->set.name, set ? set->name : "",
> - IPSET_MAXNAMELEN);
> + ret = strscpy(req_get->set.name, set ? set->name : "",
> +   IPSET_MAXNAMELEN);
>   nfnl_unlock(NFNL_SUBSYS_IPSET);
> + if (ret == -E2BIG)

I replaced the condition with

if (ret < 0)

so that it can handle future error codes from strscpy() as well.

> + goto done;
>   goto copy;
>   }
>   default:

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH v2] netfilter: ipset: replace a strncpy() with strscpy()

[Jozsef Kadlecsik]

Hi,

On Mon, 26 Nov 2018, Qian Cai wrote:

> To make overflows as obvious as possible and to prevent code from blithely
> proceeding with a truncated string. This also has a side-effect to fix a
> compilation warning when using GCC 8.2.1.
> 
> net/netfilter/ipset/ip_set_core.c: In function 'ip_set_sockfn_get':
> net/netfilter/ipset/ip_set_core.c:2027:3: warning: 'strncpy' writing 32
> bytes into a region of size 2 overflows the destination
> [-Wstringop-overflow=]
> 
> Signed-off-by: Qian Cai 
> ---
> 
> Changes since v1:
> * Checked the return value.
> 
>  net/netfilter/ipset/ip_set_core.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/net/netfilter/ipset/ip_set_core.c 
> b/net/netfilter/ipset/ip_set_core.c
> index 1577f2f76060..c6f82556f7f2 100644
> --- a/net/netfilter/ipset/ip_set_core.c
> +++ b/net/netfilter/ipset/ip_set_core.c
> @@ -2024,8 +2024,11 @@ ip_set_sockfn_get(struct sock *sk, int optval, void 
> __user *user, int *len)
>   }
>   nfnl_lock(NFNL_SUBSYS_IPSET);
>   set = ip_set(inst, req_get->set.index);
> - strncpy(req_get->set.name, set ? set->name : "",
> - IPSET_MAXNAMELEN);
> + if (strscpy(req_get->set.name, set ? set->name : "",
> + IPSET_MAXNAMELEN) == -E2BIG) {
> + ret = -E2BIG;
> + goto done;
> + }
>   nfnl_unlock(NFNL_SUBSYS_IPSET);
>   goto copy;
>   }

This second version is not OK: the netlink lock is not released in
the error path. Please use an explicit ret = strscpy() assignment first,
then check the error condition and in the error path call 
nfnl_unlock(NFNL_SUBSYS_IPSET) and goto to the final error handling.
Thanks!

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH] netfilter: ipset: replace a strncpy() with strscpy()

[Jozsef Kadlecsik]

Hi,

On Wed, 21 Nov 2018, Qian Cai wrote:

> To make overflows as obvious as possible and to prevent code from blithely
> proceeding with a truncated string. This also has a side-effect to fix a
> compilation warning using GCC 8.2.1.
> 
> net/netfilter/ipset/ip_set_core.c: In function 'ip_set_sockfn_get':
> net/netfilter/ipset/ip_set_core.c:2027:3: warning: 'strncpy' writing 32
> bytes into a region of size 2 overflows the destination
> [-Wstringop-overflow=]

But without checking the return value of strscpy(), I don't see why it's 
better to call strscpy(). Also, with your patch I get the warning:

  CC [M]  /usr/src/git/ipset/ipset/kernel/net/netfilter/xt_set.o
  CC [M]  
/usr/src/git/ipset/ipset/kernel/net/netfilter/ipset/ip_set_core.o
/usr/src/git/ipset/ipset/kernel/net/netfilter/ipset/ip_set_core.c: In 
function 'ip_set_sockfn_get':
/usr/src/git/ipset/ipset/kernel/net/netfilter/ipset/ip_set_core.c:2184:3: 
warning: ignoring return value of 'strscpy', declared with attribute 
warn_unused_result [-Wunused-result]
   strscpy(req_get->set.name, set ? set->name : "",
   ^

So please add the proper checking of the return value.

Best regards,
Jozsef
> Signed-off-by: Qian Cai 
> ---
>  net/netfilter/ipset/ip_set_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/ipset/ip_set_core.c 
> b/net/netfilter/ipset/ip_set_core.c
> index 1577f2f..915aa0d 100644
> --- a/net/netfilter/ipset/ip_set_core.c
> +++ b/net/netfilter/ipset/ip_set_core.c
> @@ -2024,7 +2024,7 @@ static int ip_set_protocol(struct net *net, struct sock 
> *ctnl,
>   }
>   nfnl_lock(NFNL_SUBSYS_IPSET);
>   set = ip_set(inst, req_get->set.index);
> - strncpy(req_get->set.name, set ? set->name : "",
> + strscpy(req_get->set.name, set ? set->name : "",
>   IPSET_MAXNAMELEN);
>   nfnl_unlock(NFNL_SUBSYS_IPSET);
>   goto copy;
> -- 
> 1.8.3.1
> 
> 

-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary

[PATCH v2] netfilter: ipset: Fix calling ip_set() macro at dumping

[Jozsef Kadlecsik]

The ip_set() macro is called when either ip_set_ref_lock held only
or no lock/nfnl mutex is held at dumping. Take this into account
properly. Also, use Pablo's suggestion to use rcu_dereference_raw(),
the ref_netlink protects the set.

Signed-off-by: Jozsef Kadlecsik 
---
 net/netfilter/ipset/ip_set_core.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_core.c 
b/net/netfilter/ipset/ip_set_core.c
index 68db946..1577f2f 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -55,11 +55,15 @@ MODULE_AUTHOR("Jozsef Kadlecsik 
");
 MODULE_DESCRIPTION("core IP set support");
 MODULE_ALIAS_NFNL_SUBSYS(NFNL_SUBSYS_IPSET);
 
-/* When the nfnl mutex is held: */
+/* When the nfnl mutex or ip_set_ref_lock is held: */
 #define ip_set_dereference(p)  \
-   rcu_dereference_protected(p, lockdep_nfnl_is_held(NFNL_SUBSYS_IPSET))
+   rcu_dereference_protected(p,\
+   lockdep_nfnl_is_held(NFNL_SUBSYS_IPSET) || \
+   lockdep_is_held(_set_ref_lock))
 #define ip_set(inst, id)   \
ip_set_dereference((inst)->ip_set_list)[id]
+#define ip_set_ref_netlink(inst,id)\
+   rcu_dereference_raw((inst)->ip_set_list)[id]
 
 /* The set types are implemented in modules and registered set types
  * can be found in ip_set_type_list. Adding/deleting types is
@@ -1251,7 +1255,7 @@ ip_set_dump_done(struct netlink_callback *cb)
struct ip_set_net *inst =
(struct ip_set_net *)cb->args[IPSET_CB_NET];
ip_set_id_t index = (ip_set_id_t)cb->args[IPSET_CB_INDEX];
-   struct ip_set *set = ip_set(inst, index);
+   struct ip_set *set = ip_set_ref_netlink(inst, index);
 
if (set->variant->uref)
set->variant->uref(set, cb, false);
@@ -1440,7 +1444,7 @@ ip_set_dump_start(struct sk_buff *skb, struct 
netlink_callback *cb)
 release_refcount:
/* If there was an error or set is done, release set */
if (ret || !cb->args[IPSET_CB_ARG0]) {
-   set = ip_set(inst, index);
+   set = ip_set_ref_netlink(inst, index);
if (set->variant->uref)
set->variant->uref(set, cb, false);
pr_debug("release set %s\n", set->name);
-- 
2.1.4

[ANNOUNCE] ipset 7.0 released

[Jozsef Kadlecsik]

Hi,

I'm happy to announce ipset 7.0 which - besides of a couple fixes and 
corrections - brings a new internal protocol version between the kernel
and userspace.

The system is fully backward compatible:

- the new kernel modules work fine with any older ipset userspace binary,
- the new ipset binary works fine with any older ip_set* kernel modules.

The new protocol version was required to be introduced in order to support 
two new functions and the extendend LIST operation, which makes possible 
to run ipset in every case entirely over netlink, without the need to use 
getsockopt().

In the userspace part the library was reworked: from now on ipset can 
fully be embedded, python/perl/etc. binding library constructed and run 
ipset commands without calling the binary itself. The libipset(3) manpage 
describes the usage of the toplevel functions needed to issue ipset 
commands.

Userspace changes:
  - Introduction of new commands and protocol version 7, updated
kernel include files
  - Add compatibility support for async in pernet_operations
  - Use more robust awk patterns to check for backward compatibility
  - Prepare the ipset tool to handle multiple protocol version
  - Fix warning message handling
  - Correct to test null valued entry in hash:net6,port,net6 test
  - Library reworked to support embedding ipset completely
  - Add compatibility to support kvcalloc()
  - Validate string type attributes in attr2data() (Stefano Brivio)
  - manpage: Add comment about matching on destination MAC address
(Stefano Brivio)
  - Add compatibility to support is_zero_ether_addr()
  - Fix use-after-free in ipset_parse_name_compat() (Stefano Brivio)
  - Fix leak in build_argv() on line parsing error (Stefano Brivio)
  - Simplify return statement in ipset_mnl_query() (Stefano Brivio)
  - tests/check_klog.sh: Try dmesg too, don't let shell terminate script
(Stefano Brivio)
Kernel part changes:
  - Introduction of new commands and protocol version 7
  - License cleanup: add SPDX license identifier to uapi header files with
no license (Greg Kroah-Hartman)
  - net: Convert ip_set_net_ops (Kirill Tkhai)
  - netfilter: Replace spin_is_locked() with lockdep (Lance Roy)
  - Fix calling ip_set() macro at dumping
  - Correct rcu_dereference() call in ip_set_put_comment()
  - netfilter: ipset: fix ip_set_list allocation failure (Andrey Ryabinin)
  - ipset: Make invalid MAC address checks consisten (Stefano Brivio)
  - ipset: Allow matching on destination MAC address for mac and ipmac 
sets (Stefano Brivio)
  - netfilter: ipset: actually allow allowable CIDR 0 in hash:net,port,net
(Eric Westbrook)
  - ipset: list:set: Decrease refcount synchronously on deletion and 
replace (Stefano Brivio)
  - netfilter: ipset: forbid family for hash:mac sets (Florent Fourcot)
  - Limit max timeout value to (UINT_MAX >> 1)/MSEC_PER_SEC
  - List timing out entries with "timeout 1" instead of zero timeout value
(Fixes bugzilla #1258)
  - netfilter: xt_set: Check hook mask correctly (Serhey Popovych)

You can download the source code of ipset from:
http://ipset.netfilter.org
ftp://ftp.netfilter.org/pub/ipset/
git://git.netfilter.org/ipset.git

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary

[PATCH 0/3] ipset patches for nf-next

[Jozsef Kadlecsik]

Hi Pablo,

Please consider to pull the next patches for nf-next:

- Introduction of new commands and thus protocol version 7. The
  new commands makes possible to eliminate the getsockopt interface
  of ipset and use solely netlink to communicate with the kernel.
  Due to the strict attribute checking both in user/kernel space,
  a new protocol number was introduced. Both the kernel/userspace is
  fully backward compatible.
- Make invalid MAC address checks consisten, from Stefano Brivio.
  The patch depends on the next one.
- Allow matching on destination MAC address for mac and ipmac sets,
  also from Stefano Brivio.

Best regards,
Jozsef

The following changes since commit af510ebd8913bee016492832f532ed919b51c09c:

  Revert "netfilter: xt_quota: fix the behavior of xt_quota module" (2018-10-19 
14:00:34 +0200)

are available in the git repository at:

  git://blackhole.kfki.hu/nf-next 23c42a403a9cfdbad6

for you to fetch changes up to 23c42a403a9cfdbad6004a556c927be7dd61a8ee:

  netfilter: ipset: Introduction of new commands and protocol version 7 
(2018-10-27 15:49:09 +0200)

--------
Jozsef Kadlecsik (1):
  netfilter: ipset: Introduction of new commands and protocol version 7

Stefano Brivio (2):
  netfilter: ipset: Allow matching on destination MAC address for mac and 
ipmac sets
  netfilter: ipset: Make invalid MAC address checks consistent

 include/linux/netfilter/ipset/ip_set.h  |   2 +-
 include/uapi/linux/netfilter/ipset/ip_set.h |  19 ++--
 net/netfilter/ipset/ip_set_bitmap_ipmac.c   |  13 ++-
 net/netfilter/ipset/ip_set_core.c   | 164 +---
 net/netfilter/ipset/ip_set_hash_ipmac.c |  27 ++---
 net/netfilter/ipset/ip_set_hash_mac.c   |  10 +-
 6 files changed, 187 insertions(+), 48 deletions(-)

[PATCH 2/3] netfilter: ipset: Make invalid MAC address checks consistent

[Jozsef Kadlecsik]

From: Stefano Brivio 

Set types bitmap:ipmac and hash:ipmac check that MAC addresses
are not all zeroes.

Introduce one missing check, and make the remaining ones
consistent, using is_zero_ether_addr() instead of comparing
against an array containing zeroes.

This was already done for hash:mac sets in commit 26c97c5d8dac
("netfilter: ipset: Use is_zero_ether_addr instead of static and
memcmp").

Signed-off-by: Stefano Brivio 
Signed-off-by: Jozsef Kadlecsik 
---
 net/netfilter/ipset/ip_set_bitmap_ipmac.c |  3 +++
 net/netfilter/ipset/ip_set_hash_ipmac.c   | 11 ---
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_bitmap_ipmac.c 
b/net/netfilter/ipset/ip_set_bitmap_ipmac.c
index 13ade57..98fc 100644
--- a/net/netfilter/ipset/ip_set_bitmap_ipmac.c
+++ b/net/netfilter/ipset/ip_set_bitmap_ipmac.c
@@ -235,6 +235,9 @@ bitmap_ipmac_kadt(struct ip_set *set, const struct sk_buff 
*skb,
else
ether_addr_copy(e.ether, eth_hdr(skb)->h_dest);
 
+   if (is_zero_ether_addr(e.ether))
+   return -EINVAL;
+
return adtfn(set, , , >ext, opt->cmdflags);
 }
 
diff --git a/net/netfilter/ipset/ip_set_hash_ipmac.c 
b/net/netfilter/ipset/ip_set_hash_ipmac.c
index fd87de3..c830c68 100644
--- a/net/netfilter/ipset/ip_set_hash_ipmac.c
+++ b/net/netfilter/ipset/ip_set_hash_ipmac.c
@@ -36,9 +36,6 @@ MODULE_ALIAS("ip_set_hash:ip,mac");
 /* Type specific function prefix */
 #define HTYPE  hash_ipmac
 
-/* Zero valued element is not supported */
-static const unsigned char invalid_ether[ETH_ALEN] = { 0 };
-
 /* IPv4 variant */
 
 /* Member elements */
@@ -108,7 +105,7 @@ hash_ipmac4_kadt(struct ip_set *set, const struct sk_buff 
*skb,
else
ether_addr_copy(e.ether, eth_hdr(skb)->h_dest);
 
-   if (ether_addr_equal(e.ether, invalid_ether))
+   if (is_zero_ether_addr(e.ether))
return -EINVAL;
 
ip4addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, );
@@ -144,7 +141,7 @@ hash_ipmac4_uadt(struct ip_set *set, struct nlattr *tb[],
if (ret)
return ret;
memcpy(e.ether, nla_data(tb[IPSET_ATTR_ETHER]), ETH_ALEN);
-   if (ether_addr_equal(e.ether, invalid_ether))
+   if (is_zero_ether_addr(e.ether))
return -IPSET_ERR_HASH_ELEM;
 
return adtfn(set, , , , flags);
@@ -224,7 +221,7 @@ hash_ipmac6_kadt(struct ip_set *set, const struct sk_buff 
*skb,
else
ether_addr_copy(e.ether, eth_hdr(skb)->h_dest);
 
-   if (ether_addr_equal(e.ether, invalid_ether))
+   if (is_zero_ether_addr(e.ether))
return -EINVAL;
 
ip6addrptr(skb, opt->flags & IPSET_DIM_ONE_SRC, );
@@ -264,7 +261,7 @@ hash_ipmac6_uadt(struct ip_set *set, struct nlattr *tb[],
return ret;
 
memcpy(e.ether, nla_data(tb[IPSET_ATTR_ETHER]), ETH_ALEN);
-   if (ether_addr_equal(e.ether, invalid_ether))
+   if (is_zero_ether_addr(e.ether))
return -IPSET_ERR_HASH_ELEM;
 
return adtfn(set, , , , flags);
-- 
2.1.4

[PATCH 1/3] netfilter: ipset: Allow matching on destination MAC address for mac and ipmac sets

[Jozsef Kadlecsik]

From: Stefano Brivio 

There doesn't seem to be any reason to restrict MAC address
matching to source MAC addresses in set types bitmap:ipmac,
hash:ipmac and hash:mac. With this patch, and this setup:

  ip netns add A
  ip link add veth1 type veth peer name veth2 netns A
  ip addr add 192.0.2.1/24 dev veth1
  ip -net A addr add 192.0.2.2/24 dev veth2
  ip link set veth1 up
  ip -net A link set veth2 up

  ip netns exec A ipset create test hash:mac
  dst=$(ip netns exec A cat /sys/class/net/veth2/address)
  ip netns exec A ipset add test ${dst}
  ip netns exec A iptables -P INPUT DROP
  ip netns exec A iptables -I INPUT -m set --match-set test dst -j ACCEPT

ipset will match packets based on destination MAC address:

  # ping -c1 192.0.2.2 >/dev/null
  # echo $?
  0

Reported-by: Yi Chen 
Signed-off-by: Stefano Brivio 
Signed-off-by: Jozsef Kadlecsik 
---
 net/netfilter/ipset/ip_set_bitmap_ipmac.c | 10 +-
 net/netfilter/ipset/ip_set_hash_ipmac.c   | 16 ++--
 net/netfilter/ipset/ip_set_hash_mac.c | 10 +-
 3 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_bitmap_ipmac.c 
b/net/netfilter/ipset/ip_set_bitmap_ipmac.c
index c00b6a2..13ade57 100644
--- a/net/netfilter/ipset/ip_set_bitmap_ipmac.c
+++ b/net/netfilter/ipset/ip_set_bitmap_ipmac.c
@@ -219,10 +219,6 @@ bitmap_ipmac_kadt(struct ip_set *set, const struct sk_buff 
*skb,
struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
u32 ip;
 
-   /* MAC can be src only */
-   if (!(opt->flags & IPSET_DIM_TWO_SRC))
-   return 0;
-
ip = ntohl(ip4addr(skb, opt->flags & IPSET_DIM_ONE_SRC));
if (ip < map->first_ip || ip > map->last_ip)
return -IPSET_ERR_BITMAP_RANGE;
@@ -233,7 +229,11 @@ bitmap_ipmac_kadt(struct ip_set *set, const struct sk_buff 
*skb,
return -EINVAL;
 
e.id = ip_to_id(map, ip);
-   memcpy(e.ether, eth_hdr(skb)->h_source, ETH_ALEN);
+
+   if (opt->flags & IPSET_DIM_ONE_SRC)
+   ether_addr_copy(e.ether, eth_hdr(skb)->h_source);
+   else
+   ether_addr_copy(e.ether, eth_hdr(skb)->h_dest);
 
return adtfn(set, , , >ext, opt->cmdflags);
 }
diff --git a/net/netfilter/ipset/ip_set_hash_ipmac.c 
b/net/netfilter/ipset/ip_set_hash_ipmac.c
index 1ab5ed2..fd87de3 100644
--- a/net/netfilter/ipset/ip_set_hash_ipmac.c
+++ b/net/netfilter/ipset/ip_set_hash_ipmac.c
@@ -103,7 +103,11 @@ hash_ipmac4_kadt(struct ip_set *set, const struct sk_buff 
*skb,
(skb_mac_header(skb) + ETH_HLEN) > skb->data)
return -EINVAL;
 
-   memcpy(e.ether, eth_hdr(skb)->h_source, ETH_ALEN);
+   if (opt->flags & IPSET_DIM_ONE_SRC)
+   ether_addr_copy(e.ether, eth_hdr(skb)->h_source);
+   else
+   ether_addr_copy(e.ether, eth_hdr(skb)->h_dest);
+
if (ether_addr_equal(e.ether, invalid_ether))
return -EINVAL;
 
@@ -211,15 +215,15 @@ hash_ipmac6_kadt(struct ip_set *set, const struct sk_buff 
*skb,
};
struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
 
-/* MAC can be src only */
-   if (!(opt->flags & IPSET_DIM_TWO_SRC))
-   return 0;
-
if (skb_mac_header(skb) < skb->head ||
(skb_mac_header(skb) + ETH_HLEN) > skb->data)
return -EINVAL;
 
-   memcpy(e.ether, eth_hdr(skb)->h_source, ETH_ALEN);
+   if (opt->flags & IPSET_DIM_ONE_SRC)
+   ether_addr_copy(e.ether, eth_hdr(skb)->h_source);
+   else
+   ether_addr_copy(e.ether, eth_hdr(skb)->h_dest);
+
if (ether_addr_equal(e.ether, invalid_ether))
return -EINVAL;
 
diff --git a/net/netfilter/ipset/ip_set_hash_mac.c 
b/net/netfilter/ipset/ip_set_hash_mac.c
index f9d5a2a..4fe5f24 100644
--- a/net/netfilter/ipset/ip_set_hash_mac.c
+++ b/net/netfilter/ipset/ip_set_hash_mac.c
@@ -81,15 +81,15 @@ hash_mac4_kadt(struct ip_set *set, const struct sk_buff 
*skb,
struct hash_mac4_elem e = { { .foo[0] = 0, .foo[1] = 0 } };
struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
 
-/* MAC can be src only */
-   if (!(opt->flags & IPSET_DIM_ONE_SRC))
-   return 0;
-
if (skb_mac_header(skb) < skb->head ||
(skb_mac_header(skb) + ETH_HLEN) > skb->data)
return -EINVAL;
 
-   ether_addr_copy(e.ether, eth_hdr(skb)->h_source);
+   if (opt->flags & IPSET_DIM_ONE_SRC)
+   ether_addr_copy(e.ether, eth_hdr(skb)->h_source);
+   else
+   ether_addr_copy(e.ether, eth_hdr(skb)->h_dest);
+
if (is_zero_ether_addr(e.ether))
return -EINVAL;
return adtfn(set, , , >ext, opt->cmdflags);
-- 
2.1.4

[PATCH 3/3] netfilter: ipset: Introduction of new commands and protocol version 7

[Jozsef Kadlecsik]

Two new commands (IPSET_CMD_GET_BYNAME, IPSET_CMD_GET_BYINDEX) are
introduced. The new commands makes possible to eliminate the getsockopt
operation (in iptables set/SET match/target) and thus use only netlink
communication between userspace and kernel for ipset. With the new
protocol version, userspace can exactly know which functionality is
supported by the running kernel.

Both the kernel and userspace is fully backward compatible.

Signed-off-by: Jozsef Kadlecsik 
---
 include/linux/netfilter/ipset/ip_set.h  |   2 +-
 include/uapi/linux/netfilter/ipset/ip_set.h |  19 ++--
 net/netfilter/ipset/ip_set_core.c   | 164 +---
 3 files changed, 160 insertions(+), 25 deletions(-)

diff --git a/include/linux/netfilter/ipset/ip_set.h 
b/include/linux/netfilter/ipset/ip_set.h
index 34fc80f..c4ce074 100644
--- a/include/linux/netfilter/ipset/ip_set.h
+++ b/include/linux/netfilter/ipset/ip_set.h
@@ -303,11 +303,11 @@ ip_set_put_flags(struct sk_buff *skb, struct ip_set *set)
 /* Netlink CB args */
 enum {
IPSET_CB_NET = 0,   /* net namespace */
+   IPSET_CB_PROTO, /* ipset protocol */
IPSET_CB_DUMP,  /* dump single set/all sets */
IPSET_CB_INDEX, /* set index */
IPSET_CB_PRIVATE,   /* set private data */
IPSET_CB_ARG0,  /* type specific */
-   IPSET_CB_ARG1,
 };
 
 /* register and unregister set references */
diff --git a/include/uapi/linux/netfilter/ipset/ip_set.h 
b/include/uapi/linux/netfilter/ipset/ip_set.h
index 60236f6..ea69ca2 100644
--- a/include/uapi/linux/netfilter/ipset/ip_set.h
+++ b/include/uapi/linux/netfilter/ipset/ip_set.h
@@ -13,8 +13,9 @@
 
 #include 
 
-/* The protocol version */
-#define IPSET_PROTOCOL 6
+/* The protocol versions */
+#define IPSET_PROTOCOL 7
+#define IPSET_PROTOCOL_MIN 6
 
 /* The max length of strings including NUL: set and type identifiers */
 #define IPSET_MAXNAMELEN   32
@@ -38,17 +39,19 @@ enum ipset_cmd {
IPSET_CMD_TEST, /* 11: Test an element in a set */
IPSET_CMD_HEADER,   /* 12: Get set header data only */
IPSET_CMD_TYPE, /* 13: Get set type */
+   IPSET_CMD_GET_BYNAME,   /* 14: Get set index by name */
+   IPSET_CMD_GET_BYINDEX,  /* 15: Get set name by index */
IPSET_MSG_MAX,  /* Netlink message commands */
 
/* Commands in userspace: */
-   IPSET_CMD_RESTORE = IPSET_MSG_MAX, /* 14: Enter restore mode */
-   IPSET_CMD_HELP, /* 15: Get help */
-   IPSET_CMD_VERSION,  /* 16: Get program version */
-   IPSET_CMD_QUIT, /* 17: Quit from interactive mode */
+   IPSET_CMD_RESTORE = IPSET_MSG_MAX, /* 16: Enter restore mode */
+   IPSET_CMD_HELP, /* 17: Get help */
+   IPSET_CMD_VERSION,  /* 18: Get program version */
+   IPSET_CMD_QUIT, /* 19: Quit from interactive mode */
 
IPSET_CMD_MAX,
 
-   IPSET_CMD_COMMIT = IPSET_CMD_MAX, /* 18: Commit buffered commands */
+   IPSET_CMD_COMMIT = IPSET_CMD_MAX, /* 20: Commit buffered commands */
 };
 
 /* Attributes at command level */
@@ -66,6 +69,7 @@ enum {
IPSET_ATTR_LINENO,  /* 9: Restore lineno */
IPSET_ATTR_PROTOCOL_MIN, /* 10: Minimal supported version number */
IPSET_ATTR_REVISION_MIN = IPSET_ATTR_PROTOCOL_MIN, /* type rev min */
+   IPSET_ATTR_INDEX,   /* 11: Kernel index of set */
__IPSET_ATTR_CMD_MAX,
 };
 #define IPSET_ATTR_CMD_MAX (__IPSET_ATTR_CMD_MAX - 1)
@@ -223,6 +227,7 @@ enum ipset_adt {
 
 /* Sets are identified by an index in kernel space. Tweak with ip_set_id_t
  * and IPSET_INVALID_ID if you want to increase the max number of sets.
+ * Also, IPSET_ATTR_INDEX must be changed.
  */
 typedef __u16 ip_set_id_t;
 
diff --git a/net/netfilter/ipset/ip_set_core.c 
b/net/netfilter/ipset/ip_set_core.c
index bc4bd247..847f764 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -768,11 +768,21 @@ EXPORT_SYMBOL_GPL(ip_set_nfnl_put);
  * The commands are serialized by the nfnl mutex.
  */
 
+static inline u8 protocol(const struct nlattr * const tb[])
+{
+   return nla_get_u8(tb[IPSET_ATTR_PROTOCOL]);
+}
+
 static inline bool
 protocol_failed(const struct nlattr * const tb[])
 {
-   return !tb[IPSET_ATTR_PROTOCOL] ||
-  nla_get_u8(tb[IPSET_ATTR_PROTOCOL]) != IPSET_PROTOCOL;
+   return !tb[IPSET_ATTR_PROTOCOL] || protocol(tb) != IPSET_PROTOCOL;
+}
+
+static inline bool
+protocol_min_failed(const struct nlattr * const tb[])
+{
+   return !tb[IPSET_ATTR_PROTOCOL] || protocol(tb) < IPSET_PROTOCOL_MIN;
 }
 
 static inline u32
@@ -886,7 +896,7 @@ static int ip_set_create(struct net *net, struct sock *ctnl,
u32 flags = flag_exist(nlh);
int ret = 0;
 
-   if (unlikely(protocol_failed(attr) ||
+   if (unlikely(protocol_min_failed(attr) ||
 !attr[IPSET_ATTR_SETN

[PATCH 3/5] netfilter: ipset: fix ip_set_list allocation failure

[Jozsef Kadlecsik]

From: Andrey Ryabinin 

ip_set_create() and ip_set_net_init() attempt to allocate physically
contiguous memory for ip_set_list. If memory is fragmented, the
allocations could easily fail:

vzctl: page allocation failure: order:7, mode:0xc0d0

Call Trace:
 dump_stack+0x19/0x1b
 warn_alloc_failed+0x110/0x180
 __alloc_pages_nodemask+0x7bf/0xc60
 alloc_pages_current+0x98/0x110
 kmalloc_order+0x18/0x40
 kmalloc_order_trace+0x26/0xa0
 __kmalloc+0x279/0x290
 ip_set_net_init+0x4b/0x90 [ip_set]
 ops_init+0x3b/0xb0
 setup_net+0xbb/0x170
 copy_net_ns+0xf1/0x1c0
 create_new_namespaces+0xf9/0x180
 copy_namespaces+0x8e/0xd0
 copy_process+0xb61/0x1a00
 do_fork+0x91/0x320

Use kvcalloc() to fallback to 0-order allocations if high order
page isn't available.

Signed-off-by: Andrey Ryabinin 
Signed-off-by: Jozsef Kadlecsik 
---
 net/netfilter/ipset/ip_set_core.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_core.c 
b/net/netfilter/ipset/ip_set_core.c
index fa15a83..68db946 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -960,7 +960,7 @@ static int ip_set_create(struct net *net, struct sock *ctnl,
/* Wraparound */
goto cleanup;
 
-   list = kcalloc(i, sizeof(struct ip_set *), GFP_KERNEL);
+   list = kvcalloc(i, sizeof(struct ip_set *), GFP_KERNEL);
if (!list)
goto cleanup;
/* nfnl mutex is held, both lists are valid */
@@ -972,7 +972,7 @@ static int ip_set_create(struct net *net, struct sock *ctnl,
/* Use new list */
index = inst->ip_set_max;
inst->ip_set_max = i;
-   kfree(tmp);
+   kvfree(tmp);
ret = 0;
} else if (ret) {
goto cleanup;
@@ -2058,7 +2058,7 @@ ip_set_net_init(struct net *net)
if (inst->ip_set_max >= IPSET_INVALID_ID)
inst->ip_set_max = IPSET_INVALID_ID - 1;
 
-   list = kcalloc(inst->ip_set_max, sizeof(struct ip_set *), GFP_KERNEL);
+   list = kvcalloc(inst->ip_set_max, sizeof(struct ip_set *), GFP_KERNEL);
if (!list)
return -ENOMEM;
inst->is_deleted = false;
@@ -2086,7 +2086,7 @@ ip_set_net_exit(struct net *net)
}
}
nfnl_unlock(NFNL_SUBSYS_IPSET);
-   kfree(rcu_dereference_protected(inst->ip_set_list, 1));
+   kvfree(rcu_dereference_protected(inst->ip_set_list, 1));
 }
 
 static struct pernet_operations ip_set_net_ops = {
-- 
2.1.4

[PATCH 1/5] netfilter: ipset: list:set: Decrease refcount synchronously on deletion and replace

[Jozsef Kadlecsik]

From: Stefano Brivio 

Commit 45040978c899 ("netfilter: ipset: Fix set:list type crash
when flush/dump set in parallel") postponed decreasing set
reference counters to the RCU callback.

An 'ipset del' command can terminate before the RCU grace period
is elapsed, and if sets are listed before then, the reference
counter shown in userspace will be wrong:

 # ipset create h hash:ip; ipset create l list:set; ipset add l
 # ipset del l h; ipset list h
 Name: h
 Type: hash:ip
 Revision: 4
 Header: family inet hashsize 1024 maxelem 65536
 Size in memory: 88
 References: 1
 Number of entries: 0
 Members:
 # sleep 1; ipset list h
 Name: h
 Type: hash:ip
 Revision: 4
 Header: family inet hashsize 1024 maxelem 65536
 Size in memory: 88
 References: 0
 Number of entries: 0
 Members:

Fix this by making the reference count update synchronous again.

As a result, when sets are listed, ip_set_name_byindex() might
now fetch a set whose reference count is already zero. Instead
of relying on the reference count to protect against concurrent
set renaming, grab ip_set_ref_lock as reader and copy the name,
while holding the same lock in ip_set_rename() as writer
instead.

Reported-by: Li Shuang 
Fixes: 45040978c899 ("netfilter: ipset: Fix set:list type crash when flush/dump 
set in parallel")
Signed-off-by: Stefano Brivio 
Signed-off-by: Jozsef Kadlecsik 
---
 include/linux/netfilter/ipset/ip_set.h |  2 +-
 net/netfilter/ipset/ip_set_core.c  | 23 +++
 net/netfilter/ipset/ip_set_list_set.c  | 17 +++--
 3 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/include/linux/netfilter/ipset/ip_set.h 
b/include/linux/netfilter/ipset/ip_set.h
index 34fc80f..1d100ef 100644
--- a/include/linux/netfilter/ipset/ip_set.h
+++ b/include/linux/netfilter/ipset/ip_set.h
@@ -314,7 +314,7 @@ enum {
 extern ip_set_id_t ip_set_get_byname(struct net *net,
 const char *name, struct ip_set **set);
 extern void ip_set_put_byindex(struct net *net, ip_set_id_t index);
-extern const char *ip_set_name_byindex(struct net *net, ip_set_id_t index);
+extern void ip_set_name_byindex(struct net *net, ip_set_id_t index, char 
*name);
 extern ip_set_id_t ip_set_nfnl_get_byindex(struct net *net, ip_set_id_t index);
 extern void ip_set_nfnl_put(struct net *net, ip_set_id_t index);
 
diff --git a/net/netfilter/ipset/ip_set_core.c 
b/net/netfilter/ipset/ip_set_core.c
index bc4bd247..fa15a83 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -693,21 +693,20 @@ ip_set_put_byindex(struct net *net, ip_set_id_t index)
 EXPORT_SYMBOL_GPL(ip_set_put_byindex);
 
 /* Get the name of a set behind a set index.
- * We assume the set is referenced, so it does exist and
- * can't be destroyed. The set cannot be renamed due to
- * the referencing either.
- *
+ * Set itself is protected by RCU, but its name isn't: to protect against
+ * renaming, grab ip_set_ref_lock as reader (see ip_set_rename()) and copy the
+ * name.
  */
-const char *
-ip_set_name_byindex(struct net *net, ip_set_id_t index)
+void
+ip_set_name_byindex(struct net *net, ip_set_id_t index, char *name)
 {
-   const struct ip_set *set = ip_set_rcu_get(net, index);
+   struct ip_set *set = ip_set_rcu_get(net, index);
 
BUG_ON(!set);
-   BUG_ON(set->ref == 0);
 
-   /* Referenced, so it's safe */
-   return set->name;
+   read_lock_bh(_set_ref_lock);
+   strncpy(name, set->name, IPSET_MAXNAMELEN);
+   read_unlock_bh(_set_ref_lock);
 }
 EXPORT_SYMBOL_GPL(ip_set_name_byindex);
 
@@ -1153,7 +1152,7 @@ static int ip_set_rename(struct net *net, struct sock 
*ctnl,
if (!set)
return -ENOENT;
 
-   read_lock_bh(_set_ref_lock);
+   write_lock_bh(_set_ref_lock);
if (set->ref != 0) {
ret = -IPSET_ERR_REFERENCED;
goto out;
@@ -1170,7 +1169,7 @@ static int ip_set_rename(struct net *net, struct sock 
*ctnl,
strncpy(set->name, name2, IPSET_MAXNAMELEN);
 
 out:
-   read_unlock_bh(_set_ref_lock);
+   write_unlock_bh(_set_ref_lock);
return ret;
 }
 
diff --git a/net/netfilter/ipset/ip_set_list_set.c 
b/net/netfilter/ipset/ip_set_list_set.c
index 072a658..4eef55d 100644
--- a/net/netfilter/ipset/ip_set_list_set.c
+++ b/net/netfilter/ipset/ip_set_list_set.c
@@ -148,9 +148,7 @@ __list_set_del_rcu(struct rcu_head * rcu)
 {
struct set_elem *e = container_of(rcu, struct set_elem, rcu);
struct ip_set *set = e->set;
-   struct list_set *map = set->data;
 
-   ip_set_put_byindex(map->net, e->id);
ip_set_ext_destroy(set, e);
kfree(e);
 }
@@ -158,15 +156,21 @@ __list_set_del_rcu(struct rcu_head * rcu)
 static inline void
 list_set_del(struct ip_set *set, struct set_elem *e)
 {
+   struct list_set *map = set->data;
+
set->elements--;
list_del_rcu(>list);
+   ip

[PATCH 4/5] netfilter: ipset: Correct rcu_dereference() call in ip_set_put_comment()

[Jozsef Kadlecsik]

The function is called when rcu_read_lock() is held and not
when rcu_read_lock_bh() is held.

Signed-off-by: Jozsef Kadlecsik 
---
 include/linux/netfilter/ipset/ip_set_comment.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/netfilter/ipset/ip_set_comment.h 
b/include/linux/netfilter/ipset/ip_set_comment.h
index 8e2bab1..70877f8d 100644
--- a/include/linux/netfilter/ipset/ip_set_comment.h
+++ b/include/linux/netfilter/ipset/ip_set_comment.h
@@ -43,11 +43,11 @@ ip_set_init_comment(struct ip_set *set, struct 
ip_set_comment *comment,
rcu_assign_pointer(comment->c, c);
 }
 
-/* Used only when dumping a set, protected by rcu_read_lock_bh() */
+/* Used only when dumping a set, protected by rcu_read_lock() */
 static inline int
 ip_set_put_comment(struct sk_buff *skb, const struct ip_set_comment *comment)
 {
-   struct ip_set_comment_rcu *c = rcu_dereference_bh(comment->c);
+   struct ip_set_comment_rcu *c = rcu_dereference(comment->c);
 
if (!c)
return 0;
-- 
2.1.4

[PATCH 5/5] netfilter: ipset: Fix calling ip_set() macro at dumping

[Jozsef Kadlecsik]

The ip_set() macro is called when either ip_set_ref_lock held only
or no lock/nfnl mutex is held at dumping. Take this into account
properly.

Signed-off-by: Jozsef Kadlecsik 
---
 net/netfilter/ipset/ip_set_core.c | 23 +++
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_core.c 
b/net/netfilter/ipset/ip_set_core.c
index 68db946..292f7c7 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -55,12 +55,27 @@ MODULE_AUTHOR("Jozsef Kadlecsik 
");
 MODULE_DESCRIPTION("core IP set support");
 MODULE_ALIAS_NFNL_SUBSYS(NFNL_SUBSYS_IPSET);
 
-/* When the nfnl mutex is held: */
+/* When the nfnl mutex or ip_set_ref_lock is held: */
 #define ip_set_dereference(p)  \
-   rcu_dereference_protected(p, lockdep_nfnl_is_held(NFNL_SUBSYS_IPSET))
+   rcu_dereference_protected(p,\
+   lockdep_nfnl_is_held(NFNL_SUBSYS_IPSET) || \
+   lockdep_is_held(_set_ref_lock))
 #define ip_set(inst, id)   \
ip_set_dereference((inst)->ip_set_list)[id]
 
+/* When the nfnl mutex is not held (dumping): */
+static inline
+struct ip_set * ip_set_no_mutex(struct ip_set_net *inst, ip_set_id_t id)
+{
+   struct ip_set *set;
+
+   rcu_read_lock();
+   set = rcu_dereference((inst)->ip_set_list)[id];
+   rcu_read_unlock();
+
+   return set;
+}
+
 /* The set types are implemented in modules and registered set types
  * can be found in ip_set_type_list. Adding/deleting types is
  * serialized by ip_set_type_mutex.
@@ -1251,7 +1266,7 @@ ip_set_dump_done(struct netlink_callback *cb)
struct ip_set_net *inst =
(struct ip_set_net *)cb->args[IPSET_CB_NET];
ip_set_id_t index = (ip_set_id_t)cb->args[IPSET_CB_INDEX];
-   struct ip_set *set = ip_set(inst, index);
+   struct ip_set *set = ip_set_no_mutex(inst, index);
 
if (set->variant->uref)
set->variant->uref(set, cb, false);
@@ -1440,7 +1455,7 @@ ip_set_dump_start(struct sk_buff *skb, struct 
netlink_callback *cb)
 release_refcount:
/* If there was an error or set is done, release set */
if (ret || !cb->args[IPSET_CB_ARG0]) {
-   set = ip_set(inst, index);
+   set = ip_set_no_mutex(inst, index);
if (set->variant->uref)
set->variant->uref(set, cb, false);
pr_debug("release set %s\n", set->name);
-- 
2.1.4

[PATCH 0/5] ipset patches for nf

[Jozsef Kadlecsik]

Hi Pablo,

Please pull the next patches for the nf tree:

- Decrease refcount synchronously on deletion and replace by
  Stefano Brivio, which fixes the reference counter shown in
  userspace.
- Allow CIDR 0 in hash:net,port,net, which is documented but
  was unnecessarily disabled, from Eric Westbrook.
- Fix allocation failure when memory is fragmented, from Andrey Ryabinin.
- Correct rcu_dereference() call in ip_set_put_comment().
- Also, correct ip_set() macro calling at dumping with respect of held
  mutex or lock.

Best regards,
Jozsef

The following changes since commit a3fb3698cadf27dc142b24394c401625e14d80d0:

  netfilter: nf_flow_table: do not remove offload when other netns's interface 
is down (2018-10-19 13:30:48 +0200)

are available in the git repository at:

  git://blackhole.kfki.hu/nf ccf966c9418db9

for you to fetch changes up to ccf966c9418db93579b7fec4e1e7cfdd3a57a05c:

  netfilter: ipset: Fix calling ip_set() macro at dumping (2018-10-22 23:33:57 
+0200)


Andrey Ryabinin (1):
  netfilter: ipset: fix ip_set_list allocation failure

Eric Westbrook (1):
  netfilter: ipset: actually allow allowable CIDR 0 in hash:net,port,net

Jozsef Kadlecsik (2):
  netfilter: ipset: Correct rcu_dereference() call in ip_set_put_comment()
  netfilter: ipset: Fix calling ip_set() macro at dumping

Stefano Brivio (1):
  netfilter: ipset: list:set: Decrease refcount synchronously on deletion 
and replace

 include/linux/netfilter/ipset/ip_set.h |  2 +-
 include/linux/netfilter/ipset/ip_set_comment.h |  4 +-
 net/netfilter/ipset/ip_set_core.c  | 54 --
 net/netfilter/ipset/ip_set_hash_netportnet.c   |  8 ++--
 net/netfilter/ipset/ip_set_list_set.c  | 17 +---
 5 files changed, 52 insertions(+), 33 deletions(-)

[PATCH 2/5] netfilter: ipset: actually allow allowable CIDR 0 in hash:net,port,net

[Jozsef Kadlecsik]

From: Eric Westbrook 

Allow /0 as advertised for hash:net,port,net sets.

For "hash:net,port,net", ipset(8) says that "either subnet
is permitted to be a /0 should you wish to match port
between all destinations."

Make that statement true.

Before:

# ipset create cidrzero hash:net,port,net
# ipset add cidrzero 0.0.0.0/0,12345,0.0.0.0/0
ipset v6.34: The value of the CIDR parameter of the IP address is invalid

# ipset create cidrzero6 hash:net,port,net family inet6
# ipset add cidrzero6 ::/0,12345,::/0
ipset v6.34: The value of the CIDR parameter of the IP address is invalid

After:

# ipset create cidrzero hash:net,port,net
# ipset add cidrzero 0.0.0.0/0,12345,0.0.0.0/0
# ipset test cidrzero 192.168.205.129,12345,172.16.205.129
192.168.205.129,tcp:12345,172.16.205.129 is in set cidrzero.

# ipset create cidrzero6 hash:net,port,net family inet6
# ipset add cidrzero6 ::/0,12345,::/0
# ipset test cidrzero6 fe80::1,12345,ff00::1
fe80::1,tcp:12345,ff00::1 is in set cidrzero6.

See also:

  https://bugzilla.kernel.org/show_bug.cgi?id=200897
  
https://github.com/ewestbrook/linux/commit/df7ff6efb0934ab6acc11f003ff1a7580d6c1d9c

Signed-off-by: Eric Westbrook 
Signed-off-by: Jozsef Kadlecsik 
---
 net/netfilter/ipset/ip_set_hash_netportnet.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_hash_netportnet.c 
b/net/netfilter/ipset/ip_set_hash_netportnet.c
index d391485..613e18e 100644
--- a/net/netfilter/ipset/ip_set_hash_netportnet.c
+++ b/net/netfilter/ipset/ip_set_hash_netportnet.c
@@ -213,13 +213,13 @@ hash_netportnet4_uadt(struct ip_set *set, struct nlattr 
*tb[],
 
if (tb[IPSET_ATTR_CIDR]) {
e.cidr[0] = nla_get_u8(tb[IPSET_ATTR_CIDR]);
-   if (!e.cidr[0] || e.cidr[0] > HOST_MASK)
+   if (e.cidr[0] > HOST_MASK)
return -IPSET_ERR_INVALID_CIDR;
}
 
if (tb[IPSET_ATTR_CIDR2]) {
e.cidr[1] = nla_get_u8(tb[IPSET_ATTR_CIDR2]);
-   if (!e.cidr[1] || e.cidr[1] > HOST_MASK)
+   if (e.cidr[1] > HOST_MASK)
return -IPSET_ERR_INVALID_CIDR;
}
 
@@ -493,13 +493,13 @@ hash_netportnet6_uadt(struct ip_set *set, struct nlattr 
*tb[],
 
if (tb[IPSET_ATTR_CIDR]) {
e.cidr[0] = nla_get_u8(tb[IPSET_ATTR_CIDR]);
-   if (!e.cidr[0] || e.cidr[0] > HOST_MASK)
+   if (e.cidr[0] > HOST_MASK)
return -IPSET_ERR_INVALID_CIDR;
}
 
if (tb[IPSET_ATTR_CIDR2]) {
e.cidr[1] = nla_get_u8(tb[IPSET_ATTR_CIDR2]);
-   if (!e.cidr[1] || e.cidr[1] > HOST_MASK)
+   if (e.cidr[1] > HOST_MASK)
return -IPSET_ERR_INVALID_CIDR;
}
 
-- 
2.1.4

Re: [PATCH] netfilter: ipset: export indexes via netlink

[Jozsef Kadlecsik]

Hi,

On Mon, 1 Oct 2018, Florent Fourcot wrote:

> Do you have any news on this topic? Can I help you to move forward for 
> inclusion?

Sorry for the extremely long delay: I have been working on the userspace 
library and it still needs a couple of days. There'll be a new release in 
the first half of this month :-)

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH ipset v3] Validate string type attributes in attr2data()

[Jozsef Kadlecsik]

On Fri, 31 Aug 2018, Stefano Brivio wrote:

> Otherwise, we are missing checks in some paths, e.g. we might
> overrun the buffer used to save the set name in callback_list()
> when we strcpy() to it.
> 
> Signed-off-by: Stefano Brivio 
> ---
> v3: Also as pointed out by Jozsef, there's no need to validate
> the set name in ipset_cmd(), this is done already while
> parsing the command line, so drop that part and change the
> patch title accordingly.
> 
> v2: As requested by Jozsef, move validation of setname length to
> attr2data() for data received via netlink, instead of doing
> it in callback_list().
> 
>  lib/session.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/lib/session.c b/lib/session.c
> index ca96aaa57ea6..16b5549e73db 100644
> --- a/lib/session.c
> +++ b/lib/session.c
> @@ -678,6 +678,10 @@ attr2data(struct ipset_session *session, struct nlattr 
> *nla[],
>   default:
>   break;
>   }
> + } else if (attr->type == MNL_TYPE_NUL_STRING) {
> + if (!d || strlen(d) >= attr->len)
> + FAILURE("Broken kernel message: "
> + "string type attribute missing or too long!");
>   }
>  #ifdef IPSET_DEBUG
>else
> -- 

Patch is applied, thanks.

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH ipset v2] Check setname length in session code before copying it

[Jozsef Kadlecsik]

Hi Stefano,

On Thu, 30 Aug 2018, Stefano Brivio wrote:

> > > @@ -2014,7 +2021,11 @@ ipset_cmd(struct ipset_session *session, enum 
> > > ipset_cmd cmd, uint32_t lineno)
> > >   if (session->lineno != 0 &&
> > >   (cmd == IPSET_CMD_ADD || cmd == IPSET_CMD_DEL)) {
> > >   /* Save setname for the next possible aggregated restore line */
> > > - strcpy(session->saved_setname, ipset_data_setname(data));
> > > + const char *setname = ipset_data_setname(data);
> > > + if (!setname || strlen(setname) >= IPSET_MAXNAMELEN)
> > > + return ipset_err(session,
> > > + "Invalid command: setname missing or too long");
> > > + strcpy(session->saved_setname, setname);  
> > 
> > I don't understand this part: it is from userspace to kernel and ADD/DEL 
> > commands already verified that setname is filled out, in build_msg().
> 
> Right, I don't need to check that setname is there. Still, its length is 
> not checked as far as I can see. Would you suggest that I move the 
> length check to build_msg()?

build_msg() gets all the data for the netlink message from the ipset_data 
structure. The structure is filled up by parsers and the parsers verify 
the input, in this case ipset_parse_setname(). So the checking is 
unnecessary here.

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH ipset v2] Check setname length in session code before copying it

[Jozsef Kadlecsik]

Hi Stefano,

On Wed, 29 Aug 2018, Stefano Brivio wrote:

> We might overrun the buffer used to save it otherwise.
> 
> Signed-off-by: Stefano Brivio 
> ---
> v2: As requested by Jozsef, move validation of setname length to
> attr2data() for data received via netlink, instead of doing
> it in callback_list().
> 
>  lib/session.c | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/session.c b/lib/session.c
> index ca96aaa57ea6..74310a19850e 100644
> --- a/lib/session.c
> +++ b/lib/session.c
> @@ -691,6 +691,13 @@ attr2data(struct ipset_session *session, struct nlattr 
> *nla[],
>   D("nla typename %s",
> (const char *) ipset_data_get(data, IPSET_OPT_TYPENAME));
>  #endif
> + if (type == IPSET_ATTR_SETNAME && nla[IPSET_ATTR_DATA]) {
> + const char *setname = ipset_data_setname(data);
> + if (!setname || strlen(setname) >= IPSET_MAXNAMELEN)
> + FAILURE("Broken kernel message: "
> + "setname missing or too long!");
> + }
> +
>   return ret;
>  }

Actually, I was thinking something like this:

if (attr->type == MNL_TYPE_UNSPEC)
return 0;
else if (attr->type == MNL_TYPE_NUL_STRING) {
const char *str = d;
if (!d || strlen(d) >= attr->len)
FAILURE(...)
} ...

That way all typename is also verified.

> @@ -2014,7 +2021,11 @@ ipset_cmd(struct ipset_session *session, enum 
> ipset_cmd cmd, uint32_t lineno)
>   if (session->lineno != 0 &&
>   (cmd == IPSET_CMD_ADD || cmd == IPSET_CMD_DEL)) {
>   /* Save setname for the next possible aggregated restore line */
> - strcpy(session->saved_setname, ipset_data_setname(data));
> + const char *setname = ipset_data_setname(data);
> + if (!setname || strlen(setname) >= IPSET_MAXNAMELEN)
> + return ipset_err(session,
> + "Invalid command: setname missing or too long");
> + strcpy(session->saved_setname, setname);

I don't understand this part: it is from userspace to kernel and ADD/DEL 
commands already verified that setname is filled out, in build_msg().

>   ipset_data_reset(data);
>   /* Don't commit: we may aggregate next command */
>   ret = 0;
> -- 

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH ipset] manpage: Add comment about matching on destination MAC address

[Jozsef Kadlecsik]

On Wed, 29 Aug 2018, Stefano Brivio wrote:

> Patch "ipset: Allow matching on destination MAC address for mac
> and ipmac sets" allows the user to match on destination MAC
> addresses in some selected cases. Add a comment to the manpage
> detailing in which cases it makes sense.
> 
> Signed-off-by: Stefano Brivio 
> ---
> Jozsef, I'm sending this as a separate patch as I guess it's more
> convenient to have kernel and manpage changes separated. Please
> let me know if I should rather squash this into the kernel patch
> itself.
> 
>  src/ipset.8 | 26 +++---
>  1 file changed, 19 insertions(+), 7 deletions(-)

Yes, it's fine this way - patch is applied.

Best regards,
Jozsef
> diff --git a/src/ipset.8 b/src/ipset.8
> index 87fb93814ddc..9f1e68f247d6 100644
> --- a/src/ipset.8
> +++ b/src/ipset.8
> @@ -451,13 +451,15 @@ The \fBbitmap:ip,mac\fR type is exceptional in the 
> sense that the MAC part can
>  be left out when adding/deleting/testing entries in the set. If we add an 
> entry
>  without the MAC address specified, then when the first time the entry is
>  matched by the kernel, it will automatically fill out the missing MAC 
> address with the
> -source MAC address from the packet. If the entry was specified with a 
> timeout value,
> -the timer starts off when the IP and MAC address pair is complete.
> +MAC address from the packet. The source MAC address is used if the entry 
> matched
> +due to a \fBsrc\fR parameter of the \fBset\fR match, and the destination MAC
> +address is used if available and the entry matched due to a \fBdst\fR 
> parameter.
> +If the entry was specified with a timeout value, the timer starts off when 
> the
> +IP and MAC address pair is complete.
>  .PP
>  The \fBbitmap:ip,mac\fR type of sets require two \fBsrc/dst\fR parameters of
> -the \fBset\fR match and \fBSET\fR target netfilter kernel modules and the 
> second
> -one must be \fBsrc\fR to match, add or delete entries, because the \fBset\fR
> -match and \fBSET\fR target have access to the source MAC address only.
> +the \fBset\fR match and \fBSET\fR target netfilter kernel modules. For 
> matches
> +on destination MAC addresses, see COMMENTS below.
>  .PP
>  Examples:
>  .IP 
> @@ -532,7 +534,7 @@ ipset add foo 192.168.1.0/24
>  ipset test foo 192.168.1.2
>  .SS hash:mac
>  The \fBhash:mac\fR set type uses a hash to store MAC addresses. Zero valued 
> MAC addresses cannot be stored in a \fBhash:mac\fR
> -type of set.
> +type of set. For matches on destination MAC addresses, see COMMENTS below.
>  .PP
>  \fICREATE\-OPTIONS\fR := [ \fBhashsize\fR \fIvalue\fR ] [ \fBmaxelem\fR 
> \fIvalue\fR ] [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ] [ 
> \fBcomment\fP ] [ \fBskbinfo\fP ]
>  .PP
> @@ -554,7 +556,7 @@ ipset test foo 01:02:03:04:05:06
>  
>  .SS hash:ip,mac
>  The \fBhash:ip,mac\fR set type uses a hash to store IP and a MAC address 
> pairs. Zero valued MAC addresses cannot be stored in a \fBhash:ip,mac\fR
> -type of set.
> +type of set. For matches on destination MAC addresses, see COMMENTS below.
>  .PP
>  \fICREATE\-OPTIONS\fR := [ \fBfamily\fR { \fBinet\fR | \fBinet6\fR } ] | [ 
> \fBhashsize\fR \fIvalue\fR ] [ \fBmaxelem\fR \fIvalue\fR ] [ \fBtimeout\fR 
> \fIvalue\fR ] [ \fBcounters\fP ] [ \fBcomment\fP ] [ \fBskbinfo\fP ]
>  .PP
> @@ -1058,6 +1060,16 @@ If you want to store random same size networks (say 
> random /24 blocks),
>  use the \fBhash:ip\fR set type. If you have got random size of netblocks, 
>  use \fBhash:net\fR.
>  .PP
> +Matching on destination MAC addresses using the \fBdst\fR parameter of the
> +\fBset\fR match netfilter kernel modules will only work if the destination 
> MAC
> +address is available in the packet at the given processing stage, that is, it
> +only applies for incoming packets in the \fBPREROUTING\fR, \fBINPUT\fR and
> +\fBFORWARD\fR chains, against the MAC address as originally found in the
> +received packet (typically, one of the MAC addresses of the local host). 
> This is
> +\fBnot\fR the destination MAC address a destination IP address resolves to,
> +after routing. If the MAC address is not available (e.g. in the \fBOUTPUT\fR
> +chain), the packet will simply not match.
> +.PP
>  Backward compatibility is maintained and old \fBipset\fR syntax is still 
> supported.
>  .PP
>  The \fBiptree\fR and \fBiptreemap\fR set types are removed: if you refer to 
> them,
> -- 
> 2.18.0
> 
> 

-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH 0/2] ipset: Destination MAC match, consistent zero MAC checks

[Jozsef Kadlecsik]

Hi Stefano,

On Fri, 17 Aug 2018, Stefano Brivio wrote:

> This series allows matching on destination MAC address in
> bitmap:ipmac, hash:mac and hash:ipmac sets, and makes checks
> against all-zero MAC addresses consistent across these three set
> types.
> 
> Stefano Brivio (2):
>   ipset: Allow matching on destination MAC address for mac and ipmac
> sets
>   ipset: Make invalid MAC address checks consistent
> 
>  net/netfilter/ipset/ip_set_bitmap_ipmac.c | 13 -
>  net/netfilter/ipset/ip_set_hash_ipmac.c   | 27 ++-
>  net/netfilter/ipset/ip_set_hash_mac.c | 10 +-
>  3 files changed, 27 insertions(+), 23 deletions(-)

Patches are applied, thanks.

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH ipset 1/4] Fix use-after-free in ipset_parse_name_compat()

[Jozsef Kadlecsik]

Hi,

On Mon, 27 Aug 2018, Stefano Brivio wrote:

> On Fri, 24 Aug 2018 21:02:12 +0200 (CEST)
> Jozsef Kadlecsik  wrote:
> 
> > > @@ -1396,10 +1396,11 @@ ipset_parse_iptimeout(struct ipset_session 
> > > *session,
> > >  #define check_setname(str, saved)
> > > \
> > >  do { 
> > > \
> > >   if (strlen(str) > IPSET_MAXNAMELEN - 1) {   \
> > > - if (saved != NULL)  \
> > > - free(saved);\
> > > - return syntax_err("setname '%s' is longer than %u characters",\
> > > + int err;\
> > > + err = syntax_err("setname '%s' is longer than %u characters",\
> > > str, IPSET_MAXNAMELEN - 1);   \
> > > + free(saved);\
> > > + return err; \
> > >   }   \
> > >  } while (0)  
> > 
> > At some places check_setname() is invoked with "saved" explicitly set to 
> > NULL, so the condition to free cannot be left out. Please resubmit a new 
> > version of the patch. Thanks!
> 
> I dropped this check because, as Sven-Haegar mentioned, free() on a
> NULL pointer has no effect. Both C89 and C99 say:
> 
>   The free function causes the space pointed to by ptr to be
> deallocated, that is, made available for further allocation.  If ptr
> is a null pointer, no action occurs.
> 
> As a side note, this pattern is being gradually removed in the kernel,
> also with the help of ifnullfree.cocci.
> 
> Should I maintain it for ipset coding style reasons?

No, I have just applied your patch. Thanks!

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH ipset 1/4] Fix use-after-free in ipset_parse_name_compat()

[Jozsef Kadlecsik]

On Sat, 25 Aug 2018, Sven-Haegar Koch wrote:

> > On Wed, 22 Aug 2018, Stefano Brivio wrote:
> > 
> > > When check_setname is used in ipset_parse_name_compat(), the
> > > 'str' and 'saved' macro arguments point in fact to the same
> > > buffer. Free the 'saved' argument only after using it.
> > > 
> > > While at it, remove a useless NULL check on 'saved'.
> > > 
> > > Signed-off-by: Stefano Brivio 
> > > ---
> > >  lib/parse.c | 7 ---
> > >  1 file changed, 4 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/lib/parse.c b/lib/parse.c
> > > index 9a79ccda796c..4963d519c631 100644
> > > --- a/lib/parse.c
> > > +++ b/lib/parse.c
> > > @@ -1396,10 +1396,11 @@ ipset_parse_iptimeout(struct ipset_session 
> > > *session,
> > >  #define check_setname(str, saved)
> > > \
> > >  do { 
> > > \
> > >   if (strlen(str) > IPSET_MAXNAMELEN - 1) {   \
> > > - if (saved != NULL)  \
> > > - free(saved);\
> > > - return syntax_err("setname '%s' is longer than %u characters",\
> > > + int err;\
> > > + err = syntax_err("setname '%s' is longer than %u characters",\
> > > str, IPSET_MAXNAMELEN - 1);   \
> > > + free(saved);\
> > > + return err; \
> > >   }   \
> > >  } while (0)
> > 
> > At some places check_setname() is invoked with "saved" explicitly set to 
> > NULL, so the condition to free cannot be left out. Please resubmit a new 
> > version of the patch. Thanks!
> 
> Why?
> 
> The free() manpage says:
> 
>   If ptr is NULL, no operation is performed.
> 
> So it is ok to always call it on a NULL pointer. The same as with 
> kfree() in the kernel, no extra NULL check needed before.

You are right. I suppose old customs die slowly :-) (in old times it was 
not proper to call kfree() with a NULL pointer).

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH 1/2] ipset: Allow matching on destination MAC address for mac and ipmac sets

[Jozsef Kadlecsik]

Hi Stefano,

On Mon, 20 Aug 2018, Stefano Brivio wrote:

> > On Fri, 17 Aug 2018, Stefano Brivio wrote:
> > 
> > > There doesn't seem to be any reason to restrict MAC address
> > > matching to source MAC addresses in set types bitmap:ipmac,
> > > hash:ipmac and hash:mac. With this patch, and this setup:
> > > 
> > >   ip netns add A
> > >   ip link add veth1 type veth peer name veth2 netns A
> > >   ip addr add 192.0.2.1/24 dev veth1
> > >   ip -net A addr add 192.0.2.2/24 dev veth2
> > >   ip link set veth1 up
> > >   ip -net A link set veth2 up
> > > 
> > >   ip netns exec A ipset create test hash:mac
> > >   dst=$(ip netns exec A cat /sys/class/net/veth2/address)
> > >   ip netns exec A ipset add test ${dst}
> > >   ip netns exec A iptables -P INPUT DROP
> > >   ip netns exec A iptables -I INPUT -m set --match-set test dst -j ACCEPT
> > > 
> > > ipset will match packets based on destination MAC address:
> > > 
> > >   # ping -c1 192.0.2.2 >/dev/null
> > >   # echo $?
> > >   0  
> > 
> > The netfilter framework "does not see" the destination MAC address. So how 
> > does it come that it can get the required information now?
> 
> Well, on the receive path, the L2 header is available and complete. On
> the sending path, it's not, but there the MAC address is all zeroes and
> no packets will match.

That's good to know, so there's no need to check in which path the MAC 
address is requested.

> Let's say I use the same setup as above, then:
> 
> # echo 1 > /proc/sys/net/netfilter/nf_log_all_netns
> # ip netns exec A iptables -A INPUT -j LOG --log-prefix "input: "
> # ip netns exec A iptables -A OUTPUT -j LOG --log-prefix "output: "
> # ping -c1 192.0.2.2 >/dev/null
> 
> What I get in the log is:
> 
> input: IN=veth2 OUT= MAC=62:68:5a:00:71:7c:72:59:6b:0e:9b:dc:08:00 
> SRC=192.0.2.1 DST=192.0.2.2 LEN=84 TOS=0x00 PREC=0x00 TTL=64 ID=39182 DF 
> PROTO=ICMP TYPE=8 CODE=0 ID=3637 SEQ=1
> output: IN= OUT=veth2 SRC=192.0.2.2 DST=192.0.2.1 LEN=84 TOS=0x00 PREC=0x00 
> TTL=64 ID=1307 PROTO=ICMP TYPE=0 CODE=0 ID=3637 SEQ=1
> 
> I see two issues with the current behaviour:
> 
> - we seem to allow but silently disregard matching on destination for sets
>   using MAC addresses
> 
> - we don't allow matching on destination MAC addresses in cases where
>   the user is legitimately expecting it to work (that is, receive path)
> 
> If you think getting destination MAC address matches to work on
> selected paths might be confusing to the user, as they might expect it
> to work everywhere (e.g. expecting it to match the MAC address of the
> destination IP address after routing), I'd rather add a note to the man
> page of ipset. I actually don't think it's the case, though.

As far as I see, the destination MAC is useful in input path (PREROUTING) 
*and* in bridge mode (ebtables) only. (In the non-bridge mode the dest MAC 
should be the MAC address of the receiving interface.) So I'd like to see 
the clear documentation in the manpage in order to make clear when dst MAC 
can be used.

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH ipset 3/4] Check setname length in session code before copying it

[Jozsef Kadlecsik]

On Wed, 22 Aug 2018, Stefano Brivio wrote:

> We might overrun the buffer used to save it otherwise.
> 
> Signed-off-by: Stefano Brivio 
> ---
>  lib/session.c | 14 --
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/session.c b/lib/session.c
> index ca96aaa57ea6..7cf3858ca97d 100644
> --- a/lib/session.c
> +++ b/lib/session.c
> @@ -1069,6 +1069,7 @@ callback_list(struct ipset_session *session, struct 
> nlattr *nla[],
>  
>   if (nla[IPSET_ATTR_DATA] != NULL) {
>   struct nlattr *cattr[IPSET_ATTR_CREATE_MAX+1] = {};
> + const char *setname;
>  
>   if (!(nla[IPSET_ATTR_TYPENAME] &&
> nla[IPSET_ATTR_FAMILY] &&
> @@ -1097,7 +1098,12 @@ callback_list(struct ipset_session *session, struct 
> nlattr *nla[],
>   cmd2name[cmd]);
>   if (list_create(session, cattr) != MNL_CB_OK)
>   return MNL_CB_ERROR;
> - strcpy(session->saved_setname, ipset_data_setname(data));
> + setname = ipset_data_setname(data);
> + if (!setname || strlen(setname) >= IPSET_MAXNAMELEN)
> + FAILURE("Broken %s kernel message: "
> + "setname missing or too long!",
> + cmd2name[cmd]);
> + strcpy(session->saved_setname, setname);
>   }
>  
>   if (nla[IPSET_ATTR_ADT] != NULL) {
> @@ -2014,7 +2020,11 @@ ipset_cmd(struct ipset_session *session, enum 
> ipset_cmd cmd, uint32_t lineno)
>   if (session->lineno != 0 &&
>   (cmd == IPSET_CMD_ADD || cmd == IPSET_CMD_DEL)) {
>   /* Save setname for the next possible aggregated restore line */
> - strcpy(session->saved_setname, ipset_data_setname(data));
> + const char *setname = ipset_data_setname(data);
> + if (!setname || strlen(setname) >= IPSET_MAXNAMELEN)
> + return ipset_err(session,
> + "Invalid command: setname missing or too long");
> + strcpy(session->saved_setname, setname);
>   ipset_data_reset(data);
>   /* Don't commit: we may aggregate next command */
>   ret = 0;

The data received via netlink is verified in attr2data() (wrapped in 
ATTR2DATA). The verification of setname length should have been done 
there. So please correct attr2data() instead.

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH ipset 4/4] Fix leak in build_argv() on line parsing error

[Jozsef Kadlecsik]

On Wed, 22 Aug 2018, Stefano Brivio wrote:

> Signed-off-by: Stefano Brivio 
> ---
>  src/ipset.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ipset.c b/src/ipset.c
> index ce1b73f51633..14a351a125f2 100644
> --- a/src/ipset.c
> +++ b/src/ipset.c
> @@ -176,7 +176,7 @@ build_argv(char *buffer)
>   if ((newargc + 1) == (int)(sizeof(newargv)/sizeof(char *))) {
>   exit_error(PARAMETER_PROBLEM,
>  "Line is too long to parse.");
> - return;
> + goto out;
>   }
>   switch (*tmp) {
>   case '"':
> @@ -200,7 +200,7 @@ build_argv(char *buffer)
>   }
>   if (!*(tmp+1) && quoted) {
>   exit_error(PARAMETER_PROBLEM, "Missing close quote!");
> - return;
> + goto out;
>   }
>   if (!*arg)
>   continue;
> @@ -209,6 +209,7 @@ build_argv(char *buffer)
>   memset(arg, 0, strlen(arg) + 1);
>   i = 0;
>   }
> +out:
>   free(arg);
>  }

Patch is applied, thanks.

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH ipset 2/4] Simplify return statement in ipset_mnl_query()

[Jozsef Kadlecsik]

On Wed, 22 Aug 2018, Stefano Brivio wrote:

> As we loop as long as 'ret' is greater than zero, and break only
> if we get an error in mnl_cb_run2 (with ret <= 0), we can just
> return ret without checking once more if it's greater than zero.
> 
> Signed-off-by: Stefano Brivio 
> ---
>  lib/mnl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/mnl.c b/lib/mnl.c
> index a0fa66ecdb80..4e075cf94f00 100644
> --- a/lib/mnl.c
> +++ b/lib/mnl.c
> @@ -115,7 +115,7 @@ ipset_mnl_query(struct ipset_handle *handle, void 
> *buffer, size_t len)
>   ret = mnl_socket_recvfrom(handle->h, buffer, len);
>   D("message received, ret: %d", ret);
>   }
> - return ret > 0 ? 0 : ret;
> + return ret;
>  }

Patch is applied, thanks.

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH ipset 1/4] Fix use-after-free in ipset_parse_name_compat()

[Jozsef Kadlecsik]

Hi Stefano,

On Wed, 22 Aug 2018, Stefano Brivio wrote:

> When check_setname is used in ipset_parse_name_compat(), the
> 'str' and 'saved' macro arguments point in fact to the same
> buffer. Free the 'saved' argument only after using it.
> 
> While at it, remove a useless NULL check on 'saved'.
> 
> Signed-off-by: Stefano Brivio 
> ---
>  lib/parse.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/parse.c b/lib/parse.c
> index 9a79ccda796c..4963d519c631 100644
> --- a/lib/parse.c
> +++ b/lib/parse.c
> @@ -1396,10 +1396,11 @@ ipset_parse_iptimeout(struct ipset_session *session,
>  #define check_setname(str, saved)\
>  do { \
>   if (strlen(str) > IPSET_MAXNAMELEN - 1) {   \
> - if (saved != NULL)  \
> - free(saved);\
> - return syntax_err("setname '%s' is longer than %u characters",\
> + int err;\
> + err = syntax_err("setname '%s' is longer than %u characters",\
> str, IPSET_MAXNAMELEN - 1);   \
> + free(saved);\
> + return err; \
>   }   \
>  } while (0)

At some places check_setname() is invoked with "saved" explicitly set to 
NULL, so the condition to free cannot be left out. Please resubmit a new 
version of the patch. Thanks!

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH 1/2] ipset: Allow matching on destination MAC address for mac and ipmac sets

[Jozsef Kadlecsik]

Hi,

On Fri, 17 Aug 2018, Stefano Brivio wrote:

> There doesn't seem to be any reason to restrict MAC address
> matching to source MAC addresses in set types bitmap:ipmac,
> hash:ipmac and hash:mac. With this patch, and this setup:
> 
>   ip netns add A
>   ip link add veth1 type veth peer name veth2 netns A
>   ip addr add 192.0.2.1/24 dev veth1
>   ip -net A addr add 192.0.2.2/24 dev veth2
>   ip link set veth1 up
>   ip -net A link set veth2 up
> 
>   ip netns exec A ipset create test hash:mac
>   dst=$(ip netns exec A cat /sys/class/net/veth2/address)
>   ip netns exec A ipset add test ${dst}
>   ip netns exec A iptables -P INPUT DROP
>   ip netns exec A iptables -I INPUT -m set --match-set test dst -j ACCEPT
> 
> ipset will match packets based on destination MAC address:
> 
>   # ping -c1 192.0.2.2 >/dev/null
>   # echo $?
>   0

The netfilter framework "does not see" the destination MAC address. So how 
does it come that it can get the required information now?

Best regards,
Jozsef

> diff --git a/net/netfilter/ipset/ip_set_bitmap_ipmac.c 
> b/net/netfilter/ipset/ip_set_bitmap_ipmac.c
> index c00b6a2e8e3c..13ade5782847 100644
> --- a/net/netfilter/ipset/ip_set_bitmap_ipmac.c
> +++ b/net/netfilter/ipset/ip_set_bitmap_ipmac.c
> @@ -219,10 +219,6 @@ bitmap_ipmac_kadt(struct ip_set *set, const struct 
> sk_buff *skb,
>   struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
>   u32 ip;
>  
> - /* MAC can be src only */
> - if (!(opt->flags & IPSET_DIM_TWO_SRC))
> - return 0;
> -
>   ip = ntohl(ip4addr(skb, opt->flags & IPSET_DIM_ONE_SRC));
>   if (ip < map->first_ip || ip > map->last_ip)
>   return -IPSET_ERR_BITMAP_RANGE;
> @@ -233,7 +229,11 @@ bitmap_ipmac_kadt(struct ip_set *set, const struct 
> sk_buff *skb,
>   return -EINVAL;
>  
>   e.id = ip_to_id(map, ip);
> - memcpy(e.ether, eth_hdr(skb)->h_source, ETH_ALEN);
> +
> + if (opt->flags & IPSET_DIM_ONE_SRC)
> + ether_addr_copy(e.ether, eth_hdr(skb)->h_source);
> + else
> + ether_addr_copy(e.ether, eth_hdr(skb)->h_dest);
>  
>   return adtfn(set, , , >ext, opt->cmdflags);
>  }
> diff --git a/net/netfilter/ipset/ip_set_hash_ipmac.c 
> b/net/netfilter/ipset/ip_set_hash_ipmac.c
> index 1ab5ed2f6839..fd87de3ed55b 100644
> --- a/net/netfilter/ipset/ip_set_hash_ipmac.c
> +++ b/net/netfilter/ipset/ip_set_hash_ipmac.c
> @@ -103,7 +103,11 @@ hash_ipmac4_kadt(struct ip_set *set, const struct 
> sk_buff *skb,
>   (skb_mac_header(skb) + ETH_HLEN) > skb->data)
>   return -EINVAL;
>  
> - memcpy(e.ether, eth_hdr(skb)->h_source, ETH_ALEN);
> + if (opt->flags & IPSET_DIM_ONE_SRC)
> + ether_addr_copy(e.ether, eth_hdr(skb)->h_source);
> + else
> + ether_addr_copy(e.ether, eth_hdr(skb)->h_dest);
> +
>   if (ether_addr_equal(e.ether, invalid_ether))
>   return -EINVAL;
>  
> @@ -211,15 +215,15 @@ hash_ipmac6_kadt(struct ip_set *set, const struct 
> sk_buff *skb,
>   };
>   struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
>  
> -  /* MAC can be src only */
> - if (!(opt->flags & IPSET_DIM_TWO_SRC))
> - return 0;
> -
>   if (skb_mac_header(skb) < skb->head ||
>   (skb_mac_header(skb) + ETH_HLEN) > skb->data)
>   return -EINVAL;
>  
> - memcpy(e.ether, eth_hdr(skb)->h_source, ETH_ALEN);
> + if (opt->flags & IPSET_DIM_ONE_SRC)
> + ether_addr_copy(e.ether, eth_hdr(skb)->h_source);
> + else
> + ether_addr_copy(e.ether, eth_hdr(skb)->h_dest);
> +
>   if (ether_addr_equal(e.ether, invalid_ether))
>   return -EINVAL;
>  
> diff --git a/net/netfilter/ipset/ip_set_hash_mac.c 
> b/net/netfilter/ipset/ip_set_hash_mac.c
> index f9d5a2a1e3d0..4fe5f243d0a3 100644
> --- a/net/netfilter/ipset/ip_set_hash_mac.c
> +++ b/net/netfilter/ipset/ip_set_hash_mac.c
> @@ -81,15 +81,15 @@ hash_mac4_kadt(struct ip_set *set, const struct sk_buff 
> *skb,
>   struct hash_mac4_elem e = { { .foo[0] = 0, .foo[1] = 0 } };
>   struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
>  
> -  /* MAC can be src only */
> - if (!(opt->flags & IPSET_DIM_ONE_SRC))
> - return 0;
> -
>   if (skb_mac_header(skb) < skb->head ||
>   (skb_mac_header(skb) + ETH_HLEN) > skb->data)
>   return -EINVAL;
>  
> - ether_addr_copy(e.ether, eth_hdr(skb)->h_source);
> + if (opt->flags & IPSET_DIM_ONE_SRC)
> + ether_addr_copy(e.ether, eth_hdr(skb)->h_source);
> + else
> + ether_addr_copy(e.ether, eth_hdr(skb)->h_dest);
> +
>   if (is_zero_ether_addr(e.ether))
>   return -EINVAL;
>   return adtfn(set, , , >ext, opt->cmdflags);
> -- 
> 2.15.1
> 
> 

-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of 

Re: Creating custom ipsets

[Jozsef Kadlecsik]

On Fri, 10 Aug 2018, Akshat Kakkar wrote:

> > No, that's a totally wrong way. ipset is independent from 
> > iptables/ip6tables: you cannot refer to a match/target/chain from 
> > ipset. It also makes no sense to reimplement those in ipset.
> 
> Yes. Thats obvious that iptables need to do decision handling, packet 
> flow, etc. Just that, I want to store that information in ipset using 
> which iptables can decide. Clearly, iptables also need to have support 
> of these modified/new ipset.

That'd mean all those things should be interfaced via the SET target - too 
complicated and still would require a lot of modifications in iptables as 
well.
 
> > If you miss functionality in nftables compared to ipset, then invest 
> > your energy in nftables instead. Dictionaries, maps are already there.
> 
> This looks to me more promising from day 1. However, are all 
> functionalities of iptables, ipset incorporated in nftables? For eg., 
> can we store connmark and tc classid in skbinfo of named set in 
> nftables?

I'd not worry about connmark/classid in nftables. Those are missing from 
nftables but I strongly believe it's simpler to add the support there. 

What's really missing from nftables yet is to handle real subnets in 
sets/maps/dicts. To add that feature needs more work and time.

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary

Re: Creating custom ipsets

[Jozsef Kadlecsik]

On Thu, 9 Aug 2018, Akshat Kakkar wrote:

> Thanks for the info. nftables set infra lacks lot of things specially 
> interfaces. Besides, I just dont want to develop new ipsets, but also 
> want to extend its functionality so as to include rule decision, natted 
> ips, etc.As you have already asked the question in place of me, I 
> appreciate your effort once again.

No, that's a totally wrong way. ipset is independent from 
iptables/ip6tables: you cannot refer to a match/target/chain from ipset. 
It also makes no sense to reimplement those in ipset.

If you miss functionality in nftables compared to ipset, then invest your 
energy in nftables instead. Dictionaries, maps are already there. 

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] netfilter: ipset: export indexes via netlink

[Jozsef Kadlecsik]

Hi Florent,

On Fri, 27 Jul 2018, Florent Fourcot wrote:

> On pyroute2 library, a method can build a python object based on netlink 
> messages:
> 
> https://github.com/svinota/pyroute2/blob/master/pyroute2/wiset.py#L174
> 
> We could of course fill index attribute with your new command, but that 
> could be nice as well to read it in the same way (list) than other 
> attributes.

Don't get me wrong, I'm just trying to understand how it'd be used.

The rules of set indices are quite straightforward: counted successively 
from zero. When a set is deleted there'll be a hole in the numbering which 
will be reused next when a new set is created. Renaming doesn't change the 
index, however swapping does.

So if one could guarantee that your library alone communicates to the 
ip_set module in the kernel, then it makes sense to pass the indices at 
listing and cache them. However that cannot be guaranteed. So you should 
recheck an index before you use it - and even in that case it's not 
mutable. Therefore I'm reluctant to add it to listing. Do you really gain 
something with it? Wouldn't make sense to add it to the header-only 
listing (as you proposed), to make possible a quick check for all indices?

The two new proposed commands are for direct, single-shot usage.

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] netfilter: ipset: export indexes via netlink

[Jozsef Kadlecsik]

Hi Florent,

On Wed, 25 Jul 2018, Florent Fourcot wrote:

> Just a small comment after a short code review: what about adding 
> IPSET_ATTR_INDEX in list command when proto is greater than 6? I agree 
> that specific commands are a good idea, but i still think that adding it 
> in list is a good idea (it's probably less relevant in ip_set_header 
> with your patch).

But what is the usercase for IPSET_ATTR_INDEX in a list command?

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] netfilter: ipset: export indexes via netlink

[Jozsef Kadlecsik]

Hi Florent,

On Mon, 16 Jul 2018, Jozsef Kadlecsik wrote:

> > Do you have any update on this? In my opinion, there are already some 
> > flags to control list output (LIST_SETNAME and LIST_HEADER), and 
> > adding one more is not really a breaking change.
> 
> Controlling list output is not the same: kernel versions which do not 
> know the flags simply list the whole sets and userspace handles it fine. 
> The only drawback is the unnecessary data transfer.
> 
> However with your proposed flag, additional data is returned. I'm 
> concerned about backward compatilibity. What happens when a new userspace 
> tool communicates with an old kernel? The only way to detect the 
> incompatility is to check that the anticipated attributes are missing. But 
> ipset strictly checks the attributes and reports protocol violations. With 
> this solution there's no way to tell the difference between an old kernel 
> or broken protocol.
> 
> > If you have any hints/idea to improve this patch, I can try to provide a 
> > new version.
> 
> I want to introduce a new protocol version. Both the kernel and 
> userspace have got the very basic parts to support multiple protocols, 
> however have never been tested, obviously. Also, I'm thinking on adding 
> two new commands to get the set by name and index and not a single one 
> for both operations. The whole thing needs time and I'm busy these weeks 
> with a cluster migration.
> 
> With a new protocol version, new userspace tools can "negotiate" the old 
> protocol version with old kernels and can easily fall back to the 
> getsockopt solution to get the required data from kernel.

Please have a look at the next patch: it introduces the two new commands 
to get the set by name or index. (The index number is transferred in 
network byte order, as every similar data in the ipset protocol.) 

According to my tests, the old userspace tool works just fine with the new 
kernel. 

diff --git a/include/linux/netfilter/ipset/ip_set.h 
b/include/linux/netfilter/ipset/ip_set.h
index 34fc80f..c4ce074 100644
--- a/include/linux/netfilter/ipset/ip_set.h
+++ b/include/linux/netfilter/ipset/ip_set.h
@@ -303,11 +303,11 @@ ip_set_put_flags(struct sk_buff *skb, struct ip_set *set)
 /* Netlink CB args */
 enum {
IPSET_CB_NET = 0,   /* net namespace */
+   IPSET_CB_PROTO, /* ipset protocol */
IPSET_CB_DUMP,  /* dump single set/all sets */
IPSET_CB_INDEX, /* set index */
IPSET_CB_PRIVATE,   /* set private data */
IPSET_CB_ARG0,  /* type specific */
-   IPSET_CB_ARG1,
 };
 
 /* register and unregister set references */
diff --git a/include/uapi/linux/netfilter/ipset/ip_set.h 
b/include/uapi/linux/netfilter/ipset/ip_set.h
index 60236f6..ea69ca2 100644
--- a/include/uapi/linux/netfilter/ipset/ip_set.h
+++ b/include/uapi/linux/netfilter/ipset/ip_set.h
@@ -13,8 +13,9 @@
 
 #include 
 
-/* The protocol version */
-#define IPSET_PROTOCOL 6
+/* The protocol versions */
+#define IPSET_PROTOCOL 7
+#define IPSET_PROTOCOL_MIN 6
 
 /* The max length of strings including NUL: set and type identifiers */
 #define IPSET_MAXNAMELEN   32
@@ -38,17 +39,19 @@ enum ipset_cmd {
IPSET_CMD_TEST, /* 11: Test an element in a set */
IPSET_CMD_HEADER,   /* 12: Get set header data only */
IPSET_CMD_TYPE, /* 13: Get set type */
+   IPSET_CMD_GET_BYNAME,   /* 14: Get set index by name */
+   IPSET_CMD_GET_BYINDEX,  /* 15: Get set name by index */
IPSET_MSG_MAX,  /* Netlink message commands */
 
/* Commands in userspace: */
-   IPSET_CMD_RESTORE = IPSET_MSG_MAX, /* 14: Enter restore mode */
-   IPSET_CMD_HELP, /* 15: Get help */
-   IPSET_CMD_VERSION,  /* 16: Get program version */
-   IPSET_CMD_QUIT, /* 17: Quit from interactive mode */
+   IPSET_CMD_RESTORE = IPSET_MSG_MAX, /* 16: Enter restore mode */
+   IPSET_CMD_HELP, /* 17: Get help */
+   IPSET_CMD_VERSION,  /* 18: Get program version */
+   IPSET_CMD_QUIT, /* 19: Quit from interactive mode */
 
IPSET_CMD_MAX,
 
-   IPSET_CMD_COMMIT = IPSET_CMD_MAX, /* 18: Commit buffered commands */
+   IPSET_CMD_COMMIT = IPSET_CMD_MAX, /* 20: Commit buffered commands */
 };
 
 /* Attributes at command level */
@@ -66,6 +69,7 @@ enum {
IPSET_ATTR_LINENO,  /* 9: Restore lineno */
IPSET_ATTR_PROTOCOL_MIN, /* 10: Minimal supported version number */
IPSET_ATTR_REVISION_MIN = IPSET_ATTR_PROTOCOL_MIN, /* type rev min */
+   IPSET_ATTR_INDEX,   /* 11: Kernel index of set */
__IPSET_ATTR_CMD_MAX,
 };
 #define IPSET_ATTR_CMD_MAX (__IPSET_ATTR_CMD_MAX - 1)
@@ -223,6 +227,7 @@ enum ipset_adt {
 
 /* Sets are identified by an index in kernel space. Tweak with ip_s

Re: [PATCH v3] ipset: list:set: Decrease refcount synchronously on deletion and replace

[Jozsef Kadlecsik]

On Tue, 17 Jul 2018, Stefano Brivio wrote:

> > Thanks, patch is applied in the ipset git tree.
> 
> Sorry for the dumb question, I'm not really used to the ipset git 
> workflow: I've seen it's in the kernel/ directory of your netfilter.org 
> repository. Will this go into the nf git tree next?

The usual workflow is that I collect several ipset patches and then send 
the batch for the nf or nf-next tree, depending on the content of the 
patches. So yes, it'll go into nf.

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] netfilter: ipset: export indexes via netlink

[Jozsef Kadlecsik]

Hi Florent,

On Mon, 16 Jul 2018, Florent Fourcot wrote:

> > Technically I have no problem with your patch. However, it means a 
> > non-versioned protocol change. I'd like to think about it and check 
> > how would be best to introduce a version change.
> 
> Do you have any update on this? In my opinion, there are already some 
> flags to control list output (LIST_SETNAME and LIST_HEADER), and adding 
> one more is not really a breaking change.

Controlling list output is not the same: kernel versions which do not know 
the flags simply list the whole sets and userspace handles it fine. The 
only drawback is the unnecessary data transfer.

However with your proposed flag, additional data is returned. I'm 
concerned about backward compatilibity. What happens when a new userspace 
tool communicates with an old kernel? The only way to detect the 
incompatility is to check that the anticipated attributes are missing. But 
ipset strictly checks the attributes and reports protocol violations. With 
this solution there's no way to tell the difference between an old kernel 
or broken protocol.

> If you have any hints/idea to improve this patch, I can try to provide a 
> new version.

I want to introduce a new protocol version. Both the kernel and userspace 
have got the very basic parts to support multiple protocols, however have 
never been tested, obviously. Also, I'm thinking on adding two new 
commands to get the set by name and index and not a single one for both 
operations. The whole thing needs time and I'm busy these weeks with a 
cluster migration.

With a new protocol version, new userspace tools can "negotiate" the old 
protocol version with old kernels and can easily fall back to the 
getsockopt solution to get the required data from kernel.

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] ipset: list:set: Decrease refcount synchronously on deletion and replace

[Jozsef Kadlecsik]

Hi Stefano,

On Sat, 14 Jul 2018, Stefano Brivio wrote:

> Commit 45040978c899 ("netfilter: ipset: Fix set:list type crash
> when flush/dump set in parallel") postponed decreasing set
> reference counters to the RCU callback.
> 
> An 'ipset del' command can terminate before the RCU grace period
> is elapsed, and if sets are listed before then, the reference
> counter shown in userspace will be wrong:
> 
>  # ipset create h hash:ip; ipset create l list:set; ipset add l
>  # ipset del l h; ipset list h
>  Name: h
>  Type: hash:ip
>  Revision: 4
>  Header: family inet hashsize 1024 maxelem 65536
>  Size in memory: 88
>  References: 1
>  Number of entries: 0
>  Members:
>  # sleep 1; ipset list h
>  Name: h
>  Type: hash:ip
>  Revision: 4
>  Header: family inet hashsize 1024 maxelem 65536
>  Size in memory: 88
>  References: 0
>  Number of entries: 0
>  Members:
> 
> Fix this by making the reference count update synchronous again.
> 
> As a result, when sets are listed, ip_set_name_byindex() might
> now fetch a set whose reference count is already zero. Instead
> of relying on the reference count to protect against concurrent
> set renaming, grab ip_set_ref_lock as reader and copy the name,
> while holding the same lock in ip_set_rename() as writer
> instead.

Thanks, patch is applied in the ipset git tree.

Best regards,
Jozsef

> Reported-by: Li Shuang 
> Fixes: 45040978c899 ("netfilter: ipset: Fix set:list type crash when 
> flush/dump set in parallel")
> Signed-off-by: Stefano Brivio 
> ---
> v3:
>   - As suggested by Jozsef, don't hold ref_netlink in
> ip_set_name_byindex(), grabbing ip_set_ref_lock as reader is
> enough, if we hold ip_set_ref_lock as writer in ip_set_rename()
>   - Don't add __ip_set_get_netlink() back, we don't need it anymore
>   - Also as pointed out by Jozsef, in list_set_del() and
> list_set_replace(), we should decrease the reference counter only
> after we're done removing the set from or swapping it in the list
> 
> v2: As pointed out by Jozsef, we can't assume that the nfnl mutex
> is taken while dumping, so we need some explicit locking
> to protect names of list:set members against changes
> 
>  include/linux/netfilter/ipset/ip_set.h |  2 +-
>  net/netfilter/ipset/ip_set_core.c  | 23 +++
>  net/netfilter/ipset/ip_set_list_set.c  | 17 +++--
>  3 files changed, 23 insertions(+), 19 deletions(-)
> 
> diff --git a/include/linux/netfilter/ipset/ip_set.h 
> b/include/linux/netfilter/ipset/ip_set.h
> index 34fc80f3eb90..1d100efe74ec 100644
> --- a/include/linux/netfilter/ipset/ip_set.h
> +++ b/include/linux/netfilter/ipset/ip_set.h
> @@ -314,7 +314,7 @@ enum {
>  extern ip_set_id_t ip_set_get_byname(struct net *net,
>const char *name, struct ip_set **set);
>  extern void ip_set_put_byindex(struct net *net, ip_set_id_t index);
> -extern const char *ip_set_name_byindex(struct net *net, ip_set_id_t index);
> +extern void ip_set_name_byindex(struct net *net, ip_set_id_t index, char 
> *name);
>  extern ip_set_id_t ip_set_nfnl_get_byindex(struct net *net, ip_set_id_t 
> index);
>  extern void ip_set_nfnl_put(struct net *net, ip_set_id_t index);
>  
> diff --git a/net/netfilter/ipset/ip_set_core.c 
> b/net/netfilter/ipset/ip_set_core.c
> index bc4bd247bb7d..fa15a831aeee 100644
> --- a/net/netfilter/ipset/ip_set_core.c
> +++ b/net/netfilter/ipset/ip_set_core.c
> @@ -693,21 +693,20 @@ ip_set_put_byindex(struct net *net, ip_set_id_t index)
>  EXPORT_SYMBOL_GPL(ip_set_put_byindex);
>  
>  /* Get the name of a set behind a set index.
> - * We assume the set is referenced, so it does exist and
> - * can't be destroyed. The set cannot be renamed due to
> - * the referencing either.
> - *
> + * Set itself is protected by RCU, but its name isn't: to protect against
> + * renaming, grab ip_set_ref_lock as reader (see ip_set_rename()) and copy 
> the
> + * name.
>   */
> -const char *
> -ip_set_name_byindex(struct net *net, ip_set_id_t index)
> +void
> +ip_set_name_byindex(struct net *net, ip_set_id_t index, char *name)
>  {
> - const struct ip_set *set = ip_set_rcu_get(net, index);
> + struct ip_set *set = ip_set_rcu_get(net, index);
>  
>   BUG_ON(!set);
> - BUG_ON(set->ref == 0);
>  
> - /* Referenced, so it's safe */
> - return set->name;
> + read_lock_bh(_set_ref_lock);
> + strncpy(name, set->name, IPSET_MAXNAMELEN);
> + read_unlock_bh(_set_ref_lock);
>  }
>  EXPORT_SYMBOL_GPL(ip_set_name_byindex);
>  
> @@ -1153,7 +1152,7 @@ static int ip_set_rename(struct net *net, struct sock 
> *ctnl,
>   if (!set)
>   return -ENOENT;
>  
> - read_lock_bh(_set_ref_lock);
> + write_lock_bh(_set_ref_lock);
>   if (set->ref != 0) {
>   ret = -IPSET_ERR_REFERENCED;
>   goto out;
> @@ -1170,7 +1169,7 @@ static int ip_set_rename(struct net *net, struct sock 
> *ctnl,
>   strncpy(set->name, name2, IPSET_MAXNAMELEN);
>  
>  out:
> 

Re: [PATCH v2] ipset: list:set: Decrease refcount synchronously on deletion and replace

[Jozsef Kadlecsik]

On Sat, 14 Jul 2018, Jozsef Kadlecsik wrote:

> Hi Stefano,
> 
> On Sun, 8 Jul 2018, Stefano Brivio wrote:
> 
> > Commit 45040978c899 ("netfilter: ipset: Fix set:list type crash
> > when flush/dump set in parallel") postponed decreasing set
> > reference counters to the RCU callback.
> > 
> > An 'ipset del' command can terminate before the RCU grace period
> > is elapsed, and if sets are listed before then, the reference
> > counter shown in userspace will be wrong:
> > 
> >  # ipset create h hash:ip; ipset create l list:set; ipset add l
> >  # ipset del l h; ipset list h
> >  Name: h
> >  Type: hash:ip
> >  Revision: 4
> >  Header: family inet hashsize 1024 maxelem 65536
> >  Size in memory: 88
> >  References: 1
> >  Number of entries: 0
> >  Members:
> >  # sleep 1; ipset list h
> >  Name: h
> >  Type: hash:ip
> >  Revision: 4
> >  Header: family inet hashsize 1024 maxelem 65536
> >  Size in memory: 88
> >  References: 0
> >  Number of entries: 0
> >  Members:
> > 
> > Fix this by making the reference count update synchronous again.
> > 
> > As a result, when sets are listed, ip_set_name_byindex() might
> > now fetch a set whose reference count is already zero. Instead
> > of relying on the reference count to protect against concurrent
> > set renaming, grab the netlink refcount while copying the name,
> > and avoid renaming if the netlink refcount is taken, in a
> > similar way as it's already done for swap and delete operations.
> > 
> > This also reverts commit db268d4dfdb9 ("ipset: remove unused
> > function __ip_set_get_netlink") and restores that function
> > since we now need it.
> > 
> > Reported-by: Li Shuang 
> > Fixes: 45040978c899 ("netfilter: ipset: Fix set:list type crash when 
> > flush/dump set in parallel")
> > Signed-off-by: Stefano Brivio 
> > ---
> > v2: As pointed out by Joszef, we can't assume that the nfnl mutex
> > is taken while dumping, so we need some explicit locking
> > to protect names of list:set members against changes
> > 
> >  include/linux/netfilter/ipset/ip_set.h |  2 +-
> >  net/netfilter/ipset/ip_set_core.c  | 31 +--
> >  net/netfilter/ipset/ip_set_list_set.c  | 17 +++--
> >  3 files changed, 33 insertions(+), 17 deletions(-)
> > 
> > diff --git a/include/linux/netfilter/ipset/ip_set.h 
> > b/include/linux/netfilter/ipset/ip_set.h
> > index 34fc80f3eb90..1d100efe74ec 100644
> > --- a/include/linux/netfilter/ipset/ip_set.h
> > +++ b/include/linux/netfilter/ipset/ip_set.h
> > @@ -314,7 +314,7 @@ enum {
> >  extern ip_set_id_t ip_set_get_byname(struct net *net,
> >  const char *name, struct ip_set **set);
> >  extern void ip_set_put_byindex(struct net *net, ip_set_id_t index);
> > -extern const char *ip_set_name_byindex(struct net *net, ip_set_id_t index);
> > +extern void ip_set_name_byindex(struct net *net, ip_set_id_t index, char 
> > *name);
> >  extern ip_set_id_t ip_set_nfnl_get_byindex(struct net *net, ip_set_id_t 
> > index);
> >  extern void ip_set_nfnl_put(struct net *net, ip_set_id_t index);
> >  
> > diff --git a/net/netfilter/ipset/ip_set_core.c 
> > b/net/netfilter/ipset/ip_set_core.c
> > index bc4bd247bb7d..4062c0396a02 100644
> > --- a/net/netfilter/ipset/ip_set_core.c
> > +++ b/net/netfilter/ipset/ip_set_core.c
> > @@ -527,6 +527,15 @@ __ip_set_put(struct ip_set *set)
> >  /* set->ref can be swapped out by ip_set_swap, netlink events (like dump) 
> > need
> >   * a separate reference counter
> >   */
> > +static inline void
> > +__ip_set_get_netlink(struct ip_set *set)
> > +{
> > +   write_lock_bh(_set_ref_lock);
> > +   BUG_ON(set->ref_netlink != 0);
> > +   set->ref_netlink++;
> > +   write_unlock_bh(_set_ref_lock);
> > +}
> > +
> >  static inline void
> >  __ip_set_put_netlink(struct ip_set *set)
> >  {
> > @@ -693,21 +702,19 @@ ip_set_put_byindex(struct net *net, ip_set_id_t index)
> >  EXPORT_SYMBOL_GPL(ip_set_put_byindex);
> >  
> >  /* Get the name of a set behind a set index.
> > - * We assume the set is referenced, so it does exist and
> > - * can't be destroyed. The set cannot be renamed due to
> > - * the referencing either.
> > - *
> > + * Set itself is protected by RCU, but its name isn't: to protect against
> > + * renaming, grab its netlink refcount (see ip_set_rename()) and copy it.
> > 

Re: [PATCH v2] ipset: list:set: Decrease refcount synchronously on deletion and replace

[Jozsef Kadlecsik]

Hi Stefano,

On Sun, 8 Jul 2018, Stefano Brivio wrote:

> Commit 45040978c899 ("netfilter: ipset: Fix set:list type crash
> when flush/dump set in parallel") postponed decreasing set
> reference counters to the RCU callback.
> 
> An 'ipset del' command can terminate before the RCU grace period
> is elapsed, and if sets are listed before then, the reference
> counter shown in userspace will be wrong:
> 
>  # ipset create h hash:ip; ipset create l list:set; ipset add l
>  # ipset del l h; ipset list h
>  Name: h
>  Type: hash:ip
>  Revision: 4
>  Header: family inet hashsize 1024 maxelem 65536
>  Size in memory: 88
>  References: 1
>  Number of entries: 0
>  Members:
>  # sleep 1; ipset list h
>  Name: h
>  Type: hash:ip
>  Revision: 4
>  Header: family inet hashsize 1024 maxelem 65536
>  Size in memory: 88
>  References: 0
>  Number of entries: 0
>  Members:
> 
> Fix this by making the reference count update synchronous again.
> 
> As a result, when sets are listed, ip_set_name_byindex() might
> now fetch a set whose reference count is already zero. Instead
> of relying on the reference count to protect against concurrent
> set renaming, grab the netlink refcount while copying the name,
> and avoid renaming if the netlink refcount is taken, in a
> similar way as it's already done for swap and delete operations.
> 
> This also reverts commit db268d4dfdb9 ("ipset: remove unused
> function __ip_set_get_netlink") and restores that function
> since we now need it.
> 
> Reported-by: Li Shuang 
> Fixes: 45040978c899 ("netfilter: ipset: Fix set:list type crash when 
> flush/dump set in parallel")
> Signed-off-by: Stefano Brivio 
> ---
> v2: As pointed out by Joszef, we can't assume that the nfnl mutex
> is taken while dumping, so we need some explicit locking
> to protect names of list:set members against changes
> 
>  include/linux/netfilter/ipset/ip_set.h |  2 +-
>  net/netfilter/ipset/ip_set_core.c  | 31 +--
>  net/netfilter/ipset/ip_set_list_set.c  | 17 +++--
>  3 files changed, 33 insertions(+), 17 deletions(-)
> 
> diff --git a/include/linux/netfilter/ipset/ip_set.h 
> b/include/linux/netfilter/ipset/ip_set.h
> index 34fc80f3eb90..1d100efe74ec 100644
> --- a/include/linux/netfilter/ipset/ip_set.h
> +++ b/include/linux/netfilter/ipset/ip_set.h
> @@ -314,7 +314,7 @@ enum {
>  extern ip_set_id_t ip_set_get_byname(struct net *net,
>const char *name, struct ip_set **set);
>  extern void ip_set_put_byindex(struct net *net, ip_set_id_t index);
> -extern const char *ip_set_name_byindex(struct net *net, ip_set_id_t index);
> +extern void ip_set_name_byindex(struct net *net, ip_set_id_t index, char 
> *name);
>  extern ip_set_id_t ip_set_nfnl_get_byindex(struct net *net, ip_set_id_t 
> index);
>  extern void ip_set_nfnl_put(struct net *net, ip_set_id_t index);
>  
> diff --git a/net/netfilter/ipset/ip_set_core.c 
> b/net/netfilter/ipset/ip_set_core.c
> index bc4bd247bb7d..4062c0396a02 100644
> --- a/net/netfilter/ipset/ip_set_core.c
> +++ b/net/netfilter/ipset/ip_set_core.c
> @@ -527,6 +527,15 @@ __ip_set_put(struct ip_set *set)
>  /* set->ref can be swapped out by ip_set_swap, netlink events (like dump) 
> need
>   * a separate reference counter
>   */
> +static inline void
> +__ip_set_get_netlink(struct ip_set *set)
> +{
> + write_lock_bh(_set_ref_lock);
> + BUG_ON(set->ref_netlink != 0);
> + set->ref_netlink++;
> + write_unlock_bh(_set_ref_lock);
> +}
> +
>  static inline void
>  __ip_set_put_netlink(struct ip_set *set)
>  {
> @@ -693,21 +702,19 @@ ip_set_put_byindex(struct net *net, ip_set_id_t index)
>  EXPORT_SYMBOL_GPL(ip_set_put_byindex);
>  
>  /* Get the name of a set behind a set index.
> - * We assume the set is referenced, so it does exist and
> - * can't be destroyed. The set cannot be renamed due to
> - * the referencing either.
> - *
> + * Set itself is protected by RCU, but its name isn't: to protect against
> + * renaming, grab its netlink refcount (see ip_set_rename()) and copy it.
>   */
> -const char *
> -ip_set_name_byindex(struct net *net, ip_set_id_t index)
> +void
> +ip_set_name_byindex(struct net *net, ip_set_id_t index, char *name)
>  {
> - const struct ip_set *set = ip_set_rcu_get(net, index);
> + struct ip_set *set = ip_set_rcu_get(net, index);
>  
>   BUG_ON(!set);
> - BUG_ON(set->ref == 0);
>  
> - /* Referenced, so it's safe */
> - return set->name;
> + __ip_set_get_netlink(set);
> + strncpy(name, set->name, IPSET_MAXNAMELEN);
> + __ip_set_put_netlink(set);
>  }
>  EXPORT_SYMBOL_GPL(ip_set_name_byindex);

This is my main concern about the patch: it boils down to *four* locking 
operations, for every single list members in a list type of set. It costs 
too much!

ip_set_name_byindex() is used in ip_set_list_set.c only, so please use 
write_lock_bh()/write_unlock_bh() directly, that should be sufficient.
 
> @@ -1158,6 +1165,10 @@ static int 

Re: [PATCH v2] ipset: list:set: Decrease refcount synchronously on deletion and replace

[Jozsef Kadlecsik]

Hi Stefano,

On Mon, 9 Jul 2018, Stefano Brivio wrote:

> On Mon, 9 Jul 2018 22:36:51 +0200 (CEST)
> Jozsef Kadlecsik  wrote:
> 
> > I believe the only modifications which are required are up till this point 
> > in ip_set_list_set.c: ip_set_put_byindex(), moved outside of the 
> > call_rcu() call. However, the ip_set_put_byindex() calls should be placed 
> > after list_del_rcu() and list_replace_rcu().
> 
> This was also the first approach I took. With the changes you
> suggested, I guess this should look like the diff at [1]. Is this what
> you meant?
> 
> However, list, flush and destroy are not serialised, so, with that
> patch, I do actually hit the BUG_ON(set->ref == 0) in
> ip_set_name_byindex(). That's why I think I can't rely on set->ref
> anymore with those changes:

Yes, you're right. I had some concerns with the second version of your 
patch and therefore looked for another way to solve the issue, badly.

So, back to the second version, see my comments in my next mail.

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] ipset: list:set: Decrease refcount synchronously on deletion and replace

[Jozsef Kadlecsik]

Hi Stefano,

On Sun, 8 Jul 2018, Stefano Brivio wrote:

> Commit 45040978c899 ("netfilter: ipset: Fix set:list type crash
> when flush/dump set in parallel") postponed decreasing set
> reference counters to the RCU callback.
> 
> An 'ipset del' command can terminate before the RCU grace period
> is elapsed, and if sets are listed before then, the reference
> counter shown in userspace will be wrong:
> 
>  # ipset create h hash:ip; ipset create l list:set; ipset add l
>  # ipset del l h; ipset list h
>  Name: h
>  Type: hash:ip
>  Revision: 4
>  Header: family inet hashsize 1024 maxelem 65536
>  Size in memory: 88
>  References: 1
>  Number of entries: 0
>  Members:
>  # sleep 1; ipset list h
>  Name: h
>  Type: hash:ip
>  Revision: 4
>  Header: family inet hashsize 1024 maxelem 65536
>  Size in memory: 88
>  References: 0
>  Number of entries: 0
>  Members:
> 
> Fix this by making the reference count update synchronous again.
> 
> As a result, when sets are listed, ip_set_name_byindex() might
> now fetch a set whose reference count is already zero. Instead
> of relying on the reference count to protect against concurrent
> set renaming, grab the netlink refcount while copying the name,
> and avoid renaming if the netlink refcount is taken, in a
> similar way as it's already done for swap and delete operations.
> 
> This also reverts commit db268d4dfdb9 ("ipset: remove unused
> function __ip_set_get_netlink") and restores that function
> since we now need it.
> 
> Reported-by: Li Shuang 
> Fixes: 45040978c899 ("netfilter: ipset: Fix set:list type crash when 
> flush/dump set in parallel")
> Signed-off-by: Stefano Brivio 
> ---
> v2: As pointed out by Joszef, we can't assume that the nfnl mutex
> is taken while dumping, so we need some explicit locking
> to protect names of list:set members against changes
> 
>  include/linux/netfilter/ipset/ip_set.h |  2 +-
>  net/netfilter/ipset/ip_set_core.c  | 31 +--
>  net/netfilter/ipset/ip_set_list_set.c  | 17 +++--
>  3 files changed, 33 insertions(+), 17 deletions(-)
> 
> diff --git a/include/linux/netfilter/ipset/ip_set.h 
> b/include/linux/netfilter/ipset/ip_set.h
> index 34fc80f3eb90..1d100efe74ec 100644
> --- a/include/linux/netfilter/ipset/ip_set.h
> +++ b/include/linux/netfilter/ipset/ip_set.h
> @@ -314,7 +314,7 @@ enum {
>  extern ip_set_id_t ip_set_get_byname(struct net *net,
>const char *name, struct ip_set **set);
>  extern void ip_set_put_byindex(struct net *net, ip_set_id_t index);
> -extern const char *ip_set_name_byindex(struct net *net, ip_set_id_t index);
> +extern void ip_set_name_byindex(struct net *net, ip_set_id_t index, char 
> *name);
>  extern ip_set_id_t ip_set_nfnl_get_byindex(struct net *net, ip_set_id_t 
> index);
>  extern void ip_set_nfnl_put(struct net *net, ip_set_id_t index);
>  
> diff --git a/net/netfilter/ipset/ip_set_core.c 
> b/net/netfilter/ipset/ip_set_core.c
> index bc4bd247bb7d..4062c0396a02 100644
> --- a/net/netfilter/ipset/ip_set_core.c
> +++ b/net/netfilter/ipset/ip_set_core.c
> @@ -527,6 +527,15 @@ __ip_set_put(struct ip_set *set)
>  /* set->ref can be swapped out by ip_set_swap, netlink events (like dump) 
> need
>   * a separate reference counter
>   */
> +static inline void
> +__ip_set_get_netlink(struct ip_set *set)
> +{
> + write_lock_bh(_set_ref_lock);
> + BUG_ON(set->ref_netlink != 0);
> + set->ref_netlink++;
> + write_unlock_bh(_set_ref_lock);
> +}
> +
>  static inline void
>  __ip_set_put_netlink(struct ip_set *set)
>  {
> @@ -693,21 +702,19 @@ ip_set_put_byindex(struct net *net, ip_set_id_t index)
>  EXPORT_SYMBOL_GPL(ip_set_put_byindex);
>  
>  /* Get the name of a set behind a set index.
> - * We assume the set is referenced, so it does exist and
> - * can't be destroyed. The set cannot be renamed due to
> - * the referencing either.
> - *
> + * Set itself is protected by RCU, but its name isn't: to protect against
> + * renaming, grab its netlink refcount (see ip_set_rename()) and copy it.
>   */
> -const char *
> -ip_set_name_byindex(struct net *net, ip_set_id_t index)
> +void
> +ip_set_name_byindex(struct net *net, ip_set_id_t index, char *name)
>  {
> - const struct ip_set *set = ip_set_rcu_get(net, index);
> + struct ip_set *set = ip_set_rcu_get(net, index);
>  
>   BUG_ON(!set);
> - BUG_ON(set->ref == 0);
>  
> - /* Referenced, so it's safe */
> - return set->name;
> + __ip_set_get_netlink(set);
> + strncpy(name, set->name, IPSET_MAXNAMELEN);
> + __ip_set_put_netlink(set);
>  }
>  EXPORT_SYMBOL_GPL(ip_set_name_byindex);

These are not required: when we call ip_set_name_byindex(), the set is 
still referenced (set->ref cannot be zero), because it's the member of the
list type of set.
  
> @@ -1158,6 +1165,10 @@ static int ip_set_rename(struct net *net, struct sock 
> *ctnl,
>   ret = -IPSET_ERR_REFERENCED;
>   goto out;
>   }
> + if 

Re: [PATCH] netfilter: ipset: export indexes via netlink

[Jozsef Kadlecsik]

Hi,

On Thu, 5 Jul 2018, Florent Fourcot wrote:

> Indexes are exported through getsockopt calls (IP_SET_OP_GET_BYNAME)
> and are mandatory for external subsystem using ipset:
>   * ipset module of tc-ematch (configured by netlink, but using
> getsockopt before to get index)
>   * SET netfilter module
> 
> The goal of this patch is to allow one user to use only netlink to get
> ipset indexes. However, since `ipset` userspace command does not accept
> new/unknow nla (structure didn't change since years), a new flag is
> introduced to ask for more data. Currently it adds only indexes, but
> application setting the flag should be ready to accept new nla in
> future.

Technically I have no problem with your patch. However, it means a 
non-versioned protocol change. I'd like to think about it and check how 
would be best to introduce a version change.

Best regards,
Jozsef
 
> Signed-off-by: Florent Fourcot 
> Signed-off-by: Victorien Molle 
> ---
>  include/uapi/linux/netfilter/ipset/ip_set.h |  4 
>  net/netfilter/ipset/ip_set_core.c   | 18 +-
>  2 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/netfilter/ipset/ip_set.h 
> b/include/uapi/linux/netfilter/ipset/ip_set.h
> index 60236f694143..8ef2560ff69e 100644
> --- a/include/uapi/linux/netfilter/ipset/ip_set.h
> +++ b/include/uapi/linux/netfilter/ipset/ip_set.h
> @@ -66,6 +66,8 @@ enum {
>   IPSET_ATTR_LINENO,  /* 9: Restore lineno */
>   IPSET_ATTR_PROTOCOL_MIN, /* 10: Minimal supported version number */
>   IPSET_ATTR_REVISION_MIN = IPSET_ATTR_PROTOCOL_MIN, /* type rev min */
> + /* attributes not sent by default (see IPSET_FLAG_EXTRA_DATA) */
> + IPSET_ATTR_INDEX,   /* 11: Index of the set */
>   __IPSET_ATTR_CMD_MAX,
>  };
>  #define IPSET_ATTR_CMD_MAX   (__IPSET_ATTR_CMD_MAX - 1)
> @@ -182,6 +184,8 @@ enum ipset_cmd_flags {
>   IPSET_FLAG_MAP_SKBPRIO = (1 << IPSET_FLAG_BIT_MAP_SKBPRIO),
>   IPSET_FLAG_BIT_MAP_SKBQUEUE = 10,
>   IPSET_FLAG_MAP_SKBQUEUE = (1 << IPSET_FLAG_BIT_MAP_SKBQUEUE),
> + IPSET_FLAG_BIT_EXTRA_DATA = 11,
> + IPSET_FLAG_EXTRA_DATA = (1 << IPSET_FLAG_BIT_EXTRA_DATA),
>   IPSET_FLAG_CMD_MAX = 15,
>  };
>  
> diff --git a/net/netfilter/ipset/ip_set_core.c 
> b/net/netfilter/ipset/ip_set_core.c
> index bc4bd247bb7d..370b79368ddb 100644
> --- a/net/netfilter/ipset/ip_set_core.c
> +++ b/net/netfilter/ipset/ip_set_core.c
> @@ -1409,6 +1409,11 @@ ip_set_dump_start(struct sk_buff *skb, struct 
> netlink_callback *cb)
>   goto release_refcount;
>   if (dump_flags & IPSET_FLAG_LIST_HEADER)
>   goto next_set;
> + if (dump_flags & IPSET_FLAG_EXTRA_DATA) {
> + if (nla_put_u16(skb, IPSET_ATTR_INDEX,
> + index))
> + goto nla_put_failure;
> + }
>   if (set->variant->uref)
>   set->variant->uref(set, cb, true);
>   /* fall through */
> @@ -1695,6 +1700,7 @@ static int ip_set_header(struct net *net, struct sock 
> *ctnl,
>const struct nlattr * const attr[],
>struct netlink_ext_ack *extack)
>  {
> + ip_set_id_t index;
>   struct ip_set_net *inst = ip_set_pernet(net);
>   const struct ip_set *set;
>   struct sk_buff *skb2;
> @@ -1705,7 +1711,7 @@ static int ip_set_header(struct net *net, struct sock 
> *ctnl,
>!attr[IPSET_ATTR_SETNAME]))
>   return -IPSET_ERR_PROTOCOL;
>  
> - set = find_set(inst, nla_data(attr[IPSET_ATTR_SETNAME]));
> + set = find_set_and_id(inst, nla_data(attr[IPSET_ATTR_SETNAME]), );
>   if (!set)
>   return -ENOENT;
>  
> @@ -1723,6 +1729,16 @@ static int ip_set_header(struct net *net, struct sock 
> *ctnl,
>   nla_put_u8(skb2, IPSET_ATTR_FAMILY, set->family) ||
>   nla_put_u8(skb2, IPSET_ATTR_REVISION, set->revision))
>   goto nla_put_failure;
> +
> + if (attr[IPSET_ATTR_FLAGS]) {
> + u32 flags = ip_set_get_h32(attr[IPSET_ATTR_FLAGS]);
> +
> + if (flags & IPSET_FLAG_EXTRA_DATA) {
> + if (nla_put_u16(skb2, IPSET_ATTR_INDEX, index))
> + goto nla_put_failure;
> + }
> + }
> +
>   nlmsg_end(skb2, nlh2);
>  
>   ret = netlink_unicast(ctnl, skb2, NETLINK_CB(skb).portid, MSG_DONTWAIT);
> -- 
> 2.11.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian 

Re: [PATCH] netfilter: ipset: list:set: Decrease refcount synchronously on deletion and replace

[Jozsef Kadlecsik]

Hi,

On Thu, 5 Jul 2018, Stefano Brivio wrote:

> > Only the netlink_recvmsg() first call is protected under nfnl lock, 
> > follow up calls happen from the netlink_dump() path which in netfilter 
> > is rcu based.
> 
> Of course, I see now, thanks for the explanation!
> 
> > We have callbacks in nfnetlink to achieve full rcu dumps.
> 
> So I guess you are suggesting to change ipset dumps to use those 
> callbacks.

ipset uses the same infrastructure, callbacks. Dumping is quite different 
from the other operations in the netlink world.
 
> Unless Jozsef has any objection, I would, at least for the moment
> being, fix this specific issue in another way (i.e. using the
> ref_netlink refcount as it's already done for swap and delete
> operations), and then at some point consider again this idea.

No objections from me :-).

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] netfilter: ipset: list:set: Decrease refcount synchronously on deletion and replace

[Jozsef Kadlecsik]

Hi,

On Thu, 28 Jun 2018, Stefano Brivio wrote:

> Commit 45040978c899 ("netfilter: ipset: Fix set:list type crash
> when flush/dump set in parallel") postponed decreasing set
> reference counters to the RCU callback.
> 
> An 'ipset del' command can terminate before the RCU grace period
> is elapsed, and if sets are listed before then, the reference
> counter shown in userspace will be wrong:
> 
>  # ipset create h hash:ip; ipset create l list:set; ipset add l
>  # ipset del l h; ipset list h
>  Name: h
>  Type: hash:ip
>  Revision: 4
>  Header: family inet hashsize 1024 maxelem 65536
>  Size in memory: 88
>  References: 1
>  Number of entries: 0
>  Members:
>  # sleep 1; ipset list h
>  Name: h
>  Type: hash:ip
>  Revision: 4
>  Header: family inet hashsize 1024 maxelem 65536
>  Size in memory: 88
>  References: 0
>  Number of entries: 0
>  Members:
> 
> Fix this by making the reference count update synchronous again.
> 
> As a result, when sets are listed, ip_set_name_byindex() might
> now fetch a set whose reference count is already zero. Instead
> of relying on the reference count to protect against concurrent
> set renaming and listing, note that those two operations are
> serialised by the nfnl mutex, and that the set itself is
> protected by RCU nowadays.

Listing is not serialized by the nfnl mutex because a netlink dump is used 
behind it. So I believe the patch is not correct and therefore I cannot 
apply it.

Best regards,
Jozsef
 
> Reported-by: Li Shuang 
> Fixes: 45040978c899 ("netfilter: ipset: Fix set:list type crash when 
> flush/dump set in parallel")
> Signed-off-by: Stefano Brivio 
> ---
>  net/netfilter/ipset/ip_set_core.c |  7 +--
>  net/netfilter/ipset/ip_set_list_set.c | 12 
>  2 files changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/net/netfilter/ipset/ip_set_core.c 
> b/net/netfilter/ipset/ip_set_core.c
> index bc4bd247bb7d..afe42af0ad13 100644
> --- a/net/netfilter/ipset/ip_set_core.c
> +++ b/net/netfilter/ipset/ip_set_core.c
> @@ -693,10 +693,7 @@ ip_set_put_byindex(struct net *net, ip_set_id_t index)
>  EXPORT_SYMBOL_GPL(ip_set_put_byindex);
>  
>  /* Get the name of a set behind a set index.
> - * We assume the set is referenced, so it does exist and
> - * can't be destroyed. The set cannot be renamed due to
> - * the referencing either.
> - *
> + * This can only be called by operations serialised by nfnl mutex.
>   */
>  const char *
>  ip_set_name_byindex(struct net *net, ip_set_id_t index)
> @@ -704,9 +701,7 @@ ip_set_name_byindex(struct net *net, ip_set_id_t index)
>   const struct ip_set *set = ip_set_rcu_get(net, index);
>  
>   BUG_ON(!set);
> - BUG_ON(set->ref == 0);
>  
> - /* Referenced, so it's safe */
>   return set->name;
>  }
>  EXPORT_SYMBOL_GPL(ip_set_name_byindex);
> diff --git a/net/netfilter/ipset/ip_set_list_set.c 
> b/net/netfilter/ipset/ip_set_list_set.c
> index 072a658fde04..9a056729968f 100644
> --- a/net/netfilter/ipset/ip_set_list_set.c
> +++ b/net/netfilter/ipset/ip_set_list_set.c
> @@ -148,9 +148,7 @@ __list_set_del_rcu(struct rcu_head * rcu)
>  {
>   struct set_elem *e = container_of(rcu, struct set_elem, rcu);
>   struct ip_set *set = e->set;
> - struct list_set *map = set->data;
>  
> - ip_set_put_byindex(map->net, e->id);
>   ip_set_ext_destroy(set, e);
>   kfree(e);
>  }
> @@ -158,14 +156,20 @@ __list_set_del_rcu(struct rcu_head * rcu)
>  static inline void
>  list_set_del(struct ip_set *set, struct set_elem *e)
>  {
> + struct list_set *map = set->data;
> +
>   set->elements--;
> + ip_set_put_byindex(map->net, e->id);
>   list_del_rcu(>list);
>   call_rcu(>rcu, __list_set_del_rcu);
>  }
>  
>  static inline void
> -list_set_replace(struct set_elem *e, struct set_elem *old)
> +list_set_replace(struct ip_set *set, struct set_elem *e, struct set_elem 
> *old)
>  {
> + struct list_set *map = set->data;
> +
> + ip_set_put_byindex(map->net, old->id);
>   list_replace_rcu(>list, >list);
>   call_rcu(>rcu, __list_set_del_rcu);
>  }
> @@ -298,7 +302,7 @@ list_set_uadd(struct ip_set *set, void *value, const 
> struct ip_set_ext *ext,
>   INIT_LIST_HEAD(>list);
>   list_set_init_extensions(set, ext, e);
>   if (n)
> - list_set_replace(e, n);
> + list_set_replace(set, e, n);
>   else if (next)
>   list_add_tail_rcu(>list, >list);
>   else if (prev)
> -- 
> 2.15.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe 

[PATCH 1/4] netfilter: xt_set: Check hook mask correctly

[Jozsef Kadlecsik]

From: Serhey Popovych 

Inserting rule before one with SET target we get error with warning in
dmesg(1) output:

  # iptables -A FORWARD -t mangle -j SET --map-set test src --map-prio
  # iptables -I FORWARD 1 -t mangle -j ACCEPT
  iptables: Invalid argument. Run `dmesg' for more information.
  # dmesg |tail -n1
  [268578.026643] mapping of prio or/and queue is allowed only from \
  OUTPUT/FORWARD/POSTROUTING chains

Rather than checking for supported hook bits for SET target check for
unsupported one as done in all rest of matches and targets.

Signed-off-by: Serhey Popovych 
Signed-off-by: Jozsef Kadlecsik 
---
 net/netfilter/xt_set.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/xt_set.c b/net/netfilter/xt_set.c
index 6f4c521..07af7db 100644
--- a/net/netfilter/xt_set.c
+++ b/net/netfilter/xt_set.c
@@ -470,7 +470,7 @@ set_target_v3_checkentry(const struct xt_tgchk_param *par)
}
if (((info->flags & IPSET_FLAG_MAP_SKBPRIO) |
 (info->flags & IPSET_FLAG_MAP_SKBQUEUE)) &&
-!(par->hook_mask & (1 << NF_INET_FORWARD |
+(par->hook_mask & ~(1 << NF_INET_FORWARD |
 1 << NF_INET_LOCAL_OUT |
 1 << NF_INET_POST_ROUTING))) {
pr_info_ratelimited("mapping of prio or/and queue is 
allowed only from OUTPUT/FORWARD/POSTROUTING chains\n");
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] netfilter: ipset: List timing out entries with "timeout 1" instead of zero

[Jozsef Kadlecsik]

When listing sets with timeout support, there's a probability that
just timing out entries with "0" timeout value is listed/saved.
However when restoring the saved list, the zero timeout value means
permanent elelements.

The new behaviour is that timing out entries are listed with "timeout 1"
instead of zero.

Fixes netfilter bugzilla #1258.

Signed-off-by: Jozsef Kadlecsik 
---
 include/linux/netfilter/ipset/ip_set_timeout.h | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/linux/netfilter/ipset/ip_set_timeout.h 
b/include/linux/netfilter/ipset/ip_set_timeout.h
index bfb3531..7ad8ddf 100644
--- a/include/linux/netfilter/ipset/ip_set_timeout.h
+++ b/include/linux/netfilter/ipset/ip_set_timeout.h
@@ -65,8 +65,14 @@ ip_set_timeout_set(unsigned long *timeout, u32 value)
 static inline u32
 ip_set_timeout_get(const unsigned long *timeout)
 {
-   return *timeout == IPSET_ELEM_PERMANENT ? 0 :
-   jiffies_to_msecs(*timeout - jiffies)/MSEC_PER_SEC;
+   u32 t;
+
+   if (*timeout == IPSET_ELEM_PERMANENT)
+   return 0;
+
+   t = jiffies_to_msecs(*timeout - jiffies)/MSEC_PER_SEC;
+   /* Zero value in userspace means no timeout */
+   return t == 0 ? 1 : t;
 }
 
 #endif /* __KERNEL__ */
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/4] netfilter: ipset: Limit max timeout value

[Jozsef Kadlecsik]

Due to the negative value condition in msecs_to_jiffies(), the real
max possible timeout value must be set to (UINT_MAX >> 1)/MSEC_PER_SEC.

Neutron Soutmun proposed the proper fix, but an insufficient one was
applied, see https://patchwork.ozlabs.org/patch/400405/.

Signed-off-by: Jozsef Kadlecsik 
---
 include/linux/netfilter/ipset/ip_set_timeout.h | 10 ++
 net/netfilter/xt_set.c |  8 
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/include/linux/netfilter/ipset/ip_set_timeout.h 
b/include/linux/netfilter/ipset/ip_set_timeout.h
index 7ad8ddf..8ce271e 100644
--- a/include/linux/netfilter/ipset/ip_set_timeout.h
+++ b/include/linux/netfilter/ipset/ip_set_timeout.h
@@ -23,6 +23,9 @@
 /* Set is defined with timeout support: timeout value may be 0 */
 #define IPSET_NO_TIMEOUT   UINT_MAX
 
+/* Max timeout value, see msecs_to_jiffies() in jiffies.h */
+#define IPSET_MAX_TIMEOUT  (UINT_MAX >> 1)/MSEC_PER_SEC
+
 #define ip_set_adt_opt_timeout(opt, set)   \
 ((opt)->ext.timeout != IPSET_NO_TIMEOUT ? (opt)->ext.timeout : (set)->timeout)
 
@@ -32,11 +35,10 @@ ip_set_timeout_uget(struct nlattr *tb)
unsigned int timeout = ip_set_get_h32(tb);
 
/* Normalize to fit into jiffies */
-   if (timeout > UINT_MAX/MSEC_PER_SEC)
-   timeout = UINT_MAX/MSEC_PER_SEC;
+   if (timeout > IPSET_MAX_TIMEOUT)
+   timeout = IPSET_MAX_TIMEOUT;
 
-   /* Userspace supplied TIMEOUT parameter: adjust crazy size */
-   return timeout == IPSET_NO_TIMEOUT ? IPSET_NO_TIMEOUT - 1 : timeout;
+   return timeout;
 }
 
 static inline bool
diff --git a/net/netfilter/xt_set.c b/net/netfilter/xt_set.c
index 07af7db..bf2890b 100644
--- a/net/netfilter/xt_set.c
+++ b/net/netfilter/xt_set.c
@@ -372,8 +372,8 @@ set_target_v2(struct sk_buff *skb, const struct 
xt_action_param *par)
 
/* Normalize to fit into jiffies */
if (add_opt.ext.timeout != IPSET_NO_TIMEOUT &&
-   add_opt.ext.timeout > UINT_MAX / MSEC_PER_SEC)
-   add_opt.ext.timeout = UINT_MAX / MSEC_PER_SEC;
+   add_opt.ext.timeout > IPSET_MAX_TIMEOUT)
+   add_opt.ext.timeout = IPSET_MAX_TIMEOUT;
if (info->add_set.index != IPSET_INVALID_ID)
ip_set_add(info->add_set.index, skb, par, _opt);
if (info->del_set.index != IPSET_INVALID_ID)
@@ -407,8 +407,8 @@ set_target_v3(struct sk_buff *skb, const struct 
xt_action_param *par)
 
/* Normalize to fit into jiffies */
if (add_opt.ext.timeout != IPSET_NO_TIMEOUT &&
-   add_opt.ext.timeout > UINT_MAX / MSEC_PER_SEC)
-   add_opt.ext.timeout = UINT_MAX / MSEC_PER_SEC;
+   add_opt.ext.timeout > IPSET_MAX_TIMEOUT)
+   add_opt.ext.timeout = IPSET_MAX_TIMEOUT;
if (info->add_set.index != IPSET_INVALID_ID)
ip_set_add(info->add_set.index, skb, par, _opt);
if (info->del_set.index != IPSET_INVALID_ID)
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/4] netfilter: ipset: forbid family for hash:mac sets

[Jozsef Kadlecsik]

From: Florent Fourcot 

Userspace `ipset` command forbids family option for hash:mac type:

ipset create test hash:mac family inet4
ipset v6.30: Unknown argument: `family'

However, this check is not done in kernel itself. When someone use
external netlink applications (pyroute2 python library for example), one
can create hash:mac with invalid family and inconsistant results from
userspace (`ipset` command cannot read set content anymore).

This patch enforce the logic in kernel, and forbids insertion of
hash:mac with a family set.

Since IP_SET_PROTO_UNDEF is defined only for hash:mac, this patch has no
impact on other hash:* sets

Signed-off-by: Florent Fourcot 
Signed-off-by: Victorien Molle 
Signed-off-by: Jozsef Kadlecsik 
---
 net/netfilter/ipset/ip_set_hash_gen.h | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/ipset/ip_set_hash_gen.h 
b/net/netfilter/ipset/ip_set_hash_gen.h
index bbad940..8a33dac 100644
--- a/net/netfilter/ipset/ip_set_hash_gen.h
+++ b/net/netfilter/ipset/ip_set_hash_gen.h
@@ -1234,7 +1234,10 @@ IPSET_TOKEN(HTYPE, _create)(struct net *net, struct 
ip_set *set,
pr_debug("Create set %s with family %s\n",
 set->name, set->family == NFPROTO_IPV4 ? "inet" : "inet6");
 
-#ifndef IP_SET_PROTO_UNDEF
+#ifdef IP_SET_PROTO_UNDEF
+   if (set->family != NFPROTO_UNSPEC)
+   return -IPSET_ERR_INVALID_FAMILY;
+#else
if (!(set->family == NFPROTO_IPV4 || set->family == NFPROTO_IPV6))
return -IPSET_ERR_INVALID_FAMILY;
 #endif
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/4] ipset patches for nf

[Jozsef Kadlecsik]

Hi Pablo,

Please pull the next patches for nf git tree:

- Check hook mask for unsupported hooks instead of supported ones in xt_set.
  (Serhey Popovych).
- List/save just timing out entries with "timeout 1" instead of "timeout 0":
  zero timeout value means permanent entries. When restoring the elements,
  we'd add non-timing out entries. Fixes netfilter bugzilla id #1258.
- Limit max timeout value to (UINT_MAX >> 1)/MSEC_PER_SEC due to the
  negative value condition in msecs_to_jiffies(). msecs_to_jiffies()
  should be revised: if one wants to set the timeout above 2147483,
  msecs_to_jiffies() sets the value to 4294967. (Reported by Maxim Masiutin).
- Forbid family for hash:mac sets in the kernel module: ipset userspace tool
  enforces it but third party tools could create sets with this parameter.
  Such sets then cannot be listed/saved with ipset itself. (Florent Fourcot)

Best regards,
Jozsef

The following changes since commit 6fcc02e3c2bddeaf628fde3c6a5ab3216d45691a:

  ipvs: fix check on xmit to non-local addresses (2018-06-04 18:28:47 +0200)

are available in the git repository at:

  git://blackhole.kfki.hu/nf cbdebe481a14b

for you to fetch changes up to cbdebe481a14b42c45aa9f4ceb5ff19b55de2c57:

  netfilter: ipset: forbid family for hash:mac sets (2018-06-06 14:01:00 +0200)


Florent Fourcot (1):
  netfilter: ipset: forbid family for hash:mac sets

Jozsef Kadlecsik (2):
  netfilter: ipset: List timing out entries with "timeout 1" instead of zero
  netfilter: ipset: Limit max timeout value

Serhey Popovych (1):
  netfilter: xt_set: Check hook mask correctly

 include/linux/netfilter/ipset/ip_set_timeout.h | 20 ++--
 net/netfilter/ipset/ip_set_hash_gen.h  |  5 -
 net/netfilter/xt_set.c | 10 +-
 3 files changed, 23 insertions(+), 12 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4] ipset patches for nf-next

[Jozsef Kadlecsik]

Hi Pablo,

On Tue, 5 Jun 2018, Pablo Neira Ayuso wrote:

> These are fixes and net-next is closed, please route them through 
> nf.git.

No problem, I'll resubmit the patches when net-next opens up again, for 
nf.git.

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/4] ipset patches for nf-next

[Jozsef Kadlecsik]

Hi Pablo,

Please pull the next patches for nf-next:

- Check hook mask for unsupported hooks instead of supported ones in xt_set.
  (Serhey Popovych).
- List/save just timing out entries with "timeout 1" instead of "timeout 0":
  zero timeout value means permanent entries. When restoring the elements,
  we'd add non-timing out entries. Fixes netfilter bugzilla id #1258.
- Limit max timeout value to (UINT_MAX >> 1)/MSEC_PER_SEC due to the negative
  value condition in msecs_to_jiffies(). msecs_to_jiffies() should be revised:
  if one wants to set the timeout above 2147483, msecs_to_jiffies() sets
  the value to 4294967. (Reported by Maxim Masiutin).
- Forbid family for hash:mac sets in the kernel module: ipset userspace tool
  enforces it but third party tools could create sets with this parameter. Such
  sets then cannot be listed/saved with ipset itself. (Florent Fourcot)

Best regards,
Jozsef

The following changes since commit f624434a0ec96ac338f10f3f7f5a2ef287dd597e:

  Merge tag 'wireless-drivers-next-for-davem-2018-05-31' of 
git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next 
(2018-06-03 11:03:10 -0400)

are available in the git repository at:

  git://blackhole.kfki.hu/nf-next 96569e20b

for you to fetch changes up to 96569e20b472394072c40c41548a37f14bc10882:

  netfilter: ipset: forbid family for hash:mac sets (2018-06-05 12:41:29 +0200)


Florent Fourcot (1):
  netfilter: ipset: forbid family for hash:mac sets

Jozsef Kadlecsik (2):
  netfilter: ipset: List timing out entries with "timeout 1" instead of zero
  netfilter: ipset: Limit max timeout value

Serhey Popovych (1):
  netfilter: xt_set: Check hook mask correctly

 include/linux/netfilter/ipset/ip_set_timeout.h | 20 ++--
 net/netfilter/ipset/ip_set_hash_gen.h  |  5 -
 net/netfilter/xt_set.c | 10 +-
 3 files changed, 23 insertions(+), 12 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] netfilter: xt_set: Check hook mask correctly

[Jozsef Kadlecsik]

From: Serhey Popovych 

Inserting rule before one with SET target we get error with warning in
dmesg(1) output:

  # iptables -A FORWARD -t mangle -j SET --map-set test src --map-prio
  # iptables -I FORWARD 1 -t mangle -j ACCEPT
  iptables: Invalid argument. Run `dmesg' for more information.
  # dmesg |tail -n1
  [268578.026643] mapping of prio or/and queue is allowed only from \
  OUTPUT/FORWARD/POSTROUTING chains

Rather than checking for supported hook bits for SET target check for
unsupported one as done in all rest of matches and targets.

Signed-off-by: Serhey Popovych 
Signed-off-by: Jozsef Kadlecsik 
---
 net/netfilter/xt_set.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/xt_set.c b/net/netfilter/xt_set.c
index 6f4c521..07af7db 100644
--- a/net/netfilter/xt_set.c
+++ b/net/netfilter/xt_set.c
@@ -470,7 +470,7 @@ set_target_v3_checkentry(const struct xt_tgchk_param *par)
}
if (((info->flags & IPSET_FLAG_MAP_SKBPRIO) |
 (info->flags & IPSET_FLAG_MAP_SKBQUEUE)) &&
-!(par->hook_mask & (1 << NF_INET_FORWARD |
+(par->hook_mask & ~(1 << NF_INET_FORWARD |
 1 << NF_INET_LOCAL_OUT |
 1 << NF_INET_POST_ROUTING))) {
pr_info_ratelimited("mapping of prio or/and queue is 
allowed only from OUTPUT/FORWARD/POSTROUTING chains\n");
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] netfilter: ipset: List timing out entries with "timeout 1" instead of zero

[Jozsef Kadlecsik]

When listing sets with timeout support, there's a probability that
just timing out entries with "0" timeout value is listed/saved.
However when restoring the saved list, the zero timeout value means
permanent elelements.

The new behaviour is that timing out entries are listed with "timeout 1"
instead of zero.

Fixes netfilter bugzilla #1258.

Signed-off-by: Jozsef Kadlecsik 
---
 include/linux/netfilter/ipset/ip_set_timeout.h | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/linux/netfilter/ipset/ip_set_timeout.h 
b/include/linux/netfilter/ipset/ip_set_timeout.h
index bfb3531..7ad8ddf 100644
--- a/include/linux/netfilter/ipset/ip_set_timeout.h
+++ b/include/linux/netfilter/ipset/ip_set_timeout.h
@@ -65,8 +65,14 @@ ip_set_timeout_set(unsigned long *timeout, u32 value)
 static inline u32
 ip_set_timeout_get(const unsigned long *timeout)
 {
-   return *timeout == IPSET_ELEM_PERMANENT ? 0 :
-   jiffies_to_msecs(*timeout - jiffies)/MSEC_PER_SEC;
+   u32 t;
+
+   if (*timeout == IPSET_ELEM_PERMANENT)
+   return 0;
+
+   t = jiffies_to_msecs(*timeout - jiffies)/MSEC_PER_SEC;
+   /* Zero value in userspace means no timeout */
+   return t == 0 ? 1 : t;
 }
 
 #endif /* __KERNEL__ */
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/4] netfilter: ipset: Limit max timeout value

[Jozsef Kadlecsik]

Due to the negative value condition in msecs_to_jiffies(), the real
max possible timeout value must be set to (UINT_MAX >> 1)/MSEC_PER_SEC.

Neutron Soutmun proposed the proper fix, but an insufficient one was
applied, see https://patchwork.ozlabs.org/patch/400405/.

Signed-off-by: Jozsef Kadlecsik 
---
 include/linux/netfilter/ipset/ip_set_timeout.h | 10 ++
 net/netfilter/xt_set.c |  8 
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/include/linux/netfilter/ipset/ip_set_timeout.h 
b/include/linux/netfilter/ipset/ip_set_timeout.h
index 7ad8ddf..8ce271e 100644
--- a/include/linux/netfilter/ipset/ip_set_timeout.h
+++ b/include/linux/netfilter/ipset/ip_set_timeout.h
@@ -23,6 +23,9 @@
 /* Set is defined with timeout support: timeout value may be 0 */
 #define IPSET_NO_TIMEOUT   UINT_MAX
 
+/* Max timeout value, see msecs_to_jiffies() in jiffies.h */
+#define IPSET_MAX_TIMEOUT  (UINT_MAX >> 1)/MSEC_PER_SEC
+
 #define ip_set_adt_opt_timeout(opt, set)   \
 ((opt)->ext.timeout != IPSET_NO_TIMEOUT ? (opt)->ext.timeout : (set)->timeout)
 
@@ -32,11 +35,10 @@ ip_set_timeout_uget(struct nlattr *tb)
unsigned int timeout = ip_set_get_h32(tb);
 
/* Normalize to fit into jiffies */
-   if (timeout > UINT_MAX/MSEC_PER_SEC)
-   timeout = UINT_MAX/MSEC_PER_SEC;
+   if (timeout > IPSET_MAX_TIMEOUT)
+   timeout = IPSET_MAX_TIMEOUT;
 
-   /* Userspace supplied TIMEOUT parameter: adjust crazy size */
-   return timeout == IPSET_NO_TIMEOUT ? IPSET_NO_TIMEOUT - 1 : timeout;
+   return timeout;
 }
 
 static inline bool
diff --git a/net/netfilter/xt_set.c b/net/netfilter/xt_set.c
index 07af7db..bf2890b 100644
--- a/net/netfilter/xt_set.c
+++ b/net/netfilter/xt_set.c
@@ -372,8 +372,8 @@ set_target_v2(struct sk_buff *skb, const struct 
xt_action_param *par)
 
/* Normalize to fit into jiffies */
if (add_opt.ext.timeout != IPSET_NO_TIMEOUT &&
-   add_opt.ext.timeout > UINT_MAX / MSEC_PER_SEC)
-   add_opt.ext.timeout = UINT_MAX / MSEC_PER_SEC;
+   add_opt.ext.timeout > IPSET_MAX_TIMEOUT)
+   add_opt.ext.timeout = IPSET_MAX_TIMEOUT;
if (info->add_set.index != IPSET_INVALID_ID)
ip_set_add(info->add_set.index, skb, par, _opt);
if (info->del_set.index != IPSET_INVALID_ID)
@@ -407,8 +407,8 @@ set_target_v3(struct sk_buff *skb, const struct 
xt_action_param *par)
 
/* Normalize to fit into jiffies */
if (add_opt.ext.timeout != IPSET_NO_TIMEOUT &&
-   add_opt.ext.timeout > UINT_MAX / MSEC_PER_SEC)
-   add_opt.ext.timeout = UINT_MAX / MSEC_PER_SEC;
+   add_opt.ext.timeout > IPSET_MAX_TIMEOUT)
+   add_opt.ext.timeout = IPSET_MAX_TIMEOUT;
if (info->add_set.index != IPSET_INVALID_ID)
ip_set_add(info->add_set.index, skb, par, _opt);
if (info->del_set.index != IPSET_INVALID_ID)
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/4] netfilter: ipset: forbid family for hash:mac sets

[Jozsef Kadlecsik]

From: Florent Fourcot 

Userspace `ipset` command forbids family option for hash:mac type:

ipset create test hash:mac family inet4
ipset v6.30: Unknown argument: `family'

However, this check is not done in kernel itself. When someone use
external netlink applications (pyroute2 python library for example), one
can create hash:mac with invalid family and inconsistant results from
userspace (`ipset` command cannot read set content anymore).

This patch enforce the logic in kernel, and forbids insertion of
hash:mac with a family set.

Since IP_SET_PROTO_UNDEF is defined only for hash:mac, this patch has no
impact on other hash:* sets

Signed-off-by: Florent Fourcot 
Signed-off-by: Victorien Molle 
Signed-off-by: Jozsef Kadlecsik 
---
 net/netfilter/ipset/ip_set_hash_gen.h | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/ipset/ip_set_hash_gen.h 
b/net/netfilter/ipset/ip_set_hash_gen.h
index bbad940..8a33dac 100644
--- a/net/netfilter/ipset/ip_set_hash_gen.h
+++ b/net/netfilter/ipset/ip_set_hash_gen.h
@@ -1234,7 +1234,10 @@ IPSET_TOKEN(HTYPE, _create)(struct net *net, struct 
ip_set *set,
pr_debug("Create set %s with family %s\n",
 set->name, set->family == NFPROTO_IPV4 ? "inet" : "inet6");
 
-#ifndef IP_SET_PROTO_UNDEF
+#ifdef IP_SET_PROTO_UNDEF
+   if (set->family != NFPROTO_UNSPEC)
+   return -IPSET_ERR_INVALID_FAMILY;
+#else
if (!(set->family == NFPROTO_IPV4 || set->family == NFPROTO_IPV6))
return -IPSET_ERR_INVALID_FAMILY;
 #endif
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] netfilter: ipset: forbid family for hash:mac sets

[Jozsef Kadlecsik]

Hi,

On Mon, 4 Jun 2018, Florent Fourcot wrote:

> Userspace `ipset` command forbids family option for hash:mac type:
> 
> ipset create test hash:mac family inet4
> ipset v6.30: Unknown argument: `family'
> 
> However, this check is not done in kernel itself. When someone use
> external netlink applications (pyroute2 python library for example), one
> can create hash:mac with invalid family and inconsistant results from
> userspace (`ipset` command cannot read set content anymore).
> 
> This patch enforce the logic in kernel, and forbids insertion of
> hash:mac with a family set.
> 
> Since IP_SET_PROTO_UNDEF is defined only for hash:mac, this patch has no
> impact on other hash:* sets
> 
> Signed-off-by: Florent Fourcot 
> Signed-off-by: Victorien Molle 
> ---
>  net/netfilter/ipset/ip_set_hash_gen.h | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)

Patch is applied in the ipset git tree, thanks.

Best regards,
Jozsef


-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH ipset] tests/check_klog.sh: Try dmesg too, don't let shell terminate script

[Jozsef Kadlecsik]

On Tue, 8 May 2018, Stefano Brivio wrote:

> Some hosts might not use /var/log/kern.log for kernel messages,
> so if we can't find a match there, try dmesg next.
> 
> If no matches are found, don't let the shell terminate the
> script, so that we have a chance to try dmesg and actually echo
> "no match!" if no matches are found: set +e before the setname
> loop.

Patch is applied in the ipset git tree, thanks!

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH nf] netfilter: xt_CT: Force user-space strings as null terminated

[Jozsef Kadlecsik]

Hi,

On Tue, 29 May 2018, Pablo Neira Ayuso wrote:

> On Tue, May 29, 2018 at 11:58:29AM +0800, gfree.w...@vip.163.com wrote:
> > From: Gao Feng 
> > 
> > The helper and timeout strings are from user-space, we need to make
> > sure they are null terminated. If not, evil user could make kernel
> > read the unexpected memory, even print it when fail to find by the
> > following codes.
> > 
> > pr_info_ratelimited("No such helper \"%s\"\n", helper_name);
> > 
> > Signed-off-by: Gao Feng 
> > ---
> >  net/netfilter/xt_CT.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/net/netfilter/xt_CT.c b/net/netfilter/xt_CT.c
> > index 8790190..f4b7d31 100644
> > --- a/net/netfilter/xt_CT.c
> > +++ b/net/netfilter/xt_CT.c
> > @@ -245,12 +245,14 @@ static int xt_ct_tg_check(const struct xt_tgchk_param 
> > *par,
> > }
> >  
> > if (info->helper[0]) {
> > +   info->helper[sizeof(info->helper) - 1] = '\0';
> 
> Probably better to reject non-nul terminated strings from userspace?

Attackers can easily build custom tools by which they then send arbitrary 
parameters to the kernel. So I think it's better to sanitize the input at 
kernel side - and do the same in our userspace tools as well :-)

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] netfilter: Fix handling simultaneous open in TCP conntrack

[Jozsef Kadlecsik]

Hi Pablo,

[Sorry for the delay.]

On Fri, 27 Apr 2018, Pablo Neira Ayuso wrote:

> On Sat, Apr 21, 2018 at 01:43:48PM +0200, Jozsef Kadlecsik wrote:
> > Dominique Martinet reported a TCP hang problem when simultaneous open 
> > was used. The problem is that the tcp_conntracks state table is not 
> > smart enough to handle the case. The state table could be fixed by 
> > introducing a new state, but that would require more lines of code 
> > compared to this patch, due to the required backward compatibility 
> > with ctnetlink.
> 
> BTW, what is exactly the problem in ctnetlink. I think probably there is 
> a way to do some mapping to avoid this. Thanks!

There's nothing wrong with ctnetlink, I was too terse.

If a new state is introduced, then there'd be a hole in several internal 
tables (tcp_conntrack_names, tcp_timeouts, tcp_conntracks state table) and 
that'd be ugly. However if the states are renumbered in order to get rid 
of the holes, then that'd broke the backward compatibility in ctnetlink - 
and userspace anyway, because the constants are exposed through 
uapi/linux/netfilter/nf_conntrack_tcp.h. Or some mapping could be used as 
you suggest but that seems to be overkill compared to the few lines of 
code in the patch.

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/1] TCP conntrack patch to handle simultaneous open

[Jozsef Kadlecsik]

Hi Pablo,

Please apply the next patch to nf-next: Dominique Martinet reported a
tcp hang issue at netdev when simultaneous open was used. The conntrack
state table could not handle the case and the patch fixes it by introducing
a new internal flag.

Best regards,
Jozsef

Jozsef Kadlecsik (1):
  netfilter: Fix handling simultaneous open in TCP conntrack

 include/uapi/linux/netfilter/nf_conntrack_tcp.h |  3 +++
 net/netfilter/nf_conntrack_proto_tcp.c  | 11 +++
 2 files changed, 14 insertions(+)

-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/1] netfilter: Fix handling simultaneous open in TCP conntrack

[Jozsef Kadlecsik]

Dominique Martinet reported a TCP hang problem when simultaneous open was used.
The problem is that the tcp_conntracks state table is not smart enough
to handle the case. The state table could be fixed by introducing a new state,
but that would require more lines of code compared to this patch, due to the
required backward compatibility with ctnetlink.

Signed-off-by: Jozsef Kadlecsik <kad...@blackhole.kfki.hu>
Reported-by: Dominique Martinet <asmad...@codewreck.org>
Tested-by: Dominique Martinet <asmad...@codewreck.org>
---
 include/uapi/linux/netfilter/nf_conntrack_tcp.h |  3 +++
 net/netfilter/nf_conntrack_proto_tcp.c  | 11 +++
 2 files changed, 14 insertions(+)

diff --git a/include/uapi/linux/netfilter/nf_conntrack_tcp.h 
b/include/uapi/linux/netfilter/nf_conntrack_tcp.h
index 74b9115..bcba72d 100644
--- a/include/uapi/linux/netfilter/nf_conntrack_tcp.h
+++ b/include/uapi/linux/netfilter/nf_conntrack_tcp.h
@@ -46,6 +46,9 @@ enum tcp_conntrack {
 /* Marks possibility for expected RFC5961 challenge ACK */
 #define IP_CT_EXP_CHALLENGE_ACK0x40
 
+/* Simultaneous open initialized */
+#define IP_CT_TCP_SIMULTANEOUS_OPEN0x80
+
 struct nf_ct_tcp_flags {
__u8 flags;
__u8 mask;
diff --git a/net/netfilter/nf_conntrack_proto_tcp.c 
b/net/netfilter/nf_conntrack_proto_tcp.c
index e97cdc1..8e67910 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -981,6 +981,17 @@ static int tcp_packet(struct nf_conn *ct,
return NF_ACCEPT; /* Don't change state */
}
break;
+   case TCP_CONNTRACK_SYN_SENT2:
+   /* tcp_conntracks table is not smart enough to handle
+* simultaneous open.
+*/
+   ct->proto.tcp.last_flags |= IP_CT_TCP_SIMULTANEOUS_OPEN;
+   break;
+   case TCP_CONNTRACK_SYN_RECV:
+   if (dir == IP_CT_DIR_REPLY && index == TCP_ACK_SET &&
+   ct->proto.tcp.last_flags & IP_CT_TCP_SIMULTANEOUS_OPEN)
+   new_state = TCP_CONNTRACK_ESTABLISHED;
+   break;
case TCP_CONNTRACK_CLOSE:
if (index == TCP_RST_SET
&& (ct->proto.tcp.seen[!dir].flags & 
IP_CT_TCP_FLAG_MAXACK_SET)
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [ANNOUNCE] ipset 6.37 released

[Jozsef Kadlecsik]

Hi,

On Tue, 10 Apr 2018, Jozsef Kadlecsik wrote:

> I'm happy to announce ipset 6.37 which includes two patches over the 
> previous release: a kernel part optimization from Joe Perches and the fix 
> for a parsing bug which prevented to use service names for ports, reported 
> by Yuri D'Elia.

ipset 6.38 is out to fix the messed up API version number in v6.37 :-).

> You can download the source code of ipset from:
> http://ipset.netfilter.org
> ftp://ftp.netfilter.org/pub/ipset/
> git://git.netfilter.org/ipset.git

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ipset 6.37

[Jozsef Kadlecsik]

On Tue, 10 Apr 2018, Jan Engelhardt wrote:

> >commit 516600858cb54906fb728d04e5edf1131ee7b3b2
> >Author: Jozsef Kadlecsik <kad...@blackhole.kfki.hu>
> >Date:   Tue Apr 10 20:48:35 2018 +0200
> >
> >Fix parsing service names for ports
> >
> >Parsing is attempted both for numbers and service names and
> >the temporary stored error message triggered to reset the state
> >parameters about the set. Reported by Yuri D'Elia.
> >
> >diff --git a/Make_global.am b/Make_global.am
> >index 4b0ac11..10334cc 100644
> >--- a/Make_global.am
> >+++ b/Make_global.am
> >@@ -69,7 +69,7 @@
> > # interface. 
> > 
> > #curr:rev:age
> >-LIBVERSION = 11:0:0
> >+LIBVERSION = 11:1:1
> > 
> > AM_CPPFLAGS = $(kinclude_CFLAGS) $(all_includes) -I$(top_srcdir)/include
> 
> This made libipset.so.11 go back to libipset.so.10. That certainly does 
> not look right.

Oh, my! From time to time I mess up the API version... Fix is on the way 
as "12.0.1". Thanks!

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[ANNOUNCE] ipset 6.37 released

[Jozsef Kadlecsik]

Hi,

I'm happy to announce ipset 6.37 which includes two patches over the 
previous release: a kernel part optimization from Joe Perches and the fix 
for a parsing bug which prevented to use service names for ports, reported 
by Yuri D'Elia.

Userspace changes:
  - Fix parsing service names for ports (reported by Yuri D'Elia)
Kernel part changes:
  - netfilter: ipset: Use is_zero_ether_addr instead of static and memcmp
(Joe Perches)

You can download the source code of ipset from:
http://ipset.netfilter.org
ftp://ftp.netfilter.org/pub/ipset/
git://git.netfilter.org/ipset.git

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH nft] payload: don't remove icmp family dependency in special cases

[Jozsef Kadlecsik]

Hi Florian,

On Tue, 27 Mar 2018, Florian Westphal wrote:

> When using nftables to filter icmp-in-ipv6 or icmpv6-in-ipv4 we
> erronously removed the dependency, i.e. "lis ruleset" shows
> 
> table ip6 filter { chain output {
>   type filter hook output priority 0; policy accept;
>   icmp type destination-unreachable
> } }
> 
> but that won't restore because of ip vs ipv6 conflict.
> 
> After this patch, this lists as
> 
>  meta l4proto icmp icmp type destination-unreachable

Just a general comment, independently from the patch: I fully understand 
that technically this is the expression to be used. But it's highly error 
prone and also difficult to read.

The language could be made more readable by allowing (and by default 
printing) a comma between the expression parts, like this:

meta l4proto icmp, icmp type destination-unreachable

Just a suggestion.

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH nf-next,RFC] netfilter: nf_conntrack_tcp: reset entry only from CLOSE and TIME_WAIT states

[Jozsef Kadlecsik]

Hi Pablo,

On Tue, 20 Mar 2018, Pablo Neira Ayuso wrote:

> Before this patch, if TCP state is < TIME_WAIT, we skip this logic. 
> Along time, we got new states over the TIME_WAIT value, such as 
> SYN_SENT2, RETRANS and UNACK, that can trigger this conntrack entry 
> reset, however this check was never updated.
> 
> My understanding is that we should only exercise the conntrack reset 
> codepath if the existing packet is triggering a transition to SYN_SENT 
> state while we were in TIME_WAIT or CLOSE states.

RETRANS and UNACK are not TCP states, just mnemonics to help assign 
special timeout values.

SYN_SENT2 is a TCP state which is the second one in the case of 
simultaneous open but from there there's no way back to reach SYN_SENT 
again.

So I think the original condition is all right. However, if you believe 
the patch makes it more clean and less error-prone, then just apply it.

Best regards,
Jozsef

> Signed-off-by: Pablo Neira Ayuso 
> ---
> While reviewing TCP tracking code, I noticed this. This is just narrowing
> down this codepath so it only works with TIME_WAIT and CLOSE states.
> 
>  net/netfilter/nf_conntrack_proto_tcp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/nf_conntrack_proto_tcp.c 
> b/net/netfilter/nf_conntrack_proto_tcp.c
> index e97cdc1cf98c..c765d83db574 100644
> --- a/net/netfilter/nf_conntrack_proto_tcp.c
> +++ b/net/netfilter/nf_conntrack_proto_tcp.c
> @@ -827,7 +827,8 @@ static int tcp_packet(struct nf_conn *ct,
>  
>   switch (new_state) {
>   case TCP_CONNTRACK_SYN_SENT:
> - if (old_state < TCP_CONNTRACK_TIME_WAIT)
> + if (old_state != TCP_CONNTRACK_TIME_WAIT &&
> + old_state != TCP_CONNTRACK_CLOSE)
>   break;
>   /* RFC 1122: "When a connection is closed actively,
>* it MUST linger in TIME-WAIT state for a time 2xMSL
> -- 
> 2.11.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [ANNOUNCE] ipset 6.28 released

[Jozsef Kadlecsik]

On Mon, 5 Mar 2018, Akshat Kakkar wrote:

> > What is your architecture?
> Its a 4.4.82 64 bit kernel

What I meant is your hardware architecture, i.e. "uname -a". But I suppose 
it's x86_64.

> and my setup has only 2 machines ... one laptop(IP:192.168.100.100)
> and other this linux machine (IP:192.168.100.1).
> 
> I have already taken sufficient time of yours, you just plz reply that 
> are you able to find an iptable rule hit with ipset having entry 
> 0.0.0.0/0,eth0. If you are able to, then I will try doing a fresh 
> installation of everything. However, cause of issue will remain unknown.

Yes, in my test environment the rule is matched. But it'd be really 
strange and definitely not normal that a reinstall is required...

root@a# ipset v
ipset v6.35, protocol version: 6
root@a# ipset l
Name: test
Type: hash:net,iface
Revision: 6
Header: family inet hashsize 1024 maxelem 65536
Size in memory: 536
References: 0
Number of entries: 1
Members:
0.0.0.0/0,eth0
root@a# iptables-save -t filter -c
# Generated by iptables-save v1.4.21 on Mon Mar  5 20:07:28 2018
*filter
:INPUT ACCEPT [0:0]  
:FORWARD ACCEPT [0:0]
:OUTPUT ACCEPT [0:0]
[0:0] -A INPUT -m set --match-set test src,src -j ACCEPT   
COMMIT 
# Completed on Mon Mar  5 20:02:50 2018

"ping -c 1 a"

root@a# iptables-save -t filter -c
# Generated by iptables-save v1.4.21 on Mon Mar  5 20:07:32 2018
*filter
:INPUT ACCEPT [0:0]
:FORWARD ACCEPT [0:0]
:OUTPUT ACCEPT [1:84]
[1:84] -A INPUT -m set --match-set test src,src -j ACCEPT
COMMIT
# Completed on Mon Mar  5 20:07:32 2018

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[ANNOUNCE] ipset 6.36 released

[Jozsef Kadlecsik]

Hi,

I'm happy to announce ipset 6.36. It comes with one important bugfix, 
which fixes a wraparound bug introduced in v6.34: adding an IPv4 range 
x.x.x.x-255.255.255.255 could lead to memory exhaustion.

Userspace changes:
  - Use 'ss' in runtest.sh but fall back to deprecated 'net-tools'
command (bugzilla id #1209)
  - build: do install libipset/args.h (Jan Engelhardt)
  - Add test to verify wraparound fix
Kernel part changes:
  - Remove duplicate module description
  - netfilter: remove messages print and boot/module load time
(Pablo Neira Ayuso)
  - Fix wraparound bug introduced in commit 48596a8ddc46 in v6.34
(reported by Thomas Schwark)

You can download the source code of ipset from:
http://ipset.netfilter.org
ftp://ftp.netfilter.org/pub/ipset/
git://git.netfilter.org/ipset.git

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [ANNOUNCE] ipset 6.28 released

[Jozsef Kadlecsik]

On Thu, 1 Mar 2018, Akshat Kakkar wrote:

> >> There is only one rule in my iptables,
> >>
> >> iptables -A INPUT -m set --match-set foo src,src -j ACCEPT
> >
> > That's the filter table. What about the other tables?
> 
> nothing in any other table.
> raw
> mangle
> nat
> 
> If entry in ipset is 0.0.0.0/0,eth0 then iptable rule is not matched.
> 
> However, if entry in ipset is 0.0.0.0/1,eth0 and 128.0.0.0/1,eth0,
> then iptable rule is matched.

What is your architecture?

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [ANNOUNCE] ipset 6.28 released

[Jozsef Kadlecsik]

On Thu, 1 Mar 2018, Akshat Kakkar wrote:

> There is only one rule in my iptables,
> 
> iptables -A INPUT -m set --match-set foo src,src -j ACCEPT

That's the filter table. What about the other tables?

> Should I reinstall my iptables (v 1.6.1) as I have modified ipset from
> 6.25 to 6.35?

No, it is not necessary, it should work fine.

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [ANNOUNCE] ipset 6.28 released

[Jozsef Kadlecsik]

On Wed, 28 Feb 2018, Akshat Kakkar wrote:

> Just to add,
> 
> with ipset having entry for 0.0.0.0/0,eth0
> 
> if I test
> ipset -T foo 192.168.100.100,eth0
> 
> its returns success.

That is what I wanted to ask to check.

> But in iptables rule it is not matching.

I made a clean reinstall of ipset at my test machine with the package 6.35 
and I'm still unable to reproduce the issue.

Use the TRACE target in the raw table and double check your iptables 
rules.

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [ANNOUNCE] ipset 6.28 released

[Jozsef Kadlecsik]

Hi,

On Thu, 22 Feb 2018, Akshat Kakkar wrote:

> I created an IPSET,
> ipset -N foo hash:net,iface
> 
> Then added member as
> ipset -A foo 0.0.0.0/0,eth0
> 
> However, following iptables rule is not matched when machine is pinged
> on its eth0 interface

What do you mean by "pinged on its eth0 interface"? Do you ping the 
machine from itself?

> iptables -A INPUT -m set --match-set foo src,src -j ACCEPT
> 
> But, if I add entry in ipset as
> ipset -A foo 192.168.100.100,eth0
> 
> And I ping from 192.168.100.100, the rule is hit.
> 
> iptables version 1.6.1, ipset version 6.35, kernel 4.4.82

I can't reproduce it with ipset 6.35.

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 0/4] net: add bpfilter

[Jozsef Kadlecsik]

Hi David,

On Mon, 19 Feb 2018, Florian Westphal wrote:

> David Miller  wrote:
> > 
> > Florian, first of all, the whole "change the iptables binary" idea is
> > a non-starter.  For the many reasons I have described in the various
> > postings I have made today.
> > 
> > It is entirely impractical.

You stressed several times that container images, virtualization 
installations don't change - and that's exaggregation. Those are updated 
as well, and not only because security updates must be rolled out, but 
because new versions of softwares are requested.

You mentioned that the hosting part can upgrade the kernel - it means that 
enabling NFTABLES is also a non-issue when the new eBPF functionality is 
switched on, if that was missing.

> You suggest:
> 
> iptables -> setsockopt -> umh (xtables -> ebpf) -> kernel
> 
> How is this different from
> 
> iptables -> setsockopt -> umh (Xtables -> nftables -> kernel
> 
> ?
> EBPF can be placed within nftables either userspace or kernel,
> there is nothing that prevents this.

So why the second scenario suggested by Florian is not possible or must be 
avoided? It not only could keep the unmodified iptables in the container 
(if that's a must from some reason), but it would make possible to replace 
it later anytime with iptables-compat/nftables.

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [nft PATCH] Enable automerge feature for anonymous sets

[Jozsef Kadlecsik]

On Wed, 7 Feb 2018, Pablo Neira Ayuso wrote:

> On Tue, Feb 06, 2018 at 07:18:47PM +0100, Phil Sutter wrote:
> > Automatic merging of adjacent/overlapping ranges upon insertion has
> > clear benefits performance- and readability-wise. The drawbacks which
> > led to disabling it by default don't apply to anonymous sets since they
> > are read-only anyway, so enable this feature for them again.
> 
> Question is, why someone would be adding elements with overlapping 
> ranges in an anonymous set? Then, when listing the ruleset you will get 
> something different to what you've added. This would also be 
> inconsistent with regards to the existing behaviour in named sets, where 
> this is turned off by default.
> 
> For named sets, that are useful to maintain white/blacklists, I 
> understand this simplifies complexity for people dealing with them. But 
> not sure for anonymous sets.
> 
> @Jeff: Is this also useful to you in the anonymous set use-case? IIRC we 
> agreed that this was good for named sets, but not for anonymous sets.

In my opinion the consistent behaviour is the most desired one. Such 
subleties that by default there's no automerge in named sets but it's on 
for anonymous sets are easily overlooked by users. Better have a flag, 
option to turn it on explicitly for a given anonymous set.

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] build: do install libipset/args.h

[Jozsef Kadlecsik]

Hi Jan,

On Mon, 22 Jan 2018, Jan Engelhardt wrote:

> libipset/types.h includes args.h, therefore args.h must be installed
> too.
> 
> Signed-off-by: Jan Engelhardt 
> ---
>  include/libipset/Makefile.am | 1 +
>  1 file changed, 1 insertion(+)

Yes, it's required. Thanks, patch is applied.

Best regards,
Jozsef 
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH nf-next] netfilter: remove messages print and boot/module load time

[Jozsef Kadlecsik]

On Fri, 19 Jan 2018, Pablo Neira Ayuso wrote:

> Several reasons for this:
> 
> * Several modules maintain internal version numbers, that they print at
>   boot/module load time, that are not exposed to userspace, as a
>   primitive mechanism to make revision number control from the earlier
>   days of Netfilter.
> 
> * IPset shows the protocol version at boot/module load time, instead
>   display this via module description, as Jozsef suggested.
> 
> * Remove copyright notice at boot/module load time in two spots, the
>   Netfilter codebase is a collective development effort, if we would
>   have to display copyrights for each contributor at boot/module load
>   time for each extensions we have, we would probably fill up logs with
>   lots of useless information - from a technical standpoint.
> 
> So let's be consistent and remove them all.
> 
> Signed-off-by: Pablo Neira Ayuso <pa...@netfilter.org>
> ---
>  net/netfilter/ipset/ip_set_core.c| 3 ++-
>  net/netfilter/nf_conntrack_netlink.c | 5 -
>  net/netfilter/nf_tables_api.c| 1 -
>  net/netfilter/nfnetlink.c| 4 
>  net/netfilter/nfnetlink_acct.c   | 2 --
>  net/netfilter/nft_compat.c   | 2 --
>  6 files changed, 2 insertions(+), 15 deletions(-)

Acked-by: Jozsef Kadlecsik <kad...@blackhole.kfki.hu>

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/1] ipset patch for the nf tree

[Jozsef Kadlecsik]

Hi Pablo,

Here follows a patch for the nf tree, please apply it:

- The patch "Fix adding an IPv4 range containing more than 2^31
  addresses" introduced a wraparound bug, which could lead to
  memory exhaustion, which is fixed here (netfilter bugzilla
  id #1212, reported by Thomas Schwark)

Best regards,
Jozsef

The following changes since commit 889c604fd0b5f6d3b8694ade229ee44124de1127:

  netfilter: x_tables: fix int overflow in xt_alloc_table_info() (2018-01-07 
00:17:23 +0100)

are available in the git repository at:

  git://blackhole.kfki.hu/nf ba31d2d88b9

for you to fetch changes up to ba31d2d88b95ce1872fc17ffd0da70b68be0a07f:

  Fix wraparound bug introduced in commit 48596a8ddc46 (2018-01-12 11:07:35 
+0100)

--------
Jozsef Kadlecsik (1):
  Fix wraparound bug introduced in commit 48596a8ddc46

 net/netfilter/ipset/ip_set_hash_ipportnet.c  | 26 ++---
 net/netfilter/ipset/ip_set_hash_net.c|  9 ---
 net/netfilter/ipset/ip_set_hash_netiface.c   |  9 ---
 net/netfilter/ipset/ip_set_hash_netnet.c | 28 +++---
 net/netfilter/ipset/ip_set_hash_netport.c| 19 ---
 net/netfilter/ipset/ip_set_hash_netportnet.c | 35 ++--
 6 files changed, 63 insertions(+), 63 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/1] Fix wraparound bug introduced in commit 48596a8ddc46

[Jozsef Kadlecsik]

The patch "netfilter: ipset: Fix adding an IPv4 range containing
more than 2^31 addresses" introduced a wraparound bug, which could
lead to memory exhaustion when adding an x.x.x.x-255.255.255.255
range to any hash:*net* types.

Fixes Netfilter's bugzilla id #1212, reported by Thomas Schwark.

Signed-off-by: Jozsef Kadlecsik <kad...@blackhole.kfki.hu>
---
 net/netfilter/ipset/ip_set_hash_ipportnet.c  | 26 ++---
 net/netfilter/ipset/ip_set_hash_net.c|  9 ---
 net/netfilter/ipset/ip_set_hash_netiface.c   |  9 ---
 net/netfilter/ipset/ip_set_hash_netnet.c | 28 +++---
 net/netfilter/ipset/ip_set_hash_netport.c| 19 ---
 net/netfilter/ipset/ip_set_hash_netportnet.c | 35 ++--
 6 files changed, 63 insertions(+), 63 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_hash_ipportnet.c 
b/net/netfilter/ipset/ip_set_hash_ipportnet.c
index 0f164e9..88b83d6 100644
--- a/net/netfilter/ipset/ip_set_hash_ipportnet.c
+++ b/net/netfilter/ipset/ip_set_hash_ipportnet.c
@@ -168,7 +168,7 @@ hash_ipportnet4_uadt(struct ip_set *set, struct nlattr 
*tb[],
struct hash_ipportnet4_elem e = { .cidr = HOST_MASK - 1 };
struct ip_set_ext ext = IP_SET_INIT_UEXT(set);
u32 ip = 0, ip_to = 0, p = 0, port, port_to;
-   u32 ip2_from = 0, ip2_to = 0, ip2_last, ip2;
+   u32 ip2_from = 0, ip2_to = 0, ip2;
bool with_ports = false;
u8 cidr;
int ret;
@@ -269,22 +269,21 @@ hash_ipportnet4_uadt(struct ip_set *set, struct nlattr 
*tb[],
ip_set_mask_from_to(ip2_from, ip2_to, e.cidr + 1);
}
 
-   if (retried)
+   if (retried) {
ip = ntohl(h->next.ip);
+   p = ntohs(h->next.port);
+   ip2 = ntohl(h->next.ip2);
+   } else {
+   p = port;
+   ip2 = ip2_from;
+   }
for (; ip <= ip_to; ip++) {
e.ip = htonl(ip);
-   p = retried && ip == ntohl(h->next.ip) ? ntohs(h->next.port)
-  : port;
for (; p <= port_to; p++) {
e.port = htons(p);
-   ip2 = retried &&
- ip == ntohl(h->next.ip) &&
- p == ntohs(h->next.port)
-   ? ntohl(h->next.ip2) : ip2_from;
-   while (ip2 <= ip2_to) {
+   do {
e.ip2 = htonl(ip2);
-   ip2_last = ip_set_range_to_cidr(ip2, ip2_to,
-   );
+   ip2 = ip_set_range_to_cidr(ip2, ip2_to, );
e.cidr = cidr - 1;
ret = adtfn(set, , , , flags);
 
@@ -292,9 +291,10 @@ hash_ipportnet4_uadt(struct ip_set *set, struct nlattr 
*tb[],
return ret;
 
ret = 0;
-   ip2 = ip2_last + 1;
-   }
+   } while (ip2++ < ip2_to);
+   ip2 = ip2_from;
}
+   p = port;
}
return ret;
 }
diff --git a/net/netfilter/ipset/ip_set_hash_net.c 
b/net/netfilter/ipset/ip_set_hash_net.c
index 1c67a17..5449e23 100644
--- a/net/netfilter/ipset/ip_set_hash_net.c
+++ b/net/netfilter/ipset/ip_set_hash_net.c
@@ -143,7 +143,7 @@ hash_net4_uadt(struct ip_set *set, struct nlattr *tb[],
ipset_adtfn adtfn = set->variant->adt[adt];
struct hash_net4_elem e = { .cidr = HOST_MASK };
struct ip_set_ext ext = IP_SET_INIT_UEXT(set);
-   u32 ip = 0, ip_to = 0, last;
+   u32 ip = 0, ip_to = 0;
int ret;
 
if (tb[IPSET_ATTR_LINENO])
@@ -193,16 +193,15 @@ hash_net4_uadt(struct ip_set *set, struct nlattr *tb[],
}
if (retried)
ip = ntohl(h->next.ip);
-   while (ip <= ip_to) {
+   do {
e.ip = htonl(ip);
-   last = ip_set_range_to_cidr(ip, ip_to, );
+   ip = ip_set_range_to_cidr(ip, ip_to, );
ret = adtfn(set, , , , flags);
if (ret && !ip_set_eexist(ret, flags))
return ret;
 
ret = 0;
-   ip = last + 1;
-   }
+   } while (ip++ < ip_to);
return ret;
 }
 
diff --git a/net/netfilter/ipset/ip_set_hash_netiface.c 
b/net/netfilter/ipset/ip_set_hash_netiface.c
index d417074..f5164c1 100644
--- a/net/netfilter/ipset/ip_set_hash_netiface.c
+++ b/net/netfilter/ipset/ip_set_hash_netiface.c
@@ -200,7 +200,7 @@ hash_netiface4_uadt(struct ip_set *set, struct nlattr *tb[],
ipset_adtfn adtfn = set->variant->adt[adt];
struct hash_netiface4_elem e = { .cidr = HOST_MASK

[ANNOUNCE] ipset 6.35 released

[Jozsef Kadlecsik]

Hi,

I'm happy to announce ipset 6.35. The internal revision handling in 
userspace has been reworked to make easier and simpler to add new
revisions/features. The kernel part changes bring a few fixes, backports
and small corrections:

Userspace changes:
  - Userspace revision handling is reworked
  - Replace the last reference to u_int8_t with uint8_t.
Kernel part changes:
  - netfilter: mark expected switch fall-throughs (Gustavo A. R. Silva)
  - License cleanup: add SPDX GPL-2.0 license identifier to files with no
license (Greg Kroah-Hartman)
  - Backport patch: netfilter: ipset: use nfnl_mutex_is_locked
  - Missing nfnl_lock()/nfnl_unlock() is added to ip_set_net_exit()
  - netfilter: ipset: use nfnl_mutex_is_locked (Florian Westphal)
  - netfilter: ipset: add resched points during set listing
(Florian Westphal)
  - Fix "don't update counters" mode when counters used at the matching
  - Backport patch: netfilter: ipset: Convert timers to use timer_setup()
  - netfilter: ipset: use swap macro instead of _manually_ swapping values
(Gustavo A. R. Silva)
  - netfilter: ipset: Fix race between dump and swap (Ross Lagerwall)
  - netfilter: ipset: pernet ops must be unregistered last (Florian
Westphal)

You can download the source code of ipset from:
http://ipset.netfilter.org
ftp://ftp.netfilter.org/pub/ipset/
git://git.netfilter.org/ipset.git

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] netfilter: ipset: Fix "don't update counters" mode when counters used at the matching

[Jozsef Kadlecsik]

The matching of the counters was not taken into account, fixed.

Signed-off-by: Jozsef Kadlecsik <kad...@blackhole.kfki.hu>
---
 include/linux/netfilter/ipset/ip_set.h |   6 ++
 include/linux/netfilter/ipset/ip_set_counter.h |  25 --
 net/netfilter/ipset/ip_set_bitmap_gen.h|   9 +-
 net/netfilter/ipset/ip_set_core.c  |  25 ++
 net/netfilter/ipset/ip_set_hash_gen.h  |  37 +++-
 net/netfilter/ipset/ip_set_list_set.c  |  21 ++---
 net/netfilter/xt_set.c | 119 +
 7 files changed, 114 insertions(+), 128 deletions(-)

diff --git a/include/linux/netfilter/ipset/ip_set.h 
b/include/linux/netfilter/ipset/ip_set.h
index 8e42253..34fc80f 100644
--- a/include/linux/netfilter/ipset/ip_set.h
+++ b/include/linux/netfilter/ipset/ip_set.h
@@ -122,6 +122,8 @@ struct ip_set_ext {
u64 bytes;
char *comment;
u32 timeout;
+   u8 packets_op;
+   u8 bytes_op;
 };
 
 struct ip_set;
@@ -339,6 +341,10 @@ extern int ip_set_get_extensions(struct ip_set *set, 
struct nlattr *tb[],
 struct ip_set_ext *ext);
 extern int ip_set_put_extensions(struct sk_buff *skb, const struct ip_set *set,
 const void *e, bool active);
+extern bool ip_set_match_extensions(struct ip_set *set,
+   const struct ip_set_ext *ext,
+   struct ip_set_ext *mext,
+   u32 flags, void *data);
 
 static inline int
 ip_set_get_hostipaddr4(struct nlattr *nla, u32 *ipaddr)
diff --git a/include/linux/netfilter/ipset/ip_set_counter.h 
b/include/linux/netfilter/ipset/ip_set_counter.h
index bb6fba4..3d33a2c 100644
--- a/include/linux/netfilter/ipset/ip_set_counter.h
+++ b/include/linux/netfilter/ipset/ip_set_counter.h
@@ -34,20 +34,33 @@ ip_set_get_packets(const struct ip_set_counter *counter)
return (u64)atomic64_read(&(counter)->packets);
 }
 
+static inline bool
+ip_set_match_counter(u64 counter, u64 match, u8 op)
+{
+   switch (op) {
+   case IPSET_COUNTER_NONE:
+   return true;
+   case IPSET_COUNTER_EQ:
+   return counter == match;
+   case IPSET_COUNTER_NE:
+   return counter != match;
+   case IPSET_COUNTER_LT:
+   return counter < match;
+   case IPSET_COUNTER_GT:
+   return counter > match;
+   }
+   return false;
+}
+
 static inline void
 ip_set_update_counter(struct ip_set_counter *counter,
- const struct ip_set_ext *ext,
- struct ip_set_ext *mext, u32 flags)
+ const struct ip_set_ext *ext, u32 flags)
 {
if (ext->packets != ULLONG_MAX &&
!(flags & IPSET_FLAG_SKIP_COUNTER_UPDATE)) {
ip_set_add_bytes(ext->bytes, counter);
ip_set_add_packets(ext->packets, counter);
}
-   if (flags & IPSET_FLAG_MATCH_COUNTERS) {
-   mext->packets = ip_set_get_packets(counter);
-   mext->bytes = ip_set_get_bytes(counter);
-   }
 }
 
 static inline bool
diff --git a/net/netfilter/ipset/ip_set_bitmap_gen.h 
b/net/netfilter/ipset/ip_set_bitmap_gen.h
index 8afe882..257ca39 100644
--- a/net/netfilter/ipset/ip_set_bitmap_gen.h
+++ b/net/netfilter/ipset/ip_set_bitmap_gen.h
@@ -127,14 +127,7 @@ mtype_test(struct ip_set *set, void *value, const struct 
ip_set_ext *ext,
 
if (ret <= 0)
return ret;
-   if (SET_WITH_TIMEOUT(set) &&
-   ip_set_timeout_expired(ext_timeout(x, set)))
-   return 0;
-   if (SET_WITH_COUNTER(set))
-   ip_set_update_counter(ext_counter(x, set), ext, mext, flags);
-   if (SET_WITH_SKBINFO(set))
-   ip_set_get_skbinfo(ext_skbinfo(x, set), ext, mext, flags);
-   return 1;
+   return ip_set_match_extensions(set, ext, mext, flags, x);
 }
 
 static int
diff --git a/net/netfilter/ipset/ip_set_core.c 
b/net/netfilter/ipset/ip_set_core.c
index 89b4445..e002990 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -472,6 +472,31 @@ ip_set_put_extensions(struct sk_buff *skb, const struct 
ip_set *set,
 }
 EXPORT_SYMBOL_GPL(ip_set_put_extensions);
 
+bool
+ip_set_match_extensions(struct ip_set *set, const struct ip_set_ext *ext,
+   struct ip_set_ext *mext, u32 flags, void *data)
+{
+   if (SET_WITH_TIMEOUT(set) &&
+   ip_set_timeout_expired(ext_timeout(data, set)))
+   return false;
+   if (SET_WITH_COUNTER(set)) {
+   struct ip_set_counter *counter = ext_counter(data, set);
+
+   if (flags & IPSET_FLAG_MATCH_COUNTERS &&
+   !(ip_set_match_counter(ip_set_get_packets(counter),
+   mext->packets, mext

[PATCH 1/3] netfilter: ipset: use swap macro instead of _manually_ swapping values

[Jozsef Kadlecsik]

From: "Gustavo A. R. Silva" <garsi...@embeddedor.com>

Make use of the swap macro and remove unnecessary variables tmp.
This makes the code easier to read and maintain.

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
Signed-off-by: Jozsef Kadlecsik <kad...@blackhole.kfki.hu>
---
 net/netfilter/ipset/ip_set_bitmap_ip.c| 8 ++--
 net/netfilter/ipset/ip_set_bitmap_ipmac.c | 8 ++--
 net/netfilter/ipset/ip_set_bitmap_port.c  | 8 ++--
 3 files changed, 6 insertions(+), 18 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_bitmap_ip.c 
b/net/netfilter/ipset/ip_set_bitmap_ip.c
index d8975a0..488d6d0 100644
--- a/net/netfilter/ipset/ip_set_bitmap_ip.c
+++ b/net/netfilter/ipset/ip_set_bitmap_ip.c
@@ -263,12 +263,8 @@ bitmap_ip_create(struct net *net, struct ip_set *set, 
struct nlattr *tb[],
ret = ip_set_get_hostipaddr4(tb[IPSET_ATTR_IP_TO], _ip);
if (ret)
return ret;
-   if (first_ip > last_ip) {
-   u32 tmp = first_ip;
-
-   first_ip = last_ip;
-   last_ip = tmp;
-   }
+   if (first_ip > last_ip)
+   swap(first_ip, last_ip);
} else if (tb[IPSET_ATTR_CIDR]) {
u8 cidr = nla_get_u8(tb[IPSET_ATTR_CIDR]);
 
diff --git a/net/netfilter/ipset/ip_set_bitmap_ipmac.c 
b/net/netfilter/ipset/ip_set_bitmap_ipmac.c
index 4c279fb..c00b6a2 100644
--- a/net/netfilter/ipset/ip_set_bitmap_ipmac.c
+++ b/net/netfilter/ipset/ip_set_bitmap_ipmac.c
@@ -337,12 +337,8 @@ bitmap_ipmac_create(struct net *net, struct ip_set *set, 
struct nlattr *tb[],
ret = ip_set_get_hostipaddr4(tb[IPSET_ATTR_IP_TO], _ip);
if (ret)
return ret;
-   if (first_ip > last_ip) {
-   u32 tmp = first_ip;
-
-   first_ip = last_ip;
-   last_ip = tmp;
-   }
+   if (first_ip > last_ip)
+   swap(first_ip, last_ip);
} else if (tb[IPSET_ATTR_CIDR]) {
u8 cidr = nla_get_u8(tb[IPSET_ATTR_CIDR]);
 
diff --git a/net/netfilter/ipset/ip_set_bitmap_port.c 
b/net/netfilter/ipset/ip_set_bitmap_port.c
index 7f9bbd7..b561ca8 100644
--- a/net/netfilter/ipset/ip_set_bitmap_port.c
+++ b/net/netfilter/ipset/ip_set_bitmap_port.c
@@ -238,12 +238,8 @@ bitmap_port_create(struct net *net, struct ip_set *set, 
struct nlattr *tb[],
 
first_port = ip_set_get_h16(tb[IPSET_ATTR_PORT]);
last_port = ip_set_get_h16(tb[IPSET_ATTR_PORT_TO]);
-   if (first_port > last_port) {
-   u16 tmp = first_port;
-
-   first_port = last_port;
-   last_port = tmp;
-   }
+   if (first_port > last_port)
+   swap(first_port, last_port);
 
elements = last_port - first_port + 1;
set->dsize = ip_set_elem_len(set, tb, 0, 0);
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] netfilter: ipset: Missing nfnl_lock()/nfnl_unlock() is added to ip_set_net_exit()

[Jozsef Kadlecsik]

Patch "netfilter: ipset: use nfnl_mutex_is_locked" is added the real
mutex locking check, which revealed the missing locking in ip_set_net_exit().

Signed-off-by: Jozsef Kadlecsik <kad...@blackhole.kfki.hu>
Reported-by: syzbot+36b06f219f2439fe6...@syzkaller.appspotmail.com
---
 net/netfilter/ipset/ip_set_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/netfilter/ipset/ip_set_core.c 
b/net/netfilter/ipset/ip_set_core.c
index e002990..728bf31 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -2078,6 +2078,7 @@ ip_set_net_exit(struct net *net)
 
inst->is_deleted = true; /* flag for ip_set_nfnl_put */
 
+   nfnl_lock(NFNL_SUBSYS_IPSET);
for (i = 0; i < inst->ip_set_max; i++) {
set = ip_set(inst, i);
if (set) {
@@ -2085,6 +2086,7 @@ ip_set_net_exit(struct net *net)
ip_set_destroy_set(set);
}
}
+   nfnl_unlock(NFNL_SUBSYS_IPSET);
kfree(rcu_dereference_protected(inst->ip_set_list, 1));
 }
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/3] ipset patches for nf-next

[Jozsef Kadlecsik]

Hi Pablo,

Please consider to apply the next patches:

- A patch to use the swap() macro instead of the manual coding
  from Gustavo A. R. Silva
- A fix to take into account the possible counter value matching
  for the "don't update counters" mode. It required a reorganizing
  in handling the extensions which resulted a simpler and cleaner code.
- The patch "netfilter: ipset: use nfnl_mutex_is_locked" revealed
  a missing mutex locking in ip_set_net_exit(), fixed.

Best regards,
Jozsef

The following changes since commit e0ca2b8c1e3c299da6baca30b528a37c485e3d69:

  netfilter: add nf_queue_entry forward declaration (2018-01-02 12:23:32 +0100)

are available in the git repository at:

  git://blackhole.kfki.hu/nf-next 83d7fe936e

for you to fetch changes up to 83d7fe936e79136275fa2ed3feea95267e0a9fe3:

  netfilter: ipset: Missing nfnl_lock()/nfnl_unlock() is added to 
ip_set_net_exit() (2018-01-06 15:24:18 +0100)


Gustavo A. R. Silva (1):
  netfilter: ipset: use swap macro instead of _manually_ swapping values

Jozsef Kadlecsik (2):
  netfilter: ipset: Fix "don't update counters" mode when counters used at 
the matching
  netfilter: ipset: Missing nfnl_lock()/nfnl_unlock() is added to 
ip_set_net_exit()

 include/linux/netfilter/ipset/ip_set.h |   6 ++
 include/linux/netfilter/ipset/ip_set_counter.h |  25 --
 net/netfilter/ipset/ip_set_bitmap_gen.h|   9 +-
 net/netfilter/ipset/ip_set_bitmap_ip.c |   8 +-
 net/netfilter/ipset/ip_set_bitmap_ipmac.c  |   8 +-
 net/netfilter/ipset/ip_set_bitmap_port.c   |   8 +-
 net/netfilter/ipset/ip_set_core.c  |  27 ++
 net/netfilter/ipset/ip_set_hash_gen.h  |  37 +++-
 net/netfilter/ipset/ip_set_list_set.c  |  21 ++---
 net/netfilter/xt_set.c | 119 +
 10 files changed, 122 insertions(+), 146 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH ipset nf-next] netfilter: ipset: add resched points during set listing

[Jozsef Kadlecsik]

Hi Florian,

On Thu, 30 Nov 2017, Florian Westphal wrote:

> When sets are extremely large we can get softlockup during ipset -L. We 
> could fix this by adding cond_resched_rcu() at the right location during 
> iteration, but this only works if RCU nesting depth is 1.
> 
> At this time entire variant->list() is called under under 
> rcu_read_lock_bh. This used to be a read_lock_bh() but as rcu doesn't 
> really lock anything, it does not appear to be needed, so remove it 
> (ipset increments set reference count before this, so a set deletion 
> should not be possible).

Yes, the call of rcu_read_lock_bh() seems to be unnecessary, the
set->variant->list() functions protect the sensitive parts with 
rcu_read_lock() anyway. Thanks!

Acked-by: Jozsef Kadlecsik <kad...@blackhole.kfki.hu>

Best regards,
Jozsef
 
> Reported-by: Li Shuang <shu...@redhat.com>
> Signed-off-by: Florian Westphal <f...@strlen.de>
> ---
>  net/netfilter/ipset/ip_set_bitmap_gen.h | 1 +
>  net/netfilter/ipset/ip_set_core.c   | 2 --
>  net/netfilter/ipset/ip_set_hash_gen.h   | 1 +
>  3 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/netfilter/ipset/ip_set_bitmap_gen.h 
> b/net/netfilter/ipset/ip_set_bitmap_gen.h
> index 5ca18f07683b5..8afe882f846d5 100644
> --- a/net/netfilter/ipset/ip_set_bitmap_gen.h
> +++ b/net/netfilter/ipset/ip_set_bitmap_gen.h
> @@ -227,6 +227,7 @@ mtype_list(const struct ip_set *set,
>   rcu_read_lock();
>   for (; cb->args[IPSET_CB_ARG0] < map->elements;
>cb->args[IPSET_CB_ARG0]++) {
> + cond_resched_rcu();
>   id = cb->args[IPSET_CB_ARG0];
>   x = get_ext(set, map, id);
>   if (!test_bit(id, map->members) ||
> diff --git a/net/netfilter/ipset/ip_set_core.c 
> b/net/netfilter/ipset/ip_set_core.c
> index 1f3c03b3bebf2..89b44458a7613 100644
> --- a/net/netfilter/ipset/ip_set_core.c
> +++ b/net/netfilter/ipset/ip_set_core.c
> @@ -1388,9 +1388,7 @@ ip_set_dump_start(struct sk_buff *skb, struct 
> netlink_callback *cb)
>   set->variant->uref(set, cb, true);
>   /* fall through */
>   default:
> - rcu_read_lock_bh();
>   ret = set->variant->list(set, skb, cb);
> - rcu_read_unlock_bh();
>   if (!cb->args[IPSET_CB_ARG0])
>   /* Set is done, proceed with next one */
>   goto next_set;
> diff --git a/net/netfilter/ipset/ip_set_hash_gen.h 
> b/net/netfilter/ipset/ip_set_hash_gen.h
> index efffc8eabafea..8ef079db7d347 100644
> --- a/net/netfilter/ipset/ip_set_hash_gen.h
> +++ b/net/netfilter/ipset/ip_set_hash_gen.h
> @@ -1143,6 +1143,7 @@ mtype_list(const struct ip_set *set,
>   rcu_read_lock();
>   for (; cb->args[IPSET_CB_ARG0] < jhash_size(t->htable_bits);
>cb->args[IPSET_CB_ARG0]++) {
> + cond_resched_rcu();
>   incomplete = skb_tail_pointer(skb);
>   n = rcu_dereference(hbucket(t, cb->args[IPSET_CB_ARG0]));
>   pr_debug("cb->arg bucket: %lu, t %p n %p\n",
> -- 
> 2.14.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH ipset nf-next] netfilter: ipset: use nfnl_mutex_is_locked

[Jozsef Kadlecsik]

Hi Florian,

On Thu, 30 Nov 2017, Florian Westphal wrote:

> Check that we really hold nfnl mutex here instead of relying on correct
> usage alone.
> 
> Signed-off-by: Florian Westphal <f...@strlen.de>

Yes, it's better this way :-)

Acked-by: Jozsef Kadlecsik <kad...@blackhole.kfki.hu>

> ---
> diff --git a/net/netfilter/ipset/ip_set_core.c 
> b/net/netfilter/ipset/ip_set_core.c
> index d5a43cad90f04..1f3c03b3bebf2 100644
> --- a/net/netfilter/ipset/ip_set_core.c
> +++ b/net/netfilter/ipset/ip_set_core.c
> @@ -57,7 +57,7 @@ MODULE_ALIAS_NFNL_SUBSYS(NFNL_SUBSYS_IPSET);
>  
>  /* When the nfnl mutex is held: */
>  #define ip_set_dereference(p)\
> - rcu_dereference_protected(p, 1)
> + rcu_dereference_protected(p, lockdep_nfnl_is_held(NFNL_SUBSYS_IPSET))
>  #define ip_set(inst, id) \
>   ip_set_dereference((inst)->ip_set_list)[id]
>  
> -- 
> 2.14.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH nf] netfilter: conntrack: lower timeout to RETRANS seconds if window is 0

[Jozsef Kadlecsik]

On Sun, 19 Nov 2017, Florian Westphal wrote:

> When zero window is announced we can get into a situation where
> connection stays around forever:
> 
> 1. One side announces zero window.
> 2. Other side closes.
> 
> In this case, no FIN is sent (stuck in send queue).
> 
> Unless other side opens the window up again conntrack
> stays in ESTABLISHED state for a very long time.
> 
> Lets alleviate this by lowering the timeout to RETRANS (5 minutes),
> the other end should be sending zero window probes to keep the
> connection established as long as a socket still exists.
> 
> Cc: Jozsef Kadlecsik <kad...@blackhole.kfki.hu>
> Signed-off-by: Florian Westphal <f...@strlen.de>

Acked-by: Jozsef Kadlecsik <kad...@blackhole.kfki.hu>

Thanks, Florian!
Jozsef
> ---
>  net/netfilter/nf_conntrack_proto_tcp.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/netfilter/nf_conntrack_proto_tcp.c 
> b/net/netfilter/nf_conntrack_proto_tcp.c
> index c11b04d269ea..684cc29010a0 100644
> --- a/net/netfilter/nf_conntrack_proto_tcp.c
> +++ b/net/netfilter/nf_conntrack_proto_tcp.c
> @@ -1039,6 +1039,9 @@ static int tcp_packet(struct nf_conn *ct,
>IP_CT_TCP_FLAG_DATA_UNACKNOWLEDGED &&
>timeouts[new_state] > timeouts[TCP_CONNTRACK_UNACK])
>   timeout = timeouts[TCP_CONNTRACK_UNACK];
> + else if (ct->proto.tcp.last_win == 0 &&
> +  timeouts[new_state] > timeouts[TCP_CONNTRACK_RETRANS])
> + timeout = timeouts[TCP_CONNTRACK_RETRANS];
>   else
>   timeout = timeouts[new_state];
>   spin_unlock_bh(>lock);
> -- 
> 2.13.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 11/21] clusterip: exit_net cleanup check added

[Jozsef Kadlecsik]

Hello Vasily,

On Mon, 6 Nov 2017, Vasily Averin wrote:

> Be sure that configs list initialized in net_init hook was return
> to initial state.

What is the goal of the patch series you sent in the third version in a 
row?

- If the deinitializations are missing from the files, the patches 
  do not fix them, just emit warnings.
- If the deinitializations are not missing, the patches are totally 
  unnecessary.

It looks like debugging... but not expressed that way, neither in the 
subject lines nor in the patch descriptions.

Best regards,
Jozsef

> Signed-off-by: Vasily Averin 
> ---
>  net/ipv4/netfilter/ipt_CLUSTERIP.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c 
> b/net/ipv4/netfilter/ipt_CLUSTERIP.c
> index 17b4ca5..4364a88 100644
> --- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
> +++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
> @@ -819,6 +819,9 @@ static void clusterip_net_exit(struct net *net)
>   cn->procdir = NULL;
>  #endif
>   nf_unregister_net_hook(net, _arp_ops);
> + WARN_ONCE(!list_empty(>configs),
> +   "net %x %s: configs list is not empty\n",
> +   net->ns.inum, __func__);
>  }
>  
>  static struct pernet_operations clusterip_net_ops = {
> -- 
> 2.7.4
> 
> 

-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] netfilter: ipset: use swap macro instead of _manually_ swapping values

[Jozsef Kadlecsik]

Hi,

On Mon, 30 Oct 2017, Gustavo A. R. Silva wrote:

> Make use of the swap macro and remove unnecessary variables tmp.
> This makes the code easier to read and maintain.
> 
> This code was detected with the help of Coccinelle.
> 
> Signed-off-by: Gustavo A. R. Silva 
> ---
>  net/netfilter/ipset/ip_set_bitmap_ip.c| 8 ++--
>  net/netfilter/ipset/ip_set_bitmap_ipmac.c | 8 ++--
>  net/netfilter/ipset/ip_set_bitmap_port.c  | 8 ++--
>  3 files changed, 6 insertions(+), 18 deletions(-)

Patch is applied in the ipset git tree and will be included in the next 
batch. Thanks!

Best regards,
Jozsef

> diff --git a/net/netfilter/ipset/ip_set_bitmap_ip.c 
> b/net/netfilter/ipset/ip_set_bitmap_ip.c
> index d8975a0..488d6d0 100644
> --- a/net/netfilter/ipset/ip_set_bitmap_ip.c
> +++ b/net/netfilter/ipset/ip_set_bitmap_ip.c
> @@ -263,12 +263,8 @@ bitmap_ip_create(struct net *net, struct ip_set *set, 
> struct nlattr *tb[],
>   ret = ip_set_get_hostipaddr4(tb[IPSET_ATTR_IP_TO], _ip);
>   if (ret)
>   return ret;
> - if (first_ip > last_ip) {
> - u32 tmp = first_ip;
> -
> - first_ip = last_ip;
> - last_ip = tmp;
> - }
> + if (first_ip > last_ip)
> + swap(first_ip, last_ip);
>   } else if (tb[IPSET_ATTR_CIDR]) {
>   u8 cidr = nla_get_u8(tb[IPSET_ATTR_CIDR]);
>  
> diff --git a/net/netfilter/ipset/ip_set_bitmap_ipmac.c 
> b/net/netfilter/ipset/ip_set_bitmap_ipmac.c
> index 4c279fb..c00b6a2 100644
> --- a/net/netfilter/ipset/ip_set_bitmap_ipmac.c
> +++ b/net/netfilter/ipset/ip_set_bitmap_ipmac.c
> @@ -337,12 +337,8 @@ bitmap_ipmac_create(struct net *net, struct ip_set *set, 
> struct nlattr *tb[],
>   ret = ip_set_get_hostipaddr4(tb[IPSET_ATTR_IP_TO], _ip);
>   if (ret)
>   return ret;
> - if (first_ip > last_ip) {
> - u32 tmp = first_ip;
> -
> - first_ip = last_ip;
> - last_ip = tmp;
> - }
> + if (first_ip > last_ip)
> + swap(first_ip, last_ip);
>   } else if (tb[IPSET_ATTR_CIDR]) {
>   u8 cidr = nla_get_u8(tb[IPSET_ATTR_CIDR]);
>  
> diff --git a/net/netfilter/ipset/ip_set_bitmap_port.c 
> b/net/netfilter/ipset/ip_set_bitmap_port.c
> index 7f9bbd7..b561ca8 100644
> --- a/net/netfilter/ipset/ip_set_bitmap_port.c
> +++ b/net/netfilter/ipset/ip_set_bitmap_port.c
> @@ -238,12 +238,8 @@ bitmap_port_create(struct net *net, struct ip_set *set, 
> struct nlattr *tb[],
>  
>   first_port = ip_set_get_h16(tb[IPSET_ATTR_PORT]);
>   last_port = ip_set_get_h16(tb[IPSET_ATTR_PORT_TO]);
> - if (first_port > last_port) {
> - u16 tmp = first_port;
> -
> - first_port = last_port;
> - last_port = tmp;
> - }
> + if (first_port > last_port)
> + swap(first_port, last_port);
>  
>   elements = last_port - first_port + 1;
>   set->dsize = ip_set_elem_len(set, tb, 0, 0);
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] netfilter: ipset: ip_set_bitmap_ipmac: use swap macro in bitmap_ipmac_create

[Jozsef Kadlecsik]

Hi,

On Sat, 28 Oct 2017, Gustavo A. R. Silva wrote:

> Make use of the swap macro and remove unnecessary variable tmp.
> This makes the code easier to read and maintain.
> 
> This code was detected with the help of Coccinelle.
> 
> Signed-off-by: Gustavo A. R. Silva 

Please resubmit the tree patches as a single one, they do the same thing. 
Thanks!

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] netfilter: ipset: Convert timers to use timer_setup()

[Jozsef Kadlecsik]

Hi,

On Wed, 4 Oct 2017, Kees Cook wrote:

> In preparation for unconditionally passing the struct timer_list pointer 
> to all timer callbacks, switch to using the new timer_setup() and 
> from_timer() to pass the timer pointer explicitly. This introduces a 
> pointer back to the struct ip_set, which is used instead of the struct 
> timer_list .data field.

Please add the same changes to net/netfilter/ipset/ip_set_list.c too, in 
order to handle all ipset modules in a single patch. I don't see a way 
either to avoid the introduction of the new pointer.

Acked-by: Jozsef Kadlecsik <kad...@blackhole.kfki.hu>

Best regards,
Jozsef
 
> Cc: Pablo Neira Ayuso <pa...@netfilter.org>
> Cc: Jozsef Kadlecsik <kad...@blackhole.kfki.hu>
> Cc: Florian Westphal <f...@strlen.de>
> Cc: "David S. Miller" <da...@davemloft.net>
> Cc: Stephen Hemminger <step...@networkplumber.org>
> Cc: simran singhal <singhalsimr...@gmail.com>
> Cc: Muhammad Falak R Wani <falakre...@gmail.com>
> Cc: netfilter-devel@vger.kernel.org
> Cc: coret...@netfilter.org
> Cc: net...@vger.kernel.org
> Cc: Thomas Gleixner <t...@linutronix.de>
> Signed-off-by: Kees Cook <keesc...@chromium.org>
> ---
> This requires commit 686fef928bba ("timer: Prepare to change timer
> callback argument type") in v4.14-rc3, but should be otherwise
> stand-alone.
> ---
>  net/netfilter/ipset/ip_set_bitmap_gen.h   | 10 +-
>  net/netfilter/ipset/ip_set_bitmap_ip.c|  2 ++
>  net/netfilter/ipset/ip_set_bitmap_ipmac.c |  2 ++
>  net/netfilter/ipset/ip_set_bitmap_port.c  |  2 ++
>  net/netfilter/ipset/ip_set_hash_gen.h | 12 +++-
>  5 files changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/net/netfilter/ipset/ip_set_bitmap_gen.h 
> b/net/netfilter/ipset/ip_set_bitmap_gen.h
> index 8ad2b52a0b32..5ca18f07683b 100644
> --- a/net/netfilter/ipset/ip_set_bitmap_gen.h
> +++ b/net/netfilter/ipset/ip_set_bitmap_gen.h
> @@ -37,11 +37,11 @@
>  #define get_ext(set, map, id)((map)->extensions + ((set)->dsize * 
> (id)))
>  
>  static void
> -mtype_gc_init(struct ip_set *set, void (*gc)(unsigned long ul_set))
> +mtype_gc_init(struct ip_set *set, void (*gc)(struct timer_list *t))
>  {
>   struct mtype *map = set->data;
>  
> - setup_timer(>gc, gc, (unsigned long)set);
> + timer_setup(>gc, gc, 0);
>   mod_timer(>gc, jiffies + IPSET_GC_PERIOD(set->timeout) * HZ);
>  }
>  
> @@ -272,10 +272,10 @@ mtype_list(const struct ip_set *set,
>  }
>  
>  static void
> -mtype_gc(unsigned long ul_set)
> +mtype_gc(struct timer_list *t)
>  {
> - struct ip_set *set = (struct ip_set *)ul_set;
> - struct mtype *map = set->data;
> + struct mtype *map = from_timer(map, t, gc);
> + struct ip_set *set = map->set;
>   void *x;
>   u32 id;
>  
> diff --git a/net/netfilter/ipset/ip_set_bitmap_ip.c 
> b/net/netfilter/ipset/ip_set_bitmap_ip.c
> index 4783efff0bde..d8975a0b4282 100644
> --- a/net/netfilter/ipset/ip_set_bitmap_ip.c
> +++ b/net/netfilter/ipset/ip_set_bitmap_ip.c
> @@ -48,6 +48,7 @@ struct bitmap_ip {
>   size_t memsize; /* members size */
>   u8 netmask; /* subnet netmask */
>   struct timer_list gc;   /* garbage collection */
> + struct ip_set *set; /* attached to this ip_set */
>   unsigned char extensions[0] /* data extensions */
>   __aligned(__alignof__(u64));
>  };
> @@ -232,6 +233,7 @@ init_map_ip(struct ip_set *set, struct bitmap_ip *map,
>   map->netmask = netmask;
>   set->timeout = IPSET_NO_TIMEOUT;
>  
> + map->set = set;
>   set->data = map;
>   set->family = NFPROTO_IPV4;
>  
> diff --git a/net/netfilter/ipset/ip_set_bitmap_ipmac.c 
> b/net/netfilter/ipset/ip_set_bitmap_ipmac.c
> index 9a065f672d3a..4c279fbd2d5d 100644
> --- a/net/netfilter/ipset/ip_set_bitmap_ipmac.c
> +++ b/net/netfilter/ipset/ip_set_bitmap_ipmac.c
> @@ -52,6 +52,7 @@ struct bitmap_ipmac {
>   u32 elements;   /* number of max elements in the set */
>   size_t memsize; /* members size */
>   struct timer_list gc;   /* garbage collector */
> + struct ip_set *set; /* attached to this ip_set */
>   unsigned char extensions[0] /* MAC + data extensions */
>   __aligned(__alignof__(u64));
>  };
> @@ -307,6 +308,7 @@ init_map_ipmac(struct ip_set *set, struct bitmap_ipmac 
> *map,
>   map->elements = elements;
>   set->timeout = IPSET_NO_TIMEOUT;
>  
> + map->set = set;
>   set->data = map;
>   set->family = NFPROTO_IPV4;
>  
> diff --git a/net/

RE: [PATCH] netfilter: fix stringop-overflow warning with UBSAN

[Jozsef Kadlecsik]

Hi,

[Sorry, at holiday I just cursory watched the mailing lists.]

On Tue, 1 Aug 2017, David Laight wrote:

> From: Arnd Bergmann
> > Sent: 31 July 2017 11:09
> > Using gcc-7 with UBSAN enabled, we get this false-positive warning:
> > 
> > net/netfilter/ipset/ip_set_core.c: In function 'ip_set_sockfn_get':
> > net/netfilter/ipset/ip_set_core.c:1998:3: error: 'strncpy' writing 32 bytes 
> > into a region of size 2
> > overflows the destination [-Werror=stringop-overflow=]
> >strncpy(req_get->set.name, set ? set->name : "",
> >^~~~
> > sizeof(req_get->set.name));
> > ~~
> > 
> > This seems completely bogus, and I could not find a nice workaround.
> > To work around it in a less elegant way, I change the ?: operator
> > into an if()/else() construct.
> > 
> > Signed-off-by: Arnd Bergmann 
> > ---
> >  net/netfilter/ipset/ip_set_core.c | 8 ++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/net/netfilter/ipset/ip_set_core.c 
> > b/net/netfilter/ipset/ip_set_core.c
> > index e495b5e484b1..d7ebb021003b 100644
> > --- a/net/netfilter/ipset/ip_set_core.c
> > +++ b/net/netfilter/ipset/ip_set_core.c
> > @@ -1995,8 +1995,12 @@ ip_set_sockfn_get(struct sock *sk, int optval, void 
> > __user *user, int *len)
> > }
> > nfnl_lock(NFNL_SUBSYS_IPSET);
> > set = ip_set(inst, req_get->set.index);
> > -   strncpy(req_get->set.name, ,
> > -   IPSET_MAXNAMELEN);
> > +   if (set)
> > +   strncpy(req_get->set.name, set->name,
> > +   sizeof(req_get->set.name));
> > +   else
> > +   memset(req_get->set.name, '\0',
> > +  sizeof(req_get->set.name));
> 
> If you use strncpy() here, the compiler might optimise the code
> back to 'how it was before'.
> 
> Or, maybe an explicit temporary: 'const char *name = set ? set->name : "";

I think the best to go with the explicit temporary variable. The if-else 
construct is too much for such a case.

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] netfilter: ipset: Fix race between dump and swap

[Jozsef Kadlecsik]

On Wed, 27 Sep 2017, Ross Lagerwall wrote:

> Fix a race between ip_set_dump_start() and ip_set_swap().
> The race is as follows:
> * Without holding the ref lock, ip_set_swap() checks ref_netlink of the
>   set and it is 0.
> * ip_set_dump_start() takes a reference on the set.
> * ip_set_swap() does the swap (even though it now has a non-zero
>   reference count).
> * ip_set_dump_start() gets the set from ip_set_list again which is now a
>   different set since it has been swapped.
> * ip_set_dump_start() calls __ip_set_put_netlink() and hits a BUG_ON due
>   to the reference count being 0.
> 
> Fix this race by extending the critical region in which the ref lock is
> held to include checking the ref counts.
> 
> The race can be reproduced with the following script:
>   while :; do
> ipset destroy hash_ip1
> ipset destroy hash_ip2
> ipset create hash_ip1 hash:ip family inet hashsize 1024 \
> maxelem 50
> ipset create hash_ip2 hash:ip family inet hashsize 30 \
> maxelem 50
> ipset create hash_ip3 hash:ip family inet hashsize 1024 \
> maxelem 50
> ipset save &
> ipset swap hash_ip3 hash_ip2
> ipset destroy hash_ip3
>     wait
>   done
> 
> Signed-off-by: Ross Lagerwall <ross.lagerw...@citrix.com>

Acked-by: Jozsef Kadlecsik <kad...@blackhole.kfki.hu>

Good catch, Pablo please apply in the nf tree. Thanks!

Best regards,
Jozsef

> ---
>  net/netfilter/ipset/ip_set_core.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/net/netfilter/ipset/ip_set_core.c 
> b/net/netfilter/ipset/ip_set_core.c
> index ba6a551..cae3e4a 100644
> --- a/net/netfilter/ipset/ip_set_core.c
> +++ b/net/netfilter/ipset/ip_set_core.c
> @@ -1185,14 +1185,17 @@ static int ip_set_swap(struct net *net, struct sock 
> *ctnl, struct sk_buff *skb,
> from->family == to->family))
>   return -IPSET_ERR_TYPE_MISMATCH;
>  
> - if (from->ref_netlink || to->ref_netlink)
> + write_lock_bh(_set_ref_lock);
> +
> + if (from->ref_netlink || to->ref_netlink) {
> + write_unlock_bh(_set_ref_lock);
>   return -EBUSY;
> + }
>  
>   strncpy(from_name, from->name, IPSET_MAXNAMELEN);
>   strncpy(from->name, to->name, IPSET_MAXNAMELEN);
>   strncpy(to->name, from_name, IPSET_MAXNAMELEN);
>  
> - write_lock_bh(_set_ref_lock);
>   swap(from->ref, to->ref);
>   ip_set(inst, from_id) = to;
>   ip_set(inst, to_id) = from;
> -- 
> 2.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] Fix adding an IPv4 range containing more than 2^31 addresses

[Jozsef Kadlecsik]

On Tue, 26 Sep 2017, Pablo Neira Ayuso wrote:

> On Sat, Sep 23, 2017 at 11:37:40PM +0200, Jozsef Kadlecsik wrote:
> > Wrong comparison prevented the hash types to add a range with
> > more than 2^31 addresses but reported as a success.
> > 
> > Fixes bugzilla id #1005, reported by Oleg Serditov and Oliver Ford.
> 
> Applied, thanks Jozsef.
> 
> Jozsef, I'm prepending the following to this patch title: "netfilter: ipset:"

I forgot to add that prefix... Thanks indeed!

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH nf v2] netfilter: ipset: pernet ops must be unregistered last

[Jozsef Kadlecsik]

On Tue, 26 Sep 2017, Florian Westphal wrote:

> Removing the ipset module leaves a small window where one cpu performs
> module removal while another runs a command like 'ipset flush'.
> 
> ipset uses net_generic(), unregistering the pernet ops frees this
> storage area.
> 
> Fix it by first removing the user-visible api handlers and the pernet
> ops last.
> 
> Fixes: 1785e8f473082 ("netfiler: ipset: Add net namespace for ipset")
> Reported-by: Li Shuang <shu...@redhat.com>
> Signed-off-by: Florian Westphal <f...@strlen.de>

Acked-by: Jozsef Kadlecsik <kad...@blackhole.kfki.hu>

Pablo, could you apply the patch in the nf tree?

Best regards,
Jozsef

> ---
>  Change since v2:
>   - unregister setsockopt first (i.e., use reverse order)
> 
>  net/netfilter/ipset/ip_set_core.c | 22 +-
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/net/netfilter/ipset/ip_set_core.c 
> b/net/netfilter/ipset/ip_set_core.c
> index e495b5e484b1..a7f049ff3049 100644
> --- a/net/netfilter/ipset/ip_set_core.c
> +++ b/net/netfilter/ipset/ip_set_core.c
> @@ -2072,25 +2072,28 @@ static struct pernet_operations ip_set_net_ops = {
>  static int __init
>  ip_set_init(void)
>  {
> - int ret = nfnetlink_subsys_register(_set_netlink_subsys);
> + int ret = register_pernet_subsys(_set_net_ops);
>  
> + if (ret) {
> + pr_err("ip_set: cannot register pernet_subsys.\n");
> + return ret;
> + }
> +
> + ret = nfnetlink_subsys_register(_set_netlink_subsys);
>   if (ret != 0) {
>   pr_err("ip_set: cannot register with nfnetlink.\n");
> + unregister_pernet_subsys(_set_net_ops);
>   return ret;
>   }
> +
>   ret = nf_register_sockopt(_set);
>   if (ret != 0) {
>   pr_err("SO_SET registry failed: %d\n", ret);
>   nfnetlink_subsys_unregister(_set_netlink_subsys);
> + unregister_pernet_subsys(_set_net_ops);
>   return ret;
>   }
> - ret = register_pernet_subsys(_set_net_ops);
> - if (ret) {
> - pr_err("ip_set: cannot register pernet_subsys.\n");
> - nf_unregister_sockopt(_set);
> - nfnetlink_subsys_unregister(_set_netlink_subsys);
> - return ret;
> - }
> +
>   pr_info("ip_set: protocol %u\n", IPSET_PROTOCOL);
>   return 0;
>  }
> @@ -2098,9 +2101,10 @@ ip_set_init(void)
>  static void __exit
>  ip_set_fini(void)
>  {
> - unregister_pernet_subsys(_set_net_ops);
>   nf_unregister_sockopt(_set);
>   nfnetlink_subsys_unregister(_set_netlink_subsys);
> +
> + unregister_pernet_subsys(_set_net_ops);
>   pr_debug("these are the famous last words\n");
>  }
>  
> -- 
> 2.13.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH nf] netfilter: ipset: pernet ops must be unregistered last

[Jozsef Kadlecsik]

Hi Florian,

On Tue, 26 Sep 2017, Florian Westphal wrote:

> Removing the ipset module leaves a small window where one cpu performs
> module removal while another runs a command like 'ipset flush'.
> 
> ipset uses net_generic(), unregistering the pernet ops frees this
> storage area.
> 
> Fix it by first removing the user-visible api handlers and the pernet
> ops last.
> 
> Fixes: 1785e8f473082 ("netfiler: ipset: Add net namespace for ipset")
> Reported-by: Li Shuang 
> Signed-off-by: Florian Westphal 
> ---
>  net/netfilter/ipset/ip_set_core.c | 23 +--
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/net/netfilter/ipset/ip_set_core.c 
> b/net/netfilter/ipset/ip_set_core.c
> index e495b5e484b1..5ec7740c2088 100644
> --- a/net/netfilter/ipset/ip_set_core.c
> +++ b/net/netfilter/ipset/ip_set_core.c
> @@ -2072,25 +2072,27 @@ static struct pernet_operations ip_set_net_ops = {
>  static int __init
>  ip_set_init(void)
>  {
> - int ret = nfnetlink_subsys_register(_set_netlink_subsys);
> + int ret = register_pernet_subsys(_set_net_ops);
> + if (ret) {
> + pr_err("ip_set: cannot register pernet_subsys.\n");
> + return ret;
> + }
>  
> + ret = nfnetlink_subsys_register(_set_netlink_subsys);
>   if (ret != 0) {
>   pr_err("ip_set: cannot register with nfnetlink.\n");
> + unregister_pernet_subsys(_set_net_ops);
>   return ret;
>   }
> +
>   ret = nf_register_sockopt(_set);
>   if (ret != 0) {
>   pr_err("SO_SET registry failed: %d\n", ret);
>   nfnetlink_subsys_unregister(_set_netlink_subsys);
> + unregister_pernet_subsys(_set_net_ops);
>   return ret;
>   }
> - ret = register_pernet_subsys(_set_net_ops);
> - if (ret) {
> - pr_err("ip_set: cannot register pernet_subsys.\n");
> - nf_unregister_sockopt(_set);
> - nfnetlink_subsys_unregister(_set_netlink_subsys);
> - return ret;
> - }
> +
>   pr_info("ip_set: protocol %u\n", IPSET_PROTOCOL);
>   return 0;
>  }
> @@ -2098,9 +2100,10 @@ ip_set_init(void)
>  static void __exit
>  ip_set_fini(void)
>  {
> - unregister_pernet_subsys(_set_net_ops);
> - nf_unregister_sockopt(_set);
>   nfnetlink_subsys_unregister(_set_netlink_subsys);
> + nf_unregister_sockopt(_set);
> +
> + unregister_pernet_subsys(_set_net_ops);
>   pr_debug("these are the famous last words\n");
>  }

Following the registration sequence in backward order, shouldn't first the 
sockopt, then the netlink_subsys be unregistered? Even if it doesn't 
matter, I think it'd be more consistent.

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ipset: fix build with musl

[Jozsef Kadlecsik]

On Mon, 25 Sep 2017, Jozsef Kadlecsik wrote:

> > Include sys/types.h for u_int8_t and define _GNU_SOURCE for musl to
> > expose it.
> > 
> > Fixes: 54802b2c2826 ("Report if the option is supported by a newer kernel 
> > release")
> > Signed-off-by: Stijn Tintel <st...@linux-ipv6.be>
> 
> Patch is applied in ipset git tree, thanks.

I reverted your patch and applied the proper fix: replaced the last 
occurence of u_int8_t in the ipset userspace tree with uint8_t.

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ipset: fix build with musl

[Jozsef Kadlecsik]

Hi Stijn,

On Mon, 25 Sep 2017, Stijn Tintel wrote:

> Include sys/types.h for u_int8_t and define _GNU_SOURCE for musl to
> expose it.
> 
> Fixes: 54802b2c2826 ("Report if the option is supported by a newer kernel 
> release")
> Signed-off-by: Stijn Tintel 
> ---
>  src/ipset.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/ipset.c b/src/ipset.c
> index 79f56b8..8d70abc 100644
> --- a/src/ipset.c
> +++ b/src/ipset.c
> @@ -14,6 +14,8 @@
>  #include/* fprintf, fgets */
>  #include   /* exit */
>  #include   /* str* */
> +#define _GNU_SOURCE
> +#include/* u_int8_t */
>  
>  #include 
>  
> -- 

Patch is applied in ipset git tree, thanks.

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/1] ipset patch for nf

[Jozsef Kadlecsik]

Hi Pablo,

Please apply the next patch for the nf tree. It fixes the issue
when an IP range with more than 2^31 addresses was specified to
add (bugzilla id #1005)

- Fix adding an IPv4 range containing more than 2^31 addresses,
  which failed silently due to wrong comparison (bugzilla id #1005).

Thanks,
Jozsef
-

The following changes since commit 7f4f7dd4417d9efd038b14d39c70170db2e0baa0:

  netfilter: ipset: ipset list may return wrong member count for set with 
timeout (2017-09-18 17:35:32 +0200)

are available in the git repository at:

  git://blackhole.kfki.hu/nf 

for you to fetch changes up to 625a0411a81ae51c12bde4452171f08174b656cd:

  Fix adding an IPv4 range containing more than 2^31 addresses (2017-09-23 
23:28:13 +0200)


Jozsef Kadlecsik (1):
  Fix adding an IPv4 range containing more than 2^31 addresses

 net/netfilter/ipset/ip_set_hash_ip.c | 22 --
 net/netfilter/ipset/ip_set_hash_ipmark.c |  2 +-
 net/netfilter/ipset/ip_set_hash_ipport.c |  2 +-
 net/netfilter/ipset/ip_set_hash_ipportip.c   |  2 +-
 net/netfilter/ipset/ip_set_hash_ipportnet.c  |  4 ++--
 net/netfilter/ipset/ip_set_hash_net.c|  2 +-
 net/netfilter/ipset/ip_set_hash_netiface.c   |  2 +-
 net/netfilter/ipset/ip_set_hash_netnet.c |  4 ++--
 net/netfilter/ipset/ip_set_hash_netport.c|  2 +-
 net/netfilter/ipset/ip_set_hash_netportnet.c |  4 ++--
 10 files changed, 24 insertions(+), 22 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/1] Fix adding an IPv4 range containing more than 2^31 addresses

[Jozsef Kadlecsik]

Wrong comparison prevented the hash types to add a range with
more than 2^31 addresses but reported as a success.

Fixes bugzilla id #1005, reported by Oleg Serditov and Oliver Ford.

Signed-off-by: Jozsef Kadlecsik <kad...@blackhole.kfki.hu>
---
 net/netfilter/ipset/ip_set_hash_ip.c | 22 --
 net/netfilter/ipset/ip_set_hash_ipmark.c |  2 +-
 net/netfilter/ipset/ip_set_hash_ipport.c |  2 +-
 net/netfilter/ipset/ip_set_hash_ipportip.c   |  2 +-
 net/netfilter/ipset/ip_set_hash_ipportnet.c  |  4 ++--
 net/netfilter/ipset/ip_set_hash_net.c|  2 +-
 net/netfilter/ipset/ip_set_hash_netiface.c   |  2 +-
 net/netfilter/ipset/ip_set_hash_netnet.c |  4 ++--
 net/netfilter/ipset/ip_set_hash_netport.c|  2 +-
 net/netfilter/ipset/ip_set_hash_netportnet.c |  4 ++--
 10 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_hash_ip.c 
b/net/netfilter/ipset/ip_set_hash_ip.c
index 20bfbd3..613eb21 100644
--- a/net/netfilter/ipset/ip_set_hash_ip.c
+++ b/net/netfilter/ipset/ip_set_hash_ip.c
@@ -123,13 +123,12 @@ hash_ip4_uadt(struct ip_set *set, struct nlattr *tb[],
return ret;
 
ip &= ip_set_hostmask(h->netmask);
+   e.ip = htonl(ip);
+   if (e.ip == 0)
+   return -IPSET_ERR_HASH_ELEM;
 
-   if (adt == IPSET_TEST) {
-   e.ip = htonl(ip);
-   if (e.ip == 0)
-   return -IPSET_ERR_HASH_ELEM;
+   if (adt == IPSET_TEST)
return adtfn(set, , , , flags);
-   }
 
ip_to = ip;
if (tb[IPSET_ATTR_IP_TO]) {
@@ -148,17 +147,20 @@ hash_ip4_uadt(struct ip_set *set, struct nlattr *tb[],
 
hosts = h->netmask == 32 ? 1 : 2 << (32 - h->netmask - 1);
 
-   if (retried)
+   if (retried) {
ip = ntohl(h->next.ip);
-   for (; !before(ip_to, ip); ip += hosts) {
e.ip = htonl(ip);
-   if (e.ip == 0)
-   return -IPSET_ERR_HASH_ELEM;
+   }
+   for (; ip <= ip_to;) {
ret = adtfn(set, , , , flags);
-
if (ret && !ip_set_eexist(ret, flags))
return ret;
 
+   ip += hosts;
+   e.ip = htonl(ip);
+   if (e.ip == 0)
+   return 0;
+
ret = 0;
}
return ret;
diff --git a/net/netfilter/ipset/ip_set_hash_ipmark.c 
b/net/netfilter/ipset/ip_set_hash_ipmark.c
index b64cf14..f3ba834 100644
--- a/net/netfilter/ipset/ip_set_hash_ipmark.c
+++ b/net/netfilter/ipset/ip_set_hash_ipmark.c
@@ -149,7 +149,7 @@ hash_ipmark4_uadt(struct ip_set *set, struct nlattr *tb[],
 
if (retried)
ip = ntohl(h->next.ip);
-   for (; !before(ip_to, ip); ip++) {
+   for (; ip <= ip_to; ip++) {
e.ip = htonl(ip);
ret = adtfn(set, , , , flags);
 
diff --git a/net/netfilter/ipset/ip_set_hash_ipport.c 
b/net/netfilter/ipset/ip_set_hash_ipport.c
index f438740..ddb8039 100644
--- a/net/netfilter/ipset/ip_set_hash_ipport.c
+++ b/net/netfilter/ipset/ip_set_hash_ipport.c
@@ -178,7 +178,7 @@ hash_ipport4_uadt(struct ip_set *set, struct nlattr *tb[],
 
if (retried)
ip = ntohl(h->next.ip);
-   for (; !before(ip_to, ip); ip++) {
+   for (; ip <= ip_to; ip++) {
p = retried && ip == ntohl(h->next.ip) ? ntohs(h->next.port)
   : port;
for (; p <= port_to; p++) {
diff --git a/net/netfilter/ipset/ip_set_hash_ipportip.c 
b/net/netfilter/ipset/ip_set_hash_ipportip.c
index 6215fb8..a7f4d7a 100644
--- a/net/netfilter/ipset/ip_set_hash_ipportip.c
+++ b/net/netfilter/ipset/ip_set_hash_ipportip.c
@@ -185,7 +185,7 @@ hash_ipportip4_uadt(struct ip_set *set, struct nlattr *tb[],
 
if (retried)
ip = ntohl(h->next.ip);
-   for (; !before(ip_to, ip); ip++) {
+   for (; ip <= ip_to; ip++) {
p = retried && ip == ntohl(h->next.ip) ? ntohs(h->next.port)
   : port;
for (; p <= port_to; p++) {
diff --git a/net/netfilter/ipset/ip_set_hash_ipportnet.c 
b/net/netfilter/ipset/ip_set_hash_ipportnet.c
index 5ab1b99..a2f19b9 100644
--- a/net/netfilter/ipset/ip_set_hash_ipportnet.c
+++ b/net/netfilter/ipset/ip_set_hash_ipportnet.c
@@ -271,7 +271,7 @@ hash_ipportnet4_uadt(struct ip_set *set, struct nlattr 
*tb[],
 
if (retried)
ip = ntohl(h->next.ip);
-   for (; !before(ip_to, ip); ip++) {
+   for (; ip <= ip_to; ip++) {
e.ip = htonl(ip);
p = retried && ip == ntohl(h->next.ip) ? ntohs(h->next.port)
   : port;
@@ -281,7 +281,7 @@ hash_ipportnet4_uadt(struct ip_set *set, struct nla

[ANNOUNCE] ipset 6.34 released

[Jozsef Kadlecsik]

Hi,

I'm happy to announce ipset 6.34, which fixes a couple bugtrack issues.
(Due to the missing function from libipset.map, v6.33 could not
successfully be built on all systems.)

Userspace changes:
  - testsuite: Make sure it can be run over ssh :-)
  - Reset state after a command failed, when multiple ones are issued
(bugzilla id #1158, reported by Dimitri Grischin)
  - Handle padding attribute properly in userspace.
  - Test to check the fix to add an IPv4 range containing more than 2^31
addresses
  - Fix the include guards on the include/libipset/linux_ip_set*.h
(bugzilla id #1139, suggested by Quentin Armitage)
  - New function added in commit 54802b2c is missing from libipset.map
(bugzilla id #1182, reported by irher...@gmail.com)

Kernel part changes:
  - Fix adding an IPv4 range containing more than 2^31 addresses
(bugzilla id #1005, reported by Oleg Serditov and Oliver Ford)

You can download the source code of ipset from:
http://ipset.netfilter.org
ftp://ftp.netfilter.org/pub/ipset/
git://git.netfilter.org/ipset.git

Best regards,
Jozsef
-
E-mail  : kad...@blackhole.kfki.hu, kadlecsik.joz...@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
  H-1525 Budapest 114, POB. 49, Hungary
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html